Have "Find base commit for fixup" ignore fixup commits for the found base commit #5210
Conversation
Currently we get an annoying error message, but we'd like the base commit to be selected, disregarding the fixup.
There's no reason for this to be a method.
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesFootnotes
|
|
I obviously think this is a great addition to lazygit! 👍 Feels like it is missing tests for the One of the reasons I left the fixups for another commit only as a warning was, that in that case the other fixup commit is probably wrong and has to be changed (after creating the fixup for the current changeset). Most likely the other fixup is either accidentally for the wrong commit or I created it manually and made a typo. That's why I thought it was reasonable to just plainly ignore all other fixup commits in #5210 or only show a warning when they are found. But this change will already be a huge improvement, even if it doesn't handle other (possibly wrong) fixup commits. |
They are here and below, I wonder what you think is missing there? I don't have these as separate integration tests if that's what you meant, but that's intentional.
That's ridiculous. This is of course one possibility, but it's an unlikely one, and I'm not going to base the functionality of the command on the assumption that the existing fixups are wrong. Finding several fixups for unrelated base commits is a very common case, and it always means that your changes need to be separated into individual fixups for these, so erroring out is the appropriate behavior. |
Yea, I missed an integration test. But you are right, with those unit tests that should be well covered.
Ah. You are right. I was thinking about an individual hunk, where this is more unlikely and also had the misconception, that it would find the other base commit as a candidate as well. But the search for the base commit is not "deep", so it stops at the first fixup and will not always also find the other base commit as a fixup candidate. |
If the ctrl-f function (Find base commit for fixup) finds a base commit plus a bunch of fixup! commits for that base commit, it ignores those fixups and selects the base commit. For the purpose of creating another fixup commit on top this is always what you want, so I'm doing this without adding a confirmation. If the user presses shift-A after that to amend their changes though, they are guaranteed to get conflicts, so for that case a warning might be useful; however, I find it unlikely that users will want to amend changes to a commit that they already created fixups for, so I'm just hoping that this won't happen in practice.
This implementation is a bit stricter than #5201, in that it only ignores fixups if the first found commit is not a fixup itself; because if it is, it's not clear whether the user wants to create another fixup for both on top, or amend the changes into each one, in which case they need to be staged individually.