[PATCH] rebase -i: inform the user they can re-order commits

[PATCH] rebase -i: inform the user they can re-order commits

[Nelson Benitez Leon]

Same as they are informed of what the different commands
do and that they can remove commits.
---
Hi, I was using git rebase -i for some time now and never
occured to me I could reorder the commit lines to affect
the order the commits are applied, learnt that recently 
from a git tutorial, I must admit I have now checked the 
man page and it's well explained, but remarking that on
the user-input screen is an useful addition imo.

 git-rebase--interactive.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 5812222..acf1f5c 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -846,6 +846,8 @@ cat >> "$todo" << EOF
 #  f, fixup = like "squash", but discard this commit's log message
 #  x, exec = run command (the rest of the line) using shell
 #
+# You can re-order the commit lines to affect the order on which
+# the commits will be applied.
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
 #
-- 
1.7.7.6

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Matthieu Moy]

Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:

> +# You can re-order the commit lines to affect the order on which
> +# the commits will be applied.

That's probably the main use of "rebase -i", so it deserves a mention,
yes.

Maybe shorten it to one line like

  You can re-order the lines to change the order of commits in history.

(BTW, is it not 'order in which', not '_on_ which'?)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Nelson Benitez Leon]

On 03/16/2012 11:21 AM, Matthieu Moy wrote:
> Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:
> 
>> +# You can re-order the commit lines to affect the order on which
>> +# the commits will be applied.
> 
> That's probably the main use of "rebase -i", so it deserves a mention,
> yes.
> 
> Maybe shorten it to one line like
> 
>   You can re-order the lines to change the order of commits in history.

I like it but the word 'history' seems a bit too technical for me, and may
scare the normal user,, what about:

You can re-order the lines to change the order the commits are applied.

> (BTW, is it not 'order in which', not '_on_ which'?)
> 

May be, non-native english speaker here..

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Matthieu Moy]

Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:

>>   You can re-order the lines to change the order of commits in history.
>
> I like it but the word 'history' seems a bit too technical for me, and may
> scare the normal user,, what about:
>
> You can re-order the lines to change the order the commits are applied.

Perfect. "are applied" says more than my "in history", since rebase -i
is all about various ways to apply commits.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Jakub Narebski]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:
> 
> > >   You can re-order the lines to change the order of commits in history.
> >
> > I like it but the word 'history' seems a bit too technical for me, and may
> > scare the normal user,, what about:
> >
> > You can re-order the lines to change the order the commits are applied.
> 
> Perfect. "are applied" says more than my "in history", since rebase -i
> is all about various ways to apply commits.

Nitpick: shouldn't it be s/the commits/commits/

    You can re-order the lines to change the order commits are applied.

-- 
Jakub Narebski

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:
>
>> +# You can re-order the commit lines to affect the order on which
>> +# the commits will be applied.
>
> That's probably the main use of "rebase -i", so it deserves a mention,
> yes.

Erm, read in isolation by people like you who _know_ what the command
does, the addition might look sensible, but I find that line of thinking
somewhat disturbing in the first place.

These are meant to be a short _reminder_ for people who _learned_ how the
command they already _know_ is spelled.

If the user does not even know that one of the primary use of the command
is to reorder, isn't he playing with fire?

If we come up with a politely worded message that makes the user realize
that the user does not know what he is doing and gently nudges the user to
read the documentation before continuing to potentially harm himself, that
would be a good addition, but...

So, I dunno.

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Nelson Benitez Leon]

On 03/16/2012 01:17 PM, Junio C Hamano wrote:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> 
>> Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:
>>
>>> +# You can re-order the commit lines to affect the order on which
>>> +# the commits will be applied.
>>
>> That's probably the main use of "rebase -i", so it deserves a mention,
>> yes.
> 
> Erm, read in isolation by people like you who _know_ what the command
> does, the addition might look sensible, but I find that line of thinking
> somewhat disturbing in the first place.
> 
> These are meant to be a short _reminder_ for people who _learned_ how the
> command they already _know_ is spelled.
> 
> If the user does not even know that one of the primary use of the command
> is to reorder, isn't he playing with fire?

In an ideal world everybody should study the manpages and documentation of
a program prior to using it, unfortunately most people often start with an
easy-to-follow tutorial where there are commands to type in, the user
will type them (confident in the tutorial reputation) and try to learn along
the way, in this case giving the user more indications about what he can 
do in any moment is, in my opinion, a good thing (as git already does). 


> If we come up with a politely worded message that makes the user realize
> that the user does not know what he is doing and gently nudges the user to
> read the documentation before continuing to potentially harm himself, that
> would be a good addition, but...
> 
> So, I dunno.

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Matthieu Moy]

Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:

> On 03/16/2012 01:17 PM, Junio C Hamano wrote:
>> If the user does not even know that one of the primary use of the command
>> is to reorder, isn't he playing with fire?
>
> In an ideal world everybody should study the manpages and documentation of
> a program prior to using it, unfortunately most people often start with an
> easy-to-follow tutorial where there are commands to type in, the user
> will type them (confident in the tutorial reputation) and try to learn along
> the way, in this case giving the user more indications about what he can 
> do in any moment is, in my opinion, a good thing (as git already does). 

The case of "rebase" is a bit different, since "rebase" is potentially
dangerous.

But if the user sees the help message of "rebase -i", then I don't think
we will stop him from breaking anything by not mentionning that
reordering commits can work. We already give other commands that rewrite
history with more or less danger, so the additional messages isn't
really harmful.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> But if the user sees the help message of "rebase -i", then I don't think
> we will stop him from breaking anything by not mentionning that
> reordering commits can work. We already give other commands that rewrite
> history with more or less danger, so the additional messages isn't
> really harmful.

I was not questioning that "You can also reorder" is dangerous in any way.

