-
-
Notifications
You must be signed in to change notification settings - Fork 593
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
Checkout commit #1499
Checkout commit #1499
Conversation
repo.set_head_detached(commit_hash.into())?; | ||
|
||
if let Err(e) = repo.checkout_head(Some( | ||
git2::build::CheckoutBuilder::new().force(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will overwrite changes made to the index, right? I am not sure we want force
to be true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested, it in fact will fail with an Err
if there are uncommitted changes. se more comments below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small improvement on the error handling and this is good to go 💪
repo.set_head_detached(commit_hash.into())?; | ||
|
||
if let Err(e) = repo.checkout_head(Some( | ||
git2::build::CheckoutBuilder::new().force(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested, it in fact will fail with an Err
if there are uncommitted changes. se more comments below
Thank you for your comments! I've fixed the code, let me know if more stuff needs fixing. 💪 |
Thank you @fralcow |
* Add keybind to checkout commit in log view * Extract commit checkout into method * add quckbar hint for checkout commit * add a smoke test * update changelog * show an error in popup --------- Co-authored-by: Omnikar <[email protected]> Co-authored-by: extrawurst <[email protected]>
This Pull Request fixes/closes #983
It changes the following:
I followed the checklist:
make check
without errors -- sync::submodules::tests::test_smoke throws an error for me, both in master and my branch:Error { code: -1, klass: 16, message: "the server did not provide a certificate" }
. I would appreciate some help fixing this.The PR by the original author went stale, I hope we can add this function in this PR.