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

`git reword` message file isn't recognized by Vim as being a git commit message

Open elipsitz opened this issue 2 years ago • 10 comments

Description of the bug

Vim (and presumably other editors) have support for recognizing if the file being edited is a git commit message file. Vim handles this with filetype=gitcommit, and applies helpful highlighting and line wrapping. Vim does not recognize that the file opened by git reword is a "gitcommit" file.

Steps to reproduce:

  1. Set your Git editor to vim
  2. git reword
  3. Observe that Vim thinks this is a plain text file.

Possible resolution

Vim recognizes this by looking at the filename (https://github.com/vim/vim/blob/master/runtime/filetype.vim#L708-L709) -- e.g. when doing git commit, it edits .git/COMMIT_EDITMSG, and then Vim sees this and treats it as a gitcommit file.

It doesn't look like there's a great way to handle this with the current setup. dialoguer::Editor doesn't allow specifying a specific temporary file path. It does allow setting a specific extension (https://docs.rs/dialoguer/latest/dialoguer/struct.Editor.html#method.extension), which it passes to tempfile's Builder::suffix (https://docs.rs/tempfile/latest/tempfile/struct.Builder.html#method.suffix). The documentation here says "Path separators are legal but not advisable.", but when I tried in practice to call .extension("/COMMIT_EDITMSG"), it broke (on macOS, at least) because something was unable to find the file that tempfile apparently created.

It looks like the functionality provided by Editor is pretty minimal -- perhaps this could be pulled into the crate, and the path .git/COMMIT_EDITMSG could be used (so it would look exactly the same as regular Git to an editor).

Workaround

You can manually run :setlocal filetype=gitcommit to set the currently opened file as being gitcommit.

Expected behavior

No response

Actual behavior

No response

Version of rustc

No response

Automated bug report

No response

Version of git-branchless

git-branchless 0.3.12

Version of git

git version 2.30.2

elipsitz avatar Jul 27 '22 17:07 elipsitz

Hi @elipsitz, thanks for reporting. cc @claytonrcarter in case they have some thoughts.

This seems pretty reasonable. Technically speaking, git reword might produce multiple commit messages separated with lines like ++ reword abc123 some message, but I don't see any reason why we shouldn't let vim treat it as filetype=gitcommit.

It looks like the functionality provided by Editor is pretty minimal -- perhaps this could be pulled into the crate,

Yeah, I thought Editor might save us some work, but we've had a few customization problems so far, so it seems like it would be better to write it ourselves at this point.

arxanas avatar Aug 04 '22 04:08 arxanas

Yes, I've also wanted something like this. Here's my thoughts, in no particular order:

  1. I just tried editting .git/COMMIT_EDITMSG in Emacs and it too (via magit the mighty) treats it as a commit message w/ formatting, line wrapping, etc.

  2. Editor really isn't doing very much for us here, and I think we can replace the relevant parts of it w/ a couple dozen lines of code, and doing so might make it possible to test this parts of the code that were not possible with Editor. (eg when the contents of the file aren't changed, or commits are added or removed during edit)

  3. ... but Editor uses a temp file, which makes file collisions unlikely. If we start editting .git/COMMIT_EDITMSG for every reword, when we would need to figure out what to do if that file already exists. (Fail and ask the user to finish their other commit/reword or to remove it manually? Clobber it?) And we would need to make sure we clean up after ourselves, which maybe goes w/o saying.

  4. branchless reword already creates a file called .git/REWORD_EDITMSG when it encounters an error, so that the user can recover their work. So we already have some behavior that's adjacent to this, and it would be easy to update this to use .git/COMMIT_EDITMSG

  5. The only other option that comes to mind would be to include some sort of "file local variable" string in the file we create. Emacs supports this via file local variables, and I thought that vim had something similar builtin but I can't find it off hand. The downside to this is that -- if it even works for vim -- we'd probably be stuck including a different "local variable" string for each editor we want to support, and that doesn't sound like any fun.

I guess I'm thinking out loud, but it seems that I've talked myself into this:

  1. Reimplement Editor w/i git-branchless
  2. Use .git/COMMIT_EDITMSG
  3. If that file exists, fail w/ a message stating such, and ask the user to finish any other commits or rewords they have open, or to just remove the file.

claytonrcarter avatar Aug 05 '22 15:08 claytonrcarter

I was just playing w/ this to learn more about how git handles COMMIT_EDITMSG (at least as of git 2.32.0) and it turns that I had totally misunderstood how this file is handled:

  • When creating a commit...
    • If the file doesn't exist, it will be created.
    • If the file exists, it will be overwritten w/o prompting.
  • When a commit is successfully created or amended, the edited file is left in place. (ie it's not deleted upon success.)
  • When the commit message is given on the command line (eg -m 'Commit message here'), the contents of the file are (silently) replaced w/ the commit message given on the CLI.
    • This is the same for creating commits and for amending them.

This muddies the waters a bit, as I think that git reword is significantly more likely to error out than git commit. (We could error while trying to reword public commits, reword merge commits, "commit mismatch" issues w/ the message, etc.) This makes error recovery (ie work or message recovery) more important for git reword than for git commit. This means that we don't want to indiscriminately clobber the file every time (like git does) and risk losing work that users may want to recover, but also gits handling of this file means that the file will almost always exist, so erroring when it does (ie almost every time) would probably be obnoxious.

I'm not set on this (ie I'm 100% open to suggestions), but we may want to consider something like this:

  • When staring git reword
    • If the file doesn't exist, create it
    • If it exists but doesn't contain any ++ reword markers, clobber it
    • If it exists but does contain some ++ reword markers, error out/prompt the user for direction
  • When git reword exits
    • If no errors occurred, delete the file
    • If any error occurred, leave the file in place

Another option might be to copy the existing file into a backup file each time we start a git reword, but then we'd need to be sure to remove old backup files periodically.

claytonrcarter avatar Aug 29 '22 02:08 claytonrcarter

@elipsitz As a workaround, you could also just configure your editor to recognize REWORD_EDITMSG as filetype=gitcommit 😃

arxanas avatar Sep 01 '22 15:09 arxanas

That doesn't actually help with this, right? Looks like that file is only created when there's an issue with git reword, not as the file that git reword opens in the configured editor.

elipsitz avatar Sep 03 '22 00:09 elipsitz

@elipsitz Oops, you're right. But as an alternative to figuring out the rules for clobbering edit messages on disk, we could change git reword so that the temporary file (regardless of where it may be created) always uses the same basename or extension or something, so that it can be detected reliably by editors.

arxanas avatar Sep 09 '22 05:09 arxanas

I forked and vendored the Editor code, so we can make this change now if we want.

arxanas avatar Jan 13 '23 09:01 arxanas

https://github.com/arxanas/git-branchless/pull/791 changes git reword to name the file COMMIT_EDITMSG-*.txt, so you should be able to configure your editor to recognize that as a gitcommit file. It might even be possible to rely on the directory name being unique and then always call the file COMMIT_EDITMSG, which would make Vim work out of the box.

arxanas avatar Feb 11 '23 22:02 arxanas

To configure Vim to use syntax highlighting after that commit, add this to your .vimrc:

autocmd BufNewFile,BufRead COMMIT_EDITMSG-*.txt set ft=gitcommit

elipsitz avatar Feb 14 '23 20:02 elipsitz

Something I missed when I opened #791 is that if multiple commits are being rewritten then git reword will use an extended syntax where the separate commit messages are each introduced with a line like ++ reword <sha>. This doesn't necessarily interact well with standard COMMIT_EDITMSG formats, which tend to highlight text in the second line in red, since the first line in a commit message should usually be followed by a blank line. It might be worth naming multi-commit files differently, especially if the name for single-commit files becomes COMMIT_EDITMSG with no extensions.

wabain avatar Feb 16 '23 18:02 wabain