Re: [PATCH] Teach 'rebase -i' the command "amend"

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Oct 5, 2009, 21:26 UTC
Message-ID: <alpine.DEB.1.00.0910052318230.4985@pacific.mpi-cbg.de>
In-Reply-To: <7vbpkl8s8x.fsf@alter.siamese.dyndns.org>

Hi,

On Mon, 5 Oct 2009, Junio C Hamano wrote:

Show 34 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Being in an editor but still not able to fix typos is a nuisance.
> >
> > NAK.
> >
> > Supporting that would be totally out of line with the way rebase -i is 
> > supposed to work.
> 
> If the rebase insn sheet were richer, and had a way to show the full
> message, like this:
> 
> pick 4973aa2 git-pull: dead code removal
>     Back when a74b170 (git-pull: disallow implicit merging to detached HEAD,
>     2007-01-15) added this check, $? referred to the error status of reading
>     HEAD as a symbolic-ref; but cd67e4d (Teach 'git pull' about --rebase,
>     2007-11-28) moved the command away from where the check is, and nobody
>     noticed the breakage.  Ever since, $? has always been 0 (tr at the end of
>     the pipe to find merge_head never fails) and other case arms were never
>     reached.
>     
>     These days, error_on_no_merge_candidates function is prepared to handle a
>     detached HEAD case, which was what the code this patch removes used to
>     handle.
>     
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
> 
> I do not see why we shouldn't allow people to edit any part of the above
> to reword.
> 
> I would even understand (but not necessarily agree) if somebody wants to
> give the patch text and let users edit to reapply.
> 
> So I do not agree with your "totally out of line" at all.

Are you serious? "rebase -i" was _always_ about showing an edit script, i.e. to tell Git _what_ you want to do with _which_ commits, identified by short commit names.

The oneline was _always_ meant as a pure convenience for the user.

This paradigm has been true even to back when "rebase -i" was still called "edit-patch-series", and that is the reason I claimed that it is totally out of line. Because it is.

Show 8 quoted lines

> > Besides, if you already have typos in the commit subject, you _better_ 
> > check the whole commit message, so: double NAK.
> 
> That sounds a bit too dogmatic.
> 
> But I tend to agree with you that we would be better off not accepting
> such a "retitle" patch, as it strongly encourages single-liner commit log
> messages.

Now, that is at least as dogmatic as what I proposed.

> Oh, there was no patch?  Then nevermind...

Sorry, but I changed my mind on this attitude. Earlier, you would find me valuing arguments only when backed up by code. But since the GitTogether in Berlin I know of at least one discussion where somebody simply had not enough time to back up a (submodule-related) argument, the validity of the argument notwithstanding.

So not always is a "no patch? Nevermind" the correct thing to do.

In this particular case, however, I agree. Just touching up the first line of commit messages is such a special case, and so removed from what a "rebase" is about that I would claim that this case would be better handled by a really trivial shell script that launches an editor and then drives filter-branch based on what the user said.

Ciao, Dscho