Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(switch): accept revset arguments, and explicitly support - shorthand #1463

Merged
merged 4 commits into from
Feb 24, 2025

Conversation

claytonrcarter
Copy link
Collaborator

@claytonrcarter claytonrcarter commented Dec 8, 2024

This updated git switch to accept a revset as it's argument, and also to support - (to checkout previously checked out branch/commit) by passing it through to git.

The revset parameter must evaluate to a set with a single head, and I've found this handy in cases like current(...) to move to the latest version of a commit if I had an old smartlog or SHA still on screen or such, and also switch foo: for when I have a branch ref handy but I really want to checkout the latest detached commit in that stack.

But with revset support added, switch - broke because it looked a poorly formed revset:

Message:  A fatal error occurred:
   0: parse error in "-": parse error: Unrecognized token `-` found at 0:1
      Expected one of "(", "..", ":", "::", a commit/branch/tag or a string literal
   1: parse error: Unrecognized token `-` found at 0:1
      Expected one of "(", "..", ":", "::", a commit/branch/tag or a string literal

The last commit in this stack adds an explicit check for - so that this support is restored.

Apparently, revparse_single() in libgit2 doesn't support this. ~~I didn't patch - support into repo.revparse_single_commit() but I'm thinking as I write this that perhaps that may be better? There is already a special case in that function to work around another libgit2 issue, and I suppose we could detect if the passed spec is - and then pass @{-1} to revparse_single() instead. 🤔 ~~ edit I made this change and it seems to be working.

@claytonrcarter claytonrcarter force-pushed the switch-revset branch 3 times, most recently from 5cc1b7a to df58ccf Compare December 10, 2024 16:11
Copy link
Owner

@arxanas arxanas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Both of these issues have bothered me personally as well.

@claytonrcarter claytonrcarter force-pushed the switch-revset branch 2 times, most recently from 45bc173 to 85d3df3 Compare January 24, 2025 03:28
@claytonrcarter
Copy link
Collaborator Author

Thanks! Both of these issues have bothered me personally as well.

Thanks for the review! I've updated my change, and I'll park this here for a few days while I continue using it locally in case I missed anything.

@claytonrcarter claytonrcarter merged commit f3c1861 into arxanas:master Feb 24, 2025
13 checks passed
@claytonrcarter claytonrcarter deleted the switch-revset branch February 24, 2025 01:14
@claytonrcarter
Copy link
Collaborator Author

This has been working fine for me. Let's see what everyone else thinks of it. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants