Re: [PATCH 0/1] sequencer: comment out the 'squash!' line

From: Jeff King <peff@peff.net>
Date: Jan 7, 2020, 11:15 UTC
Message-ID: <20200107111556.GC1073219@coredump.intra.peff.net>
In-Reply-To: <20200107033639.GH92456@google.com>

On Mon, Jan 06, 2020 at 07:36:39PM -0800, Jonathan Nieder wrote:

Show 18 quoted lines

> Jeff King wrote:
> 
> > But I thought that was the point of "squash" versus "fixup"? One
> > includes the commit message, and the other does not.
> >
> > I do think "commit --squash" is mostly useless for that reason, and I
> > suspect we could do a better job in the documentation about pushing
> > people to "--fixup".
> >
> > But --squash _can_ be useful with other options to populate the commit
> > message (e.g., "--edit", which just pre-populates the subject with the
> > right "squash!" line but lets you otherwise write a normal commit
> > message). If that's the workflow you're using, then I'm sympathetic to
> > auto-removing just a "squash!" line, as it's automated garbage that is
> > only meant as a signal for --autosquash.
> 
> It's a signal for --autosquash and it gives a visual signal to humans
> of where the squashed commit came from.

True, but I think any proposal here would continue to include that text in the human-readable output (I was sloppy to say "auto-remove"; it is really "auto-comment").

Or do you mean that it's useful in the final, squashed commit? I'd argue that a normal subject line might be so, but the "squash!" line doesn't saying anything that's not in the main subject already. It tells you that there _was_ a squash, but isn't erasing that origin kind of the point of a squash?

> --squash already implies --edit, supporting this kind of workflow.

Ah, that makes sense. I don't use it myself, so I did a quick test earlier. But I jumped too quickly to assuming I needed "--edit" (the "--squash" entry in git-commit(1) talks about being able to use "-m", which I read too much into).

Show 23 quoted lines

> If we could turn back time and start over, would we want something
> like the following?
> 
>  1. if someone leaves the squash! message as is, include it as is in
>     the commit message without commenting out
> 
>  2. if someone edits the squash! commit message to include a body
>     describing what is being squashed in, include the squash! line as
>     part of the commented marker
> 
>  3. if someone leaves the (uncommented) squash! message in after being
>     presented with an editor at --autosquash time, reopen the editor
>     with some text verifying they really meant to do that
> 
> It's rare that concatenated commit messages make sense to be used as
> is, especially when trailers (sign-offs, Fixes, etc) are involved.  I
> suspect that (3) is more important than (2) here --- we're using the
> same space in the editor for input and output, and the result is a
> kind of error-prone process of getting the output right.
> 
> Since we can't turn back time, one possibility would be to make tools
> like "git show --check" notice the squash! lines.  Would that be
> useful?

What if (3) issued a warning to stderr insted of re-invoking the editor? Then "git commit --amend" could be used to fix it, with no change in behavior.

-Peff