What is dangerous is to give an illusion that it is safe to use "rebase
-i" without reading a decent documentation that does not even say that
reordering is one of the primary purpose of the command, and the mindset
that it is OK to give such an illusion with these reminder lines.

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> But if the user sees the help message of "rebase -i", then I don't think
>> we will stop him from breaking anything by not mentionning that
>> reordering commits can work. We already give other commands that rewrite
>> history with more or less danger, so the additional messages isn't
>> really harmful.
>
> I was not questioning that "You can also reorder" is dangerous in any way.
>
> What is dangerous is to give an illusion that it is safe to use "rebase
> -i" without reading a decent documentation that does not even say that
> reordering is one of the primary purpose of the command, and the mindset
> that it is OK to give such an illusion with these reminder lines.

Yes, but my point is that you can already do that. Nelson just gave an
example when explaining how he came out with this patch.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>>
>>> But if the user sees the help message of "rebase -i", then I don't think
>>> we will stop him from breaking anything by not mentionning that
>>> reordering commits can work. We already give other commands that rewrite
>>> history with more or less danger, so the additional messages isn't
>>> really harmful.
>>
>> I was not questioning that "You can also reorder" is dangerous in any way.
>>
>> What is dangerous is to give an illusion that it is safe to use "rebase
>> -i" without reading a decent documentation that does not even say that
>> reordering is one of the primary purpose of the command, and the mindset
>> that it is OK to give such an illusion with these reminder lines.
>
> Yes, but my point is that you can already do that. Nelson just gave an
> example when explaining how he came out with this patch.

Yes, I know that.

And a patch like this will _encourage_ the users to follow that path even
more. Is it a good thing?  That is what I was questioning.

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> But if the user sees the help message of "rebase -i", then I don't think
>> we will stop him from breaking anything by not mentionning that
>> reordering commits can work. We already give other commands that rewrite
>> history with more or less danger, so the additional messages isn't
>> really harmful.
>
> I was not questioning that "You can also reorder" is dangerous in any way.
>
> What is dangerous is to give an illusion that it is safe to use "rebase
> -i" without reading a decent documentation that does not even say that
> reordering is one of the primary purpose of the command, and the mindset
> that it is OK to give such an illusion with these reminder lines.

Another reason we may not want to say "you can re-order" is that we will
probably need to say that you cannot arbitrarily re-order (can s/f come at
the beginning?) unless you want to actively harm the user. Once we go down
that path, we would end up repeating the manual.

Having said that, I do not personally care about this too deeply.  Let's
queue a reworded Nelson's patch so it is not lost while waiting for others
to chime in.

How about this?  I avoided phrases "applying commit" (incorrect: you apply
patches, and you replay or cherry-pick commits) and "history" (to people
who know, it is redundant; to people who don't, it is vague mystery), but
added one more useful reminder (top to bottom).

 git-rebase--interactive.sh |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 5812222..fd04c8f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -846,6 +846,8 @@ cat >> "$todo" << EOF
 #  f, fixup = like "squash", but discard this commit's log message
 #  x, exec = run command (the rest of the line) using shell
 #
+# These lines can be re-ordered; they are executed from top to bottom.
+# 
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
 #

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Having said that, I do not personally care about this too deeply.  Let's
> queue a reworded Nelson's patch so it is not lost while waiting for others
> to chime in.
>
> How about this?  I avoided phrases "applying commit" (incorrect: you apply
> patches, and you replay or cherry-pick commits) and "history" (to people
> who know, it is redundant; to people who don't, it is vague mystery), but
> added one more useful reminder (top to bottom).

I didn't hear anything from anybody on this one, so I take it that nobody
deeply cares either way.

I'll queue this to 'next' so that we can start the post-1.7.10 cycle with
this patch.

-- >8 --
Subject: [PATCH] rebase -i: remind that the lines are top-to-bottom

Nelson Benitez Leon opened a discussion with a patch with this in the
note:

    Hi, I was using git rebase -i for some time now and never occured to
    me I could reorder the commit lines to affect the order the commits
    are applied, learnt that recently from a git tutorial.

Nelson's patch was to stress the fact that the lines in the insn sheet can
be re-ordered in a much more verbose way.  Let's add a one-liner reminder
and also remind that the lines in the insn sheet is read from top to
bottom, unlike the "git log" output.

Discussion-triggered-by: Nelson Benitez Leon
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-rebase--interactive.sh |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 5812222..2b7eb6d 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -846,6 +846,8 @@ cat >> "$todo" << EOF
 #  f, fixup = like "squash", but discard this commit's log message
 #  x, exec = run command (the rest of the line) using shell
 #
+# These lines can be re-ordered; they are executed from top to bottom.
+#
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
 #
-- 
1.7.10.rc1.63.g3c4e6

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -846,6 +846,8 @@ cat >> "$todo" << EOF
>  #  f, fixup = like "squash", but discard this commit's log message
>  #  x, exec = run command (the rest of the line) using shell
>  #
> +# These lines can be re-ordered; they are executed from top to bottom.
> +#

Great, it says everything that needs to be said, in just a line!

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] rebase -i: inform the user they can re-order commits

[Philip Oakley]

From: "Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr> Sent: Wednesday, March 
21, 2012 8:25 AM
> Junio C Hamano <gitster@pobox.com> writes:
>
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -846,6 +846,8 @@ cat >> "$todo" << EOF
>>  #  f, fixup = like "squash", but discard this commit's log message
>>  #  x, exec = run command (the rest of the line) using shell
>>  #
>> +# These lines can be re-ordered; they are executed from top to bottom.
>> +#
>
> Great, it says everything that needs to be said, in just a line!
>
> -- 
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
> --

I agree. It says all that needs to be made clear.
Philip Oakley