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

4 messages, 4 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:29

Johannes Schindelin [off-list ref] writes:
quoted
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 [off-list ref]

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.
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.

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

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

From: Sverre Rabbelier <hidden>
Date: 2016-06-15 22:47:29

Heya,

2009/10/5 Junio C Hamano [off-list ref]:
If the rebase insn sheet were richer, and had a way to show the full
message, like this:
But that's not what rebase -i's insn sheet is about is it?  It's not
"rewrite my commits so that they are the way I want them", it's about
"change the order of my commits"/squash some/drop some. The polishing
of the commits itself is done after finishing the insn sheet.
I do not see why we shouldn't allow people to edit any part of the above
to reword.
Because it then becomes very hard to do any actual reordering. I'm not
saying it's a bad idea, just that it's a bad idea to do it in 'git
rebase -i', conceptually. Although what you suggest would be useful to
me, I just think it should be a different command, git rewrite perhaps
:P. Definitely not "git rebase -i --rewrite-message" though, becaue it
is not at all about rebasing anymore.

-- 
Cheers,

Sverre Rabbelier

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

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:47:29

Hi,

On Mon, 5 Oct 2009, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
quoted
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 [off-list ref]

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.
quoted
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

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

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:47:29

Johannes Schindelin schrieb:
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.
Good that you mention it: The one-liner is really convenient. I tend to
replace it by reminders like "====== fix unused var", "===== edit msg",
etc. (after marking the commit "edit") and I'm glad that rebase-i later
prints the one-liner from the insn file rather than the original subject
line before it stops.

(I do it this way because on Windows I can't afford to call rebase-i on a
long patch series for every single task. Rather, I plan the tasks while
the insn editor is open, and this way I keep reminders about what the plan
was.)

No, I definitely don't want the one-liners to end up in the commit
message. ;-)

-- Hannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help