Amending non-HEAD commits?

[claytonrcarter]

While finishing up #304 just now, I found myself repeatedly doing this dance:

# review code, made changes, realize they belong somewhere else in the stack, then ...
❯ git stash
❯ git checkout <correct commit>
❯ git stash pop
❯ git amend
❯ git checkout -

I would have much preferred to just do something like git amend <correct commit>.

Does this seem like something that should be built in to git amend, or would it be best to leave it for scripting/aliasing?

[martinvonz]

While finishing up #304 just now, I found myself repeatedly doing this dance:

# review code, made changes, realize they belong somewhere else in the stack, then ...
❯ git stash
❯ git checkout <correct commit>
❯ git stash pop
❯ git amend
❯ git checkout -

An alternative to the first three commands i git checkout -m <correct commit>. I don't know if that makes the conflict resolution flow trickier (e.g. undoing if you realize there are too large conflicts to deal with at the moment).

I would have much preferred to just do something like git amend <correct commit>.

One option is git commit --fixup <correct commit> followed by git rebase -i --autosquash (there's also a config for --autosquash).

Does this seem like something that should be built in to git amend, or would it be best to leave it for scripting/aliasing?

Despite the alternatives mentioned above, I think it does. Mercurial got this feature very recently (in a separate command -hg fixup - rather than part of hg amend). I also added the feature to jj a while ago. One thing I realized then was that it could be generalized to move changes out of any commit and into any commit. I'm very happy with how that turned out and how amend then just became a special case of moving changes from the working copy into the parent commit. I think you could do the same in git-branchless even though the working copy is not an actual commit for you. I'll leave it to @arxanas and you to decide whether it's a good idea. It'll certainly be more work than just baking it into git amend.

[arxanas]

Does this seem like something that should be built in to git amend, or would it be best to leave it for scripting/aliasing?

I think it's sensible. git-revise actually has this feature, but unfortunately, it doesn't invoke hooks, so it doesn't interoperate well with git-branchless (see mystor/git-revise#106). Also see the discussion here about implementing an absorb command: martinvonz/jj#170

The workflows you're using now are documented at https://github.com/arxanas/git-branchless/wiki/Workflow:-Editing-an-old-commit's-contents#make-additional-child-commits, but it doesn't mention the -m flag to git checkout, or that you can now use git amend.

Personally, I either do:

• git checkout -m <target> commit.
• git commit -m 'temp: some work' && git rebase -i.
  • Sometimes I use fixup directly in git rebase -i;
  • and sometimes, I first commute the commit upward in the stack to make sure it merges cleanly and that tests pass on it, and only then combine it with its parent commit.

Like @martinvonz mentions, running git checkout -m can make the conflict-resolution workflow more complicated. This is also a problem for git amend:

• The patch might not apply cleanly, requiring manual resolution;
• and then the descendant commits also might not apply cleanly.

The main design issue is that, at present, git amend is not "undo-safe". If you have working copy changes and you combine them with the current commit, there is no easy way to split the changes back out again. I've filed #313 to track this issue. Implementing that would also help us with the above points.

I'm happy to accept a change which implements git amend on a target commit; or you could also try to make git-revise invoke hooks, which would let you do the same thing.

One thing I realized then was that it could be generalized to move changes out of any commit and into any commit.

We just implemented machinery to replace one commit with another in the RebasePlanBuilder. If we support the working-copy-as-a-commit and update that functionality to replace one commit with zero or more commits, then I think we'd be pretty close to what you're describing here.