Skip to content

Ability to fixup into the above commit #4514

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

Open
jesseduffield opened this issue Apr 26, 2025 · 9 comments · May be fixed by #4526
Open

Ability to fixup into the above commit #4514

jesseduffield opened this issue Apr 26, 2025 · 9 comments · May be fixed by #4526
Labels
enhancement New feature or request

Comments

@jesseduffield
Copy link
Owner

jesseduffield commented Apr 26, 2025

Is your feature request related to a problem? Please describe.
Right now you're able to fixup a commit down into the commit below. That is, you combine the two commits and retain the message of the lower commit. However, often I find myself wanting to preserve the message of the higher commit.

Describe the solution you'd like
Instead of just showing a confirmation popup when fixup'ing, show a menu asking the user if they want to fixup into the commit below (current behaviour) or fixup into the commit above.

@jesseduffield jesseduffield added the enhancement New feature or request label Apr 26, 2025
@jesseduffield jesseduffield changed the title Ability to squash/fixup up or down Ability to fixup into the above commit Apr 26, 2025
@jesseduffield
Copy link
Owner Author

I previously included 'squash' in the description but on reflection I actually never use the squash feature and I don't think there's a real use case there given that you merge the two messages anyway.

@stefanhaller
Copy link
Collaborator

often I find myself wanting to preserve the message of the higher commit.

I'd be curious why this is. Can you describe a workflow that makes this necessary? This is not skepticism, just curiosity (because I rarely have this need myself).

It should actually be easy to implement using the -C option of the fixup rebase todo.

One thing I'm unsure about is how to do it when you're already in an interactive rebase; pressing 'f' there currently doesn't show a popup, would you add the menu there, too?

Also, in an interactive rebase we currently don't visualize the -C flag at all, maybe we should do that somehow, so that people can tell what's going to happen.

And finally, "or fixup into the commit above": this sounds like you would select the lower of the two commits that you want to squash and press 'f' on it, is that right? I would find this confusing; I'd still select the first commit (higher up in the list), and ask whether they want to keep the commit message of the first or the second commit. This way it matches better what they see if they ever stop in the interactive rebase for some reason.

And finally, what about range selection? I suppose we would keep the message of the top-most commit in that case?

@jesseduffield
Copy link
Owner Author

I'd be curious why this is. Can you describe a workflow that makes this necessary? This is not skepticism, just curiosity (because I rarely have this need myself).

Good question. A general pattern does not come to mind. I might need to just wait until I have the use case again to see what specific situation it is.

One thing I'm unsure about is how to do it when you're already in an interactive rebase; pressing 'f' there currently doesn't show a popup, would you add the menu there, too?

Hmm. I do like the fact that currently that's just one keypress and not two. Not sure what's better: being consistent or just supporting the use case outside a rebase.

And finally, "or fixup into the commit above": this sounds like you would select the lower of the two commits that you want to squash and press 'f' on it, is that right? I would find this confusing; I'd still select the first commit (higher up in the list), and ask whether they want to keep the commit message of the first or the second commit. This way it matches better what they see if they ever stop in the interactive rebase for some reason.

Initially I was thinking that for any commit range you select, if you choose to fixup the below commit then it's the commit under that range, otherwise it's the commit above the range. But it now occurs to me that the choice of which commit you want to fixup into, and which commit message you want to retain, are two separate things.

Thinking out loud, maybe this would be easier if it was just really easy to copy+paste commit messages into other commits. I could just copy commit A's message, fixup A into B, then paste A's message into B (overwriting it). This assumes I don't care about situations when the authors of the two commits differ, and in my own personal use case, I don't care about that.

@stefanhaller
Copy link
Collaborator

I could just copy commit A's message, fixup A into B, then paste A's message into B (overwriting it).

This feels like a somewhat clunky workaround to me, given that git already has the ability to fixup A into B while keeping A's message. We only need to find a good UI for this.

I think we can make this all work, I just find it import to keep the concept of fixups always going downwards. Which means that if you want to fixup B into A, you still need to select A and press 'f' on it. You then get the menu asking which commit message you want to keep, so all is well. For a range selection we can hopefully make it clear through a tool tip which exact message is going to be kept.

I also think it would be really nice if we could somehow visualize the state of a fixup todo, but as long as we are not offering the feature in an interactive rebase, users will never see it (because you can't get conflicts), so it's not so important. The only other case where we use the -C flag currently is when auto-squashing amend! commits, but in this case it's clear enough from the subject what's going to happen.

@jesseduffield
Copy link
Owner Author

I think we can make this all work, I just find it import to keep the concept of fixups always going downwards. Which means that if you want to fixup B into A, you still need to select A and press 'f' on it. You then get the menu asking which commit message you want to keep, so all is well. For a range selection we can hopefully make it clear through a tool tip which exact message is going to be kept.

I agree in principal, but I want to test things out for a bit to see how often I want a range to be fixup'd upwards, cos if so, that will be awkward with a downward-fixup-only approach.

@stefanhaller
Copy link
Collaborator

This would still work easily, you just need to adapt your thinking about it a little bit. If you have commits A, B, and C, and you want to squash B and C upwards into A, you'll select A and B and hit 'f', and then pick the option to keep A's message.

@jesseduffield
Copy link
Owner Author

I see, but does that mean we need to provide one menu option for each of A, B, and C, when it comes to picking the message? If so I'm okay with that

@stefanhaller
Copy link
Collaborator

Hm, good question. I would have said it's enough to provide one for A's message. If you want another one in the middle of the range, you can easily do two separate commands.

I also think that the bottom-most message should come first in the menu, so that 'f enter' does the same as before.

@stefanhaller
Copy link
Collaborator

One thing I'm unsure about is how to do it when you're already in an interactive rebase; pressing 'f' there currently doesn't show a popup, would you add the menu there, too?

Hmm. I do like the fact that currently that's just one keypress and not two. Not sure what's better: being consistent or just supporting the use case outside a rebase.

I thought about this some more, and I think I find it important to offer the menu in an interactive rebase too. Very often, I need to reorder commits before I squash them, and in that case I'll start a rebase first because reordering is so much faster then. It would be annoying to then have to exit the rebase first before you can squash your commits upwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants