With stacked branches, create fixup commit at the end of the branch it belongs to

[stefanhaller]

• PR Description

When working with stacked branches, and creating a fixup commit for a commit in one of the lower branches of the stack, the fixup was created at the top of the stack and the user needed to move it down to the right branch manually. This is unnecessary extra work; create it at the end of the right branch automatically.

• Please check if the PR fulfills these requirements

• Cheatsheets are up-to-date (run go generate ./...)
• Code has been formatted (see here)
• Tests have been added/updated (see here for the integration test guide)
• Text is internationalised (see here)
• If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
• Docs have been updated if necessary
• You've read through your own file changes for silly mistakes etc

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for a793f701	✅ 83.67%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (a793f70)	Report Missing	Report Missing	Report Missing
Head commit (b22149d)	50330	43212	85.86%

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 (#3892)	98	82	83.67%

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

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}

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. ↩

[jesseduffield]

This is a good feature. See comments.

One thing I'm confused about: why do we need the changeToFixup part? My understanding is that we could just use a 'pick' in the interactive rebase and it would work fine because the actual squashing down of the commit will occur later.

[jesseduffield]

This variable name suggests that it's the index of the current branch's head, but when I think 'head of current branch' I think 'the commit with an index of 0'. It's moreso talking about the head of the branch that 'owns' the commit. Not sure if there's any established lingo for that, but I'd be happy to use the term 'owner' and rename this to 'headOfOwnerBranchIdx'. At any rate I'd add a comment explaining what we're doing: finding the lowest branch in the stack that contains the commit.

Another term we could use is 'original'.

[stefanhaller]

I agree that "current" is not a good name. I guess I was thinking about the "current selected commit" or something.

Owner is a little bit better (not very much, tbh :) Changed to that in 9553e30.

Another option that I considered was "target branch".

[jesseduffield]

Likewise I'd call this moveFixupCommitToOwnerStackedBranch. Using Current just makes me think about the current branch. Thoughts?

[stefanhaller]

Changed to that in 9553e30. Again, I wonder if moveFixupCommitToTargetBranch is better.

[jesseduffield]

I don't like target branch because the word 'target' seems arbitrary to me; like using 'selected' branch: it implies that something is doing the targeting and that it could have targeted something else. But a commit isn't really targeting the branch: it's owned by the branch.

I agree that 'owner' is still awkward but I think it's clearer than 'target'.

[stefanhaller]

One thing I'm confused about: why do we need the changeToFixup part? My understanding is that we could just use a 'pick' in the interactive rebase and it would work fine because the actual squashing down of the commit will occur later.

Yes exactly, but I'm reusing an existing function that unconditionally changed the 'pick' to 'fixup', because that's what we need to do for the "amend to" command. I had to make that configurable, because we don't want it here.

[jesseduffield]

Yes exactly, but I'm reusing an existing function that unconditionally changed the 'pick' to 'fixup', because that's what we need to do for the "amend to" command. I had to make that configurable, because we don't want it here.

Ah yes, that makes sense.

[jesseduffield]

LGTM, great work