jesseduffield / lazygit

simple terminal UI for git commands
MIT License
50.3k stars 1.78k forks source link

Remember commit message for current branch when closing / re-opening Lazygit #2720

Open lougreenwood opened 1 year ago

lougreenwood commented 1 year ago

Is your feature request related to a problem? Please describe. I use Lazygit in nvim using toggleterm.

Often I try to commit and I get a pre-commit hook error from some linter, so I need to close the lazygit float term, fix the error and re-open it. When I re-open lazy got my old commit message is gone, so I need to re-type it again.

Describe the solution you'd like If Lazygit could remember the commit message for the current branch until that commit is successful it would improve my workflow. The MacOS Git GUI app, Fork has a similar feature which saves the commit message, this is where my inspiration came from.

I would guess that Lazygit could forget the commit message if all changes are discarded.

Describe alternatives you've considered setting up nvim-remote so I don't need to close the toggle term, but this seems like a work around for this solution, I imagine that my use case isn't the only one which would benefit from lazygit remembering the commit message.

mark2185 commented 1 year ago

Does toggleterm have to close the running program? Can't it just hide it and show it again?

Also, does that happen in regular usage of lazygit, i.e. outside of nvim? I'm not sure if an error because of pre-commit hooks clears the commit message, and I'm too lazy to set it up.

Having lazygit remember a commit message across restarts isn't really needed nor wanted.

stefanhaller commented 1 year ago

I'm not sure if an error because of pre-commit hooks clears the commit message,

Yes it does, see https://github.com/jesseduffield/lazygit/issues/2679. Pretty annoying, actually.

dagadbm commented 1 year ago

I was just about to suggest this actually.

I have some precommit hooks for eslint and sometimes I forget dangling code which is caught on commit. This closes lazygit and effectively I end up loosing the commit message and I need to retype it again.

It would be nice if we could press up arrow and it would give you the last commit message you wrote (no matter if the commit was succesfull or not)

robclancy commented 1 year ago

The commit description being cleared is extremely frustrating to the point I won't use this to commit anymore incase I close my draft to check something.

jesseduffield commented 1 year ago

We could store the commit message in the state.yml file. This is the same place that we store the list of recent repos. I bet this would be easy to implement. We'd add a field for commit messages here like so:

// AppState stores data between runs of the app like when the last update check
// was performed and which other repos have been checked out
type AppState struct {
    LastUpdateCheck     int64
    RecentRepos         []string
    StartupPopupVersion int

    // these are for custom commands typed in directly, not for custom commands in the lazygit config
    CustomCommandsHistory []string
    HideCommandLog        bool
    // Map from the path of the worktree to the state of the worktree
    Worktrees             map[string]WorktreeState
}

type WorktreeState struct {
    // Draft commit messages exist against worktrees, not repos, because in one repo
    // there may be multiple worktrees, and each worktree may have a different draft
    // commit message
    DraftCommitMessage string
}

Then in https://github.com/jesseduffield/lazygit/blob/57bb1aa69853de458faf886f077dfca9250dc28d/pkg/gui/controllers/helpers/commits_helper.go#L114, rather than call GetPreservedMessage() and SetPreservedMessage() on the commit message context, we would have the helper itself define those methods and have them read/write to that state file e.g. like:

worktreeState, ok := self.c.GetAppState().Worktrees[self.c.WorktreePath()]
if !ok {
  worktreeState = config.WorktreeState{}
  self.c.GetAppState().Worktrees[self.c.WorktreePath()] = worktreeState
}
worktreeState.DraftCommitMessage = draftCommitMessage
err := self.c.SaveAppState()

There are a few things we'd need to do like have a method for getting the current worktree path (my worktrees PR isn't in yet). But otherwise it sounds fairly straightforward.

Something worth thinking about, though is whether we want to introduce a pattern for per-repo (or worktree) state/config that lives inside the repo (i.e. inside the .git folder). This would prevent issues when repos are moved, because there are no hard-coded paths involved. I think this is worth thinking about

robclancy commented 1 year ago

I did a long commit message the other day knowing to not close it because it decides I want to type it out twice. But little did I know that scrolling the diff behind the box will also close it and goodbye commit message.

Just store it where it stores the smaller box message? The big box with the description label is the one that goes away.

jesseduffield commented 1 year ago

@robclancy it should be storing both the summary and the description. Are you saying that it currently only stores the summary? If so could you raise a separate issue for that?

robclancy commented 1 year ago

Yes, and I realize this is probably the wrong issue. The summary is always saved fine and the description is always lost.

EDIT: Works for me now. I updated today (on arch, dunno if the update is old or recent but I did a system update at the end of last week as well) and it keeps the description as well now. (these comments can prob be marked as off topic)

jesseduffield commented 1 year ago

Good to know!

divaltor commented 4 months ago

What's the status of issue?

elvinmahmudov commented 1 month ago

What is the status of this ? It has been more than a year and still no updates? @jesseduffield

jesseduffield commented 1 month ago

The status is that it's still a great idea, we should totally do it, it's totally doable, but it hasn't been a priority.

In response to some comments I made earlier:

There are a few things we'd need to do like have a method for getting the current worktree path (my worktrees https://github.com/jesseduffield/lazygit/pull/2147 isn't in yet). But otherwise it sounds fairly straightforward.

This is now implemented.

Something worth thinking about, though is whether we want to introduce a pattern for per-repo (or worktree) state/config that lives inside the repo (i.e. inside the .git folder). This would prevent issues when repos are moved, because there are no hard-coded paths involved. I think this is worth thinking about

Just storing the state in the global state file seems fine to me. Yes, if you move the repo, you'll lose your mid-way commit message, but that is a very rare edge case, and we do per-repo state files as a second step if we want.

If anybody wants to take a stab at this, I'm happy to provide guidance. (tagging @karimkhaleel @AzraelSec @jwhitley in case one of you was looking for a new issue!).

If you yourself would like to work on this @elvinmahmudov let me know.

AzraelSec commented 1 month ago

I was looking for something to work on - I'd be up to give this a shot if it's fine for you @jesseduffield

stefanhaller commented 1 month ago

Just storing the state in the global state file seems fine to me.

Another option could be to store the message in .git/COMMIT_EDITMSG. This is the file that git itself uses to store the message, so it would give us a bit of interoperability with git cli; for example, you could start typing the message in lazygit, then cancel, and type shift-C to proceed in the editor, and your message would show up there (I think). Or vice versa.

The only complication is that we'd have to strip comments from the file (lines beginning with #) when reading from it, in case the commit was started in git.

jesseduffield commented 1 month ago

@stefanhaller Ah yes, I like that a lot

@AzraelSec sounds good! I've assigned to you