git-gui icon indicating copy to clipboard operation
git-gui copied to clipboard

Accidental click on "Amend Last Commit" destroys draft of commit message

Open annulen opened this issue 5 years ago • 7 comments

You can say that this is not a serious issue and one just should avoid misclicks, however, with traditional git commit + editor workflow it's much harder to lose typed text irreversibly.

annulen avatar Mar 17 '20 16:03 annulen

Most user-friendly solution would be to keep old commit message in temporary file or just stored in memory in variable, and put it back into editor if user switches back from amend mode.

Less user-friendly but still OK is to show a confirmation dialog if "Amend Last Commit" is clicked and there is a text to be destroyed

annulen avatar Mar 17 '20 16:03 annulen

This is the most serious issue of the entire Git GUI! And I'm sure that it also happens under different circumstances (maybe after a Rescan when some other operations have been done in parallel, but I can't deliberately reproduce it). We need some form of commit message history, even for the scenarios when you screw up the Undo & Redo at a wrong moment. And obviously, even if you ignore power outages & Windows stability, one of the reasons why this is a must have is the frequency of Git GUI (wish.exe) freezing up.

scscgit avatar Jan 19 '21 17:01 scscgit

Some additional information which might be useful to someone trying to implement this (based on some brief code spelunking):

  • While running, it appears a backup of the current text of the commit message UI control is stored in .git/GITGUI_BCK.
  • The above backup file seems to be created via a procedure named backup_commit_buffer: https://github.com/prati0100/git-gui/blob/df4f9e28f64ea97032ec70d9c8894dc87a1b7f9e/git-gui.sh#L4122-L4151
  • The name of the commit message UI control appears to be ui_comm also referred to as $ui_comm.
  • It appears that the current commit message text is also stored in .git/GITGUI_MSG if it is non-empty when the program exits.
  • See also:
    • https://github.com/prati0100/git-gui/blob/df4f9e28f64ea97032ec70d9c8894dc87a1b7f9e/git-gui.sh#L4100-L4120
    • https://github.com/prati0100/git-gui/blob/df4f9e28f64ea97032ec70d9c8894dc87a1b7f9e/git-gui.sh#L3924-L3949
    • https://github.com/prati0100/git-gui/blob/df4f9e28f64ea97032ec70d9c8894dc87a1b7f9e/git-gui.sh#L3475-L3506
    • https://github.com/prati0100/git-gui/blob/df4f9e28f64ea97032ec70d9c8894dc87a1b7f9e/git-gui.sh#L3448-L3466
    • https://github.com/prati0100/git-gui/blob/df4f9e28f64ea97032ec70d9c8894dc87a1b7f9e/git-gui.sh#L2719-L2723
    • https://github.com/prati0100/git-gui/blob/df4f9e28f64ea97032ec70d9c8894dc87a1b7f9e/git-gui.sh#L1474-L1486
    • https://github.com/prati0100/git-gui/blob/df4f9e28f64ea97032ec70d9c8894dc87a1b7f9e/git-gui.sh#L1574-L1594
    • https://github.com/prati0100/git-gui/blob/df4f9e28f64ea97032ec70d9c8894dc87a1b7f9e/git-gui.sh#L2305-L2339
    • (Clears message text.) https://github.com/prati0100/git-gui/blob/df4f9e28f64ea97032ec70d9c8894dc87a1b7f9e/lib/commit.tcl#L69-L73
    • (Clears message text.) https://github.com/prati0100/git-gui/blob/df4f9e28f64ea97032ec70d9c8894dc87a1b7f9e/lib/commit.tcl#L121-L123
  • Presumably we want to add confirmation/auto-save of message text before deleting/clearing message text.
  • In the interim, it might be a sufficient workaround to disable the "amend" etc button when the commit message text area isn't empty?

Hope these are some useful pointers for someone. :)

follower avatar Dec 14 '21 04:12 follower

Presumably we want to add confirmation/auto-save of message text before deleting/clearing message text.

I think it would make more sense to use two separate backup files for "amended" and "new" messages, and clear respective backup when commit is done. Works intuitively and no need to confirm anything.

annulen avatar Dec 14 '21 07:12 annulen

GITGUI_BCK was introduced in 4578c5cb690df98d0d5fbea043114b4b7dec8f57, commit message contains details how GITGUI_BCK and GITGUI_MSG files are used

annulen avatar Dec 21 '21 02:12 annulen

I think it would make more sense to use two separate backup files for "amended" and "new" messages, and clear respective backup when commit is done. Works intuitively and no need to confirm anything.

Actually, it's not that simple, as "amended" message becomes irrelevant after HEAD moves. I see two possible ways to handle this situation:

  1. Don't backup "amended" message at all, so editing in "amend" mode always starts from original message. It's simpler to implement and is easy to understand, though may cause a loss of work if non-trivial edits were done in "amend" mode.
  2. Do backup "amended" message, but save corresponding HEAD hash alongside, drop it if current HEAD is different.

annulen avatar Dec 21 '21 04:12 annulen

git citool --amend doesn't restore message from GITGUI_MSG, so I guess doing #1 would not be a step back in functionality.

annulen avatar Dec 21 '21 20:12 annulen