forked from jesseduffield/lazygit
-
Notifications
You must be signed in to change notification settings - Fork 0
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
[pull] master from jesseduffield:master #279
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: thirdkeyword <fliterdashen@gmail.com>
- **PR Description** Resolves #3421 The error is "Expected exactly one original SHA, found 0" but the merge commit hash (`d6a7a04c626e40071133de26ebe8fdd225caa5c0`) is present in the rebase TODO file.   However, the commit is missed during search because the filter is only looking for pick commits: https://github.com/jesseduffield/lazygit/blob/580818e935e19a67f7fe1bbb148224a95781879c/pkg/utils/rebase_todo.go#L238 Checking for merge commits as well fixes the issue. I believe only pick and merge should be valid here. If already in an interactive rebase, lazygit only allows amending to the current HEAD commit. When that happens, this whole interactive rebase logic is bypassed and lazygit just performs `git commit --amend`: https://github.com/jesseduffield/lazygit/blob/580818e935e19a67f7fe1bbb148224a95781879c/pkg/gui/controllers/local_commits_controller.go#L668 This is the reason why amending to a HEAD merge commit currently works whereas non-HEAD does not.
It was added in 2018 (700f8c7), but I don't know for what purpose. It just took me 15 minutes to figure out why my new file todo.go wasn't added, so I'm removing this entry as I find it more harmful than helpful.
… fork Sometimes it takes a while to get PRs accepted upstream, and this blocks our progress. Since I'm pretty much the only one making changes there anyway, it makes sense to point to my fork directly.
…copied branches These tests succeed here, but have comments explaining which bits are undesired. See next commit for a more detailed explanation why.
The rebase.updateRefs feature of git is very useful to rebase a stack of branches and keep everything nicely stacked; however, it is usually in the way when you make a copy of a branch and want to rebase it "away" from the original branch in some way or other. For example, the original branch might sit on main, and you want to rebase the copy onto devel to see if things still compile there. Or you want to do some heavy history rewriting experiments on the copy, but keep the original branch in case the experiments fail. Or you want to split a branch in two because it contains two unrelated sets of changes; so you make a copy, and drop half of the commits from the copy, then check out the original branch and drop the other half of the commits from it. In all these cases, git's updateRefs feature insists on moving the original branch along with the copy in the first rebase that you make on the copy. I think this is a bug in git, it should create update-ref todos only for branches that point into the middle of your branch (because only then do they form a stack), not when they point at the head (because then it's a copy). I had a long discussion about this on the git mailing list [1], but people either don't agree or don't care enough. So we fix this on our side: whenever we start a rebase for whatever reason, be it interactive, non-interactive, or behind-the-scenes, we drop any update-ref todos that are at the very top of the todo list, which fixes all the above-mentioned scenarios nicely. I will admit that there's one scenario where git's behavior is the desired one, and the fix in this PR makes it worse: when you create a new branch off of an existing one, with the intention of creating a stack of branches, but before you make the first commit on the new branch you realize some problem with the first branch (e.g. a commit that needs to be reworded or dropped). It this case you do want both branches to be affected by the change. In my experience this scenario is much rarer than the other ones that I described above, and it's also much easier to recover from: just check out the other branch again and hard-reset it to the rebased one. [1] https://public-inbox.org/git/354f9fed-567f-42c8-9da9-148a5e223022@haller-berlin.de/
- **PR Description** The rebase.updateRefs feature of git is very useful to rebase a stack of branches and keep everything nicely stacked; however, it is usually in the way when you make a copy of a branch and want to rebase it "away" from the original branch in some way or other. For example, the original branch might sit on main, and you want to rebase the copy onto devel to see if things still compile there. Or you want to do some heavy history rewriting experiments on the copy, but keep the original branch in case the experiments fail. Or you want to split a branch in two because it contains two unrelated sets of changes; so you make a copy, and drop half of the commits from the copy, then check out the original branch and drop the other half of the commits from it. In all these cases, git's updateRefs feature insists on moving the original branch along with the copy in the first rebase that you make on the copy. I think this is a bug in git, it should create update-ref todos only for branches that point into the middle of your branch (because only then do they form a stack), not when they point at the head (because then it's a copy). I had a long discussion about this on the git mailing list [1], but people either don't agree or don't care enough. So we fix this on our side: whenever we start a rebase for whatever reason, be it interactive, non-interactive, or behind-the-scenes, we drop any update-ref todos that are at the very top of the todo list, which fixes all the above-mentioned scenarios nicely. I will admit that there's one scenario where git's behavior is the desired one, and the fix in this PR makes it worse: when you create a new branch off of an existing one, with the intention of creating a stack of branches, but before you make the first commit on the new branch you realize some problem with the first branch (e.g. a commit that needs to be reworded or dropped). It this case you do want both branches to be affected by the change. In my experience this scenario is much rarer than the other ones that I described above, and it's also much easier to recover from: just check out the other branch again and hard-reset it to the rebased one. [1] https://public-inbox.org/git/354f9fed-567f-42c8-9da9-148a5e223022@haller-berlin.de/
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )