martinvonz / jj

A Git-compatible VCS that is both simple and powerful
https://martinvonz.github.io/jj/
Apache License 2.0
9.27k stars 321 forks source link

FR: empty commit message should be a no-op #4414

Open leeavital opened 2 months ago

leeavital commented 2 months ago

Is your feature request related to a problem? Please describe.

Sometimes I type jj commit when I mean jj squash or meant jj commit -i. It would be great to have a way to back out of this.

In git, I'd get out of this by:

Describe the solution you'd like

If I jj commit and the commit message is empty, I should not create a new commit

Describe alternatives you've considered

None, I'm not sure if there are legitimate use cases for creating commits without a description

Additional context Add any other context or screenshots about the feature request here.

ilyagr commented 2 months ago

One workaround is to do something like :cq! in Vim to get the editor to exit with an error code. Another workaround, of course, is jj undo.

I think jj commit fell through the cracks, in most cases leaving the text as is cancels the operation.

For some operations, like jj describe, leaving the commit message empty is valid in jj. Strictly speaking, that makes sense for jj commit too, which is probably it works this way now. However, we could consider this sufficiently against the spirit of what jj commit is for and say that making it easier to cancel jj commit is more important, and thus an empty message should abort as you suggested.

Another option I am wondering about is to provide some directive that generically cancels operations, e.g. abort the operation if the first line of the file is "JJ STOP" (and document it in the commit message template).

yuja commented 2 months ago

While I agree that jj commit with no description can be an error, it's unclear for jj split or jj squash. So I prefer a general solution like JJ STOP. A truly empty content (without JJ: lines) could be interpreted an interrupt signal if that helps.

ilyagr commented 2 months ago

I think you're right.

IMO, jj split and (somewhat less importantly) jj squash should certainly allow an empty description (as opposed to aborting when they see it). Rephrasing a part of Yuya's point, this means that making the exception for jj commit wouldn't solve the same problem for jj split or jj split.

martinvonz commented 2 months ago

I agree.

A truly empty content (without JJ: lines) could be interpreted an interrupt signal if that helps.

But this might be a good idea.

ilyagr commented 2 months ago

A truly empty content (without JJ: lines) could be interpreted an interrupt signal if that helps.

But this might be a good idea.

This could result in surprising behavior and rare bugs if the commit message is generated by a script or another tool.[^human] (Unless we go into a direction where description messages have syntax, like the multi-commit descriptions do, where an empty file is an obvious syntax error)

[^human]: In principle, I could also imagine a human deleting everything from a file before writing the new description (if they treat it like writing a novel that requires no distraction :) ). It would also affect such a theoretical human.

So, I'm leaning more towards "JJ STOP". Fortunately, jj undo exists, so if a user thinks that deleting everything cancels and is surprised when it doesn't, they can fix it.

essiene commented 2 months ago

Footnotes

  1. In principle, I could also imagine a human deleting everything from a file before writing the new description (if they treat it like writing a novel that requires no distraction :) ). It would also affect such a theoretical human.

I do this a lot of the time, but I think it's fine if it doesn't create a commit, since the wc is save anyway. I don't loose anything, since I didn't type anything.

So FWIW, +1 for treating a completely empty file as a signal to not create a new commit (but not for squash or split)

arxanas commented 2 months ago
  1. In principle, I could also imagine a human deleting everything from a file before writing the new description (if they treat it like writing a novel that requires no distraction :) ). It would also affect such a theoretical human.

FWIW I almost always do this. JJ: is particularly annoying because it looks like a metadata field for Phabricator (maybe also Gerrit?). But I do the same for # in Git and HG: (?) for Mercurial because they're distracting to read.

If I want to cancel, I use :cq in Vim. However, it's worth noting in this thread that some editors (particularly code --wait on the command-line) don't offer a way to exit with a non-zero exit code.

I think that the overall workflow of being able to cancel a commit message editor should be addressed, but I'm skeptical that implementing an in-band solution like special interpretations of the file contents is a maintainable solution in the long run. It seems like a decent amount of complexity for a fairly small gain.

The general UI principle I follow is that it's better to support undoable operations than comfirmable/cancellable operations. jj already offers jj undo to address this. (Many git-branchless operations print "use git undo" as the only contextual advice to cancel/undo the change.)

My proposed solution would be to detect the empty file contents and surface a hint saying something like "the entire file contents were deleted, which resulted in an empty commit message; if this was not intentional, run: jj undo".