Have "Find base commit for fixup" ignore fixup commits for the found base commit

[stefanhaller]

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.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for 0339b411	✅ 100.00%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (0339b41)	Report Missing	Report Missing	Report Missing
Head commit (268d193)	59474	51803	87.10%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#5210)	81	81	100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Footnotes

1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

[mricherzhagen]

I obviously think this is a great addition to lazygit! 👍

Feels like it is missing tests for the fixup! fixup! ... and fixup! another commit cases, though.

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.

[stefanhaller]

Feels like it is missing tests for the fixup! fixup! ... and fixup! another commit cases, though.

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.

in that case the other fixup commit is probably wrong

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.

[mricherzhagen]

Feels like it is missing tests for the fixup! fixup! ... and fixup! another commit cases, though.

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.

Yea, I missed an integration test. But you are right, with those unit tests that should be well covered.

in that case the other fixup commit is probably wrong

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.

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.