-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Comments
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. |
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 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 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? |
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.
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.
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. |
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 |
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. |
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. |
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 |
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. |
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. |
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.
The text was updated successfully, but these errors were encountered: