Clear preserved commit message when entering CommitEditorPanel #4558
Conversation
Looks good to me.
I'd be in favor of doing that. Maybe
I don't think that's a bug, but this call could be removed, because it does nothing (since we are opening the tags panel with preserveMessage=false, see below). Could be another, separate cleanup commit.
Yes. I think the idea is that creating tags doesn't take part in the remember-my-partial-commit-message business. I'm not sure how relevant this really is in practice, but something like:
|
Stole your name because naming is hard 😆
Yeah, I misused the word "bug" there, I just meant "This is weird code". I have removed that call
I have done some research and I believe this is now never relevant in practice. So I added a commit removing the check. With previous commits like 41a7afb we have been whittling down the usages of OnCommitSuccess, and at this point there are 3 call sites:
which tracking their upstream leads to:
^ This is the method we are adding this clearing to in this PR, and I'm thinking it should clear the preserved message regardless of the previous state of (But now that I've put the whole paragraph into words, I'm actually less convinced than when it was an idea floating around in my head. I'm open to reverting this last commit if people want) |
Misunderstanding (I think). I was talking about the relevance of the described behavior (i.e. that tag creation doesn't interfere with aborting commits and preserving their message), not about the relevance of some code. This is just a UX decision, you can't answer it by looking at code or at the commit history. Personally I think the behavior is good the way it is. You might argue that starting to create an annotated tag, typing a message, canceling out, and starting over should also preserve the message for that, but I have never heard requests for this, and if we wanted it, we would probably have to preserve two different messages, one for commits and one for tags. But I'm not proposing we do that.
Ah, I wasn't aware of this scenario. I totally agree that this doesn't make sense, and that unconditionally clearing the message is better. So the end state of the branch looks good to me now, but I would personally have chosen a different order of the commits to make things a bit clearer:
I'll leave it up to you if you want to change it to this. |
I like the sound of that history, but the renames make it marginally too annoying for me to want to move the commits around. |
This is no change in behavior because OnCommitSuccess only clears the message when the commit message panel was opened with preserveMessage=true, which it isn't in the case of creating a tag. And this is in fact the desired behavior, because we don't want creating a tag to interfere with preserving commit messages in any way.
Previously it would only clear the message if the panel had been opened with preserveMessage=true; we don't need this check, because callers know how they opened the panel, and whether they want to clear the message.
In one case it was actually called *before* making a commit (when switching from the commit message panel to committing in the editor). And clearing the preserved message is the only thing it does, so name it after what it does rather than when it's called.
|
Hm, that literally took 5 minutes. I pushed |
|
Hm, I'm curious what your flow is that working through rebase conflicts isn't that annoying. On my first thought, I was going to move the commits around with a set of individual I then tried it again using Reset to your branch, and pushed |
ChrisMcD1
force-pushed
the
4557-preserved-commit-message
branch
from
2988df7 to
3bd8a92
Compare
I didn't rebase at all, I just recreated the commits from scratch. The changes were so small that this was way faster than resolving conflicts. 😄
Yes, I recommend that in general. I use |
Fixes the problem described in #4557:
I often find myself initiating a commit message with
c, but then halfway through typing the message, realize that is is likely to be more substantial than I thought. I press<esc>and thenCto launch my editor where I can edit a larger commit body more effectively. I finish my commit message, close my editor, return to LazyGit and all is well.Unfortunately, the next time I press
c, creating a totally new commit, my partially completed message is still present. It no longer is relevant, so I have to delete the few words I typed before.go generate ./...)