[PATCH] rebase -i: add config to abbreviate command name

[PATCH] rebase -i: add config to abbreviate command name

[Liam Beguin]

Add the 'rebase.abbrevCmd' boolean config option to allow
the user to abbreviate the default command name while editing
the 'git-rebase-todo' file.

Signed-off-by: Liam Beguin <liambeguin@gmail.com>
---
Notes:

 *  This allows the lines to remain aligned when using single
    letter commands.

 Documentation/config.txt     | 3 +++
 Documentation/git-rebase.txt | 3 +++
 git-rebase--interactive.sh   | 8 ++++++--
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 475e874d5155..59b64832aeb4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2614,6 +2614,9 @@ rebase.instructionFormat::
 	the instruction list during an interactive rebase.  The format will automatically
 	have the long commit hash prepended to the format.
 
+rebase.abbrevCmd::
+	If set to true, abbreviate command name in interactive mode.
+
 receive.advertiseAtomic::
 	By default, git-receive-pack will advertise the atomic push
 	capability to its clients. If you don't want to advertise this
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 67d48e688315..0c423d903625 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -222,6 +222,9 @@ rebase.missingCommitsCheck::
 rebase.instructionFormat::
 	Custom commit list format to use during an `--interactive` rebase.
 
+rebase.abbrevCmd::
+	If set to true, abbreviate command name in interactive mode.
+
 OPTIONS
 -------
 --onto <newbase>::
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2c9c0165b5ab..9f3e82b79615 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1210,6 +1210,10 @@ else
 	revisions=$onto...$orig_head
 	shortrevisions=$shorthead
 fi
+
+rebasecmd=pick
+test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p
+
 format=$(git config --get rebase.instructionFormat)
 # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
 git rev-list $merges_option --format="%m%H ${format:-%s}" \
@@ -1228,7 +1232,7 @@ do
 
 	if test t != "$preserve_merges"
 	then
-		printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+		printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
 	else
 		if test -z "$rebase_root"
 		then
@@ -1246,7 +1250,7 @@ do
 		if test f = "$preserve"
 		then
 			touch "$rewritten"/$sha1
-			printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+			printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
 		fi
 	fi
 done
-- 
2.9.3

Re: [PATCH] rebase -i: add config to abbreviate command name

[Johannes Schindelin]

Hi Liam,

On Sun, 23 Apr 2017, Liam Beguin wrote:

> Add the 'rebase.abbrevCmd' boolean config option to allow
> the user to abbreviate the default command name while editing
> the 'git-rebase-todo' file.

This patch does not handle the `git rebase --edit-todo` subcommand.
Intentional?

Ciao,
Johannes

Re: [PATCH] rebase -i: add config to abbreviate command name

[liam BEGUIN]

Hi, 

On Mon, 2017-04-24 at 12:26 +0200, Johannes Schindelin wrote:
> Hi Liam,
> 
> On Sun, 23 Apr 2017, Liam Beguin wrote:
> 
> > Add the 'rebase.abbrevCmd' boolean config option to allow
> > the user to abbreviate the default command name while editing
> > the 'git-rebase-todo' file.
> 
> This patch does not handle the `git rebase --edit-todo` subcommand.
> Intentional?

no, this is not intentional, I'll make the changes.
 
> 
> Ciao,
> Johannes

Thanks, 
Liam

Re: [PATCH] rebase -i: add config to abbreviate command name

[Jeff King]

On Sun, Apr 23, 2017 at 11:23:47PM -0400, Liam Beguin wrote:

> Add the 'rebase.abbrevCmd' boolean config option to allow
> the user to abbreviate the default command name while editing
> the 'git-rebase-todo' file.

Just reading this, I was confused about what the patch actually did.
Reading the code, I figured it out, but perhaps an example would make
sense. Like:

  This means that we will print:

    p 1234abcd subject line

  in the todo file rather than:

    pick 1234abcd subject line

And then of course that left me wondering why somebody would want to do
that. I understand wanting to _type_ the abbreviated version, but surely
it's not too much work to read the full word?

Then I saw:

> ---
> Notes:
> 
>  *  This allows the lines to remain aligned when using single
>     letter commands.

That makes some sense. it should probably be part of the commit message,
so that future readers of "git log" understand why the change was made.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 475e874d5155..59b64832aeb4 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2614,6 +2614,9 @@ rebase.instructionFormat::
>  	the instruction list during an interactive rebase.  The format will automatically
>  	have the long commit hash prepended to the format.
>  
> +rebase.abbrevCmd::
> +	If set to true, abbreviate command name in interactive mode.

Similar to the commit message, this might need to go into more detail.
It was not immediately obvious to me that "command name" means the
command-names in the instruction list.

-Peff

Re: [PATCH] rebase -i: add config to abbreviate command name

[liam BEGUIN]

Hi Johannes,

On Mon, 2017-04-24 at 12:26 +0200, Johannes Schindelin wrote:
> Hi Liam,
> 
> On Sun, 23 Apr 2017, Liam Beguin wrote:
> 
> > Add the 'rebase.abbrevCmd' boolean config option to allow
> > the user to abbreviate the default command name while editing
> > the 'git-rebase-todo' file.
> 
> This patch does not handle the `git rebase --edit-todo` subcommand.
> Intentional?

After a little more investigation, I'm not sure what should be added for
the `git rebase --edit-todo` subcommand. It seems like it uses the same 
text that was added the first time (with `git rebase -i`). 
Do you have a bit more information about what you meant? 
I don't use this subcommand very often, I'm most likely missing something.

> 
> Ciao,
> Johannes

Thanks, 
Liam 

[PATCH v2] rebase -i: add config to abbreviate command-names

[Liam Beguin]

Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i`
to abbreviate the command-names in the instruction list.

This means that `git rebase -i` would print:
    p deadbee The oneline of this commit
    ...

instead of:
    pick deadbee The oneline of this commit
    ...

Using a single character command-name allows the lines to remain
aligned, making the whole set more readable.

Signed-off-by: Liam Beguin <liambeguin@gmail.com>
---
Changes since v1:
 - Improve Documentation and commit message

 Documentation/config.txt     | 19 +++++++++++++++++++
 Documentation/git-rebase.txt | 19 +++++++++++++++++++
 git-rebase--interactive.sh   |  8 ++++++--
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 475e874d5155..8b1877f2df91 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2614,6 +2614,25 @@ rebase.instructionFormat::
 	the instruction list during an interactive rebase.  The format will automatically
 	have the long commit hash prepended to the format.
 
+rebase.abbrevCmd::
+	If set to true, `git rebase -i` will abbreviate the command-names in the
+	instruction list. This means that instead of looking like this,
+
+-------------------------------------------
+	pick deadbee The oneline of this commit
+	pick fa1afe1 The oneline of the next commit
+	...
+-------------------------------------------
+
+	the list would use the short version of the command resulting in
+	something like this.
+
+-------------------------------------------
+	p deadbee The oneline of this commit
+	p fa1afe1 The oneline of the next commit
+	...
+-------------------------------------------
+
 receive.advertiseAtomic::
 	By default, git-receive-pack will advertise the atomic push
 	capability to its clients. If you don't want to advertise this
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 67d48e688315..7d97c0483241 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -222,6 +222,25 @@ rebase.missingCommitsCheck::
 rebase.instructionFormat::
 	Custom commit list format to use during an `--interactive` rebase.
 
+rebase.abbrevCmd::
+	If set to true, `git rebase -i` will abbreviate the command-names in the
+	instruction list. This means that instead of looking like this,
+
+-------------------------------------------
+	pick deadbee The oneline of this commit
+	pick fa1afe1 The oneline of the next commit
+	...
+-------------------------------------------
+
+	the list would use the short version of the command resulting in
+	something like this.
+
+-------------------------------------------
+	p deadbee The oneline of this commit
+	p fa1afe1 The oneline of the next commit
+	...
+-------------------------------------------
+
 OPTIONS
 -------
 --onto <newbase>::
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2c9c0165b5ab..9f3e82b79615 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1210,6 +1210,10 @@ else
 	revisions=$onto...$orig_head
 	shortrevisions=$shorthead
 fi
+
+rebasecmd=pick
+test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p
+
 format=$(git config --get rebase.instructionFormat)
 # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
 git rev-list $merges_option --format="%m%H ${format:-%s}" \
@@ -1228,7 +1232,7 @@ do
 
 	if test t != "$preserve_merges"
 	then
-		printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+		printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
 	else
 		if test -z "$rebase_root"
 		then
@@ -1246,7 +1250,7 @@ do
 		if test f = "$preserve"
 		then
 			touch "$rewritten"/$sha1
-			printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+			printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
 		fi
 	fi
 done
-- 
2.9.3

[PATCH v2] rebase -i: add config to abbreviate command-names

[Liam Beguin]

Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i`
to abbreviate the command-names in the instruction list.

This means that `git rebase -i` would print:
    p deadbee The oneline of this commit
    ...

instead of:
    pick deadbee The oneline of this commit
    ...

Using a single character command-name allows the lines to remain
aligned, making the whole set more readable.

Signed-off-by: Liam Beguin <liambeguin@gmail.com>
---
Changes since v1:
 - Improve Documentation and commit message

 Documentation/config.txt     | 19 +++++++++++++++++++
 Documentation/git-rebase.txt | 19 +++++++++++++++++++
 git-rebase--interactive.sh   |  8 ++++++--
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 475e874d5155..8b1877f2df91 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2614,6 +2614,25 @@ rebase.instructionFormat::
 	the instruction list during an interactive rebase.  The format will automatically
 	have the long commit hash prepended to the format.
 
+rebase.abbrevCmd::
+	If set to true, `git rebase -i` will abbreviate the command-names in the
+	instruction list. This means that instead of looking like this,
+
+-------------------------------------------
+	pick deadbee The oneline of this commit
+	pick fa1afe1 The oneline of the next commit
+	...
+-------------------------------------------
+
+	the list would use the short version of the command resulting in
+	something like this.
+
+-------------------------------------------
+	p deadbee The oneline of this commit
+	p fa1afe1 The oneline of the next commit
+	...
+-------------------------------------------
+
 receive.advertiseAtomic::
 	By default, git-receive-pack will advertise the atomic push
 	capability to its clients. If you don't want to advertise this
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 67d48e688315..7d97c0483241 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -222,6 +222,25 @@ rebase.missingCommitsCheck::
 rebase.instructionFormat::
 	Custom commit list format to use during an `--interactive` rebase.
 
+rebase.abbrevCmd::
+	If set to true, `git rebase -i` will abbreviate the command-names in the
+	instruction list. This means that instead of looking like this,
+
+-------------------------------------------
+	pick deadbee The oneline of this commit
+	pick fa1afe1 The oneline of the next commit
+	...
+-------------------------------------------
+
+	the list would use the short version of the command resulting in
+	something like this.
+
+-------------------------------------------
+	p deadbee The oneline of this commit
+	p fa1afe1 The oneline of the next commit
+	...
+-------------------------------------------
+
 OPTIONS
 -------
 --onto <newbase>::
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2c9c0165b5ab..9f3e82b79615 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1210,6 +1210,10 @@ else
 	revisions=$onto...$orig_head
 	shortrevisions=$shorthead
 fi
+
+rebasecmd=pick
+test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p
+
 format=$(git config --get rebase.instructionFormat)
 # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
 git rev-list $merges_option --format="%m%H ${format:-%s}" \
@@ -1228,7 +1232,7 @@ do
 
 	if test t != "$preserve_merges"
 	then
-		printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+		printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
 	else
 		if test -z "$rebase_root"
 		then
@@ -1246,7 +1250,7 @@ do
 		if test f = "$preserve"
 		then
 			touch "$rewritten"/$sha1
-			printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
+			printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
 		fi
 	fi
 done
-- 
2.9.3

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Junio C Hamano]

Liam Beguin <liambeguin@gmail.com> writes:

> Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i`
> to abbreviate the command-names in the instruction list.
>
> This means that `git rebase -i` would print:
>     p deadbee The oneline of this commit
>     ...
>
> instead of:
>     pick deadbee The oneline of this commit
>     ...

Whenever I see "This means that...", my automatic reaction is "The
author expects what s/he wrote previously is not understandable, and
is making another try to give something more readable.  As this is
not a real time communication, why not rewrite the incomprehensible
part before wasting the time of the readers by throwing at them what
is known to the author to be unreadble, only to clarify with 'This
means that...' later?"

But I think in this case, you do not even have to say "This means
that".  What you wrote, without "This means that", i.e.

    Add the 'rebase.abbrevCommand' configuration variable to tell
    `git rebase -i` to show commands abbreviated in the instruction
    list, i.e.

         p deadbee The oneline of this commit
         ...

    instead of:

         pick deadbee The oneline of this commit
         ...

is quite readable.

> Using a single character command-name allows the lines to remain
> aligned, making the whole set more readable.

Hmph.  I have trouble with "lines remain aligned".  Depending on the
object names of commits, don't you end up getting something like
this that is not aligned?

    p deadbee The oneline
    p e2cb6ab8 Another commit

Or are you happy with only the beginning of object names aligned,
without the actual titles aligned?

Personally I am happy with the beginning of each instruction line
aligned, so from that point of view, this patch is a mild Meh to me,
even though I do a fair amount of "rebase -i" myself.  But obviously
I am not the only user of Git you need to please, so...

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Jacob Keller]

On Mon, Apr 24, 2017 at 11:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Personally I am happy with the beginning of each instruction line
> aligned, so from that point of view, this patch is a mild Meh to me,
> even though I do a fair amount of "rebase -i" myself.  But obviously
> I am not the only user of Git you need to please, so...

I would instead justify this as making it easier to change the action,
since you only need to rewrite a single letter, which at least in vim
takes "r<letter>" to change the action, vs slightly more keystrokes
such as "ct <letter" or otherwise.

Also, if you change the default commit hash length, it becomes long
enough to cover most commits and you see all commits at say 12 digits
commit hash and everything is nicely aligned.

Thanks,
Jake

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Andreas Schwab]

On Apr 25 2017, Liam Beguin <liambeguin@gmail.com> wrote:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 475e874d5155..8b1877f2df91 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2614,6 +2614,25 @@ rebase.instructionFormat::
>  	the instruction list during an interactive rebase.  The format will automatically
>  	have the long commit hash prepended to the format.
>  
> +rebase.abbrevCmd::
> +	If set to true, `git rebase -i` will abbreviate the command-names in the
> +	instruction list. This means that instead of looking like this,
> +
> +-------------------------------------------
> +	pick deadbee The oneline of this commit
> +	pick fa1afe1 The oneline of the next commit
> +	...
> +-------------------------------------------
> +
> +	the list would use the short version of the command resulting in
> +	something like this.
> +
> +-------------------------------------------
> +	p deadbee The oneline of this commit
> +	p fa1afe1 The oneline of the next commit
> +	...
> +-------------------------------------------

That doesn't explain the point of the option.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Andreas Schwab]

On Apr 25 2017, Liam Beguin <liambeguin@gmail.com> wrote:

> Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i`
> to abbreviate the command-names in the instruction list.
>
> This means that `git rebase -i` would print:
>     p deadbee The oneline of this commit
>     ...
>
> instead of:
>     pick deadbee The oneline of this commit
>     ...
>
> Using a single character command-name allows the lines to remain
> aligned, making the whole set more readable.

Perhaps there should rather be an option to tell rebase to align the
columns?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Philip Oakley]

From: "Liam Beguin" <liambeguin@gmail.com>
> Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i`
> to abbreviate the command-names in the instruction list.
>
> This means that `git rebase -i` would print:
>    p deadbee The oneline of this commit
>    ...
>
> instead of:
>    pick deadbee The oneline of this commit
>    ...
>
> Using a single character command-name allows the lines to remain
> aligned, making the whole set more readable.
>
> Signed-off-by: Liam Beguin <liambeguin@gmail.com>
> ---
> Changes since v1:
> - Improve Documentation and commit message
>
> Documentation/config.txt     | 19 +++++++++++++++++++
> Documentation/git-rebase.txt | 19 +++++++++++++++++++
> git-rebase--interactive.sh   |  8 ++++++--
> 3 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 475e874d5155..8b1877f2df91 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2614,6 +2614,25 @@ rebase.instructionFormat::
>  the instruction list during an interactive rebase.  The format will 
> automatically
>  have the long commit hash prepended to the format.
>
> +rebase.abbrevCmd::
> + If set to true, `git rebase -i` will abbreviate the command-names in the
> + instruction list. This means that instead of looking like this,
> +
> +-------------------------------------------
> + pick deadbee The oneline of this commit
> + pick fa1afe1 The oneline of the next commit
> + ...
> +-------------------------------------------
> +
> + the list would use the short version of the command resulting in
> + something like this.

Perhaps use an example which does have rebase commands of different lengths, 
such as 'pick', 'squash', 'reword' to more clearly show the intent of 
alignment and subsequent ease of editing?

--
Philip


> +
> +-------------------------------------------
> + p deadbee The oneline of this commit
> + p fa1afe1 The oneline of the next commit
> + ...
> +-------------------------------------------
> +
> receive.advertiseAtomic::
>  By default, git-receive-pack will advertise the atomic push
>  capability to its clients. If you don't want to advertise this
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 67d48e688315..7d97c0483241 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -222,6 +222,25 @@ rebase.missingCommitsCheck::
> rebase.instructionFormat::
>  Custom commit list format to use during an `--interactive` rebase.
>
> +rebase.abbrevCmd::
> + If set to true, `git rebase -i` will abbreviate the command-names in the
> + instruction list. This means that instead of looking like this,
> +
> +-------------------------------------------
> + pick deadbee The oneline of this commit
> + pick fa1afe1 The oneline of the next commit
> + ...
> +-------------------------------------------
> +
> + the list would use the short version of the command resulting in
> + something like this.
> +
> +-------------------------------------------
> + p deadbee The oneline of this commit
> + p fa1afe1 The oneline of the next commit
> + ...
> +-------------------------------------------
> +
> OPTIONS
> -------
> --onto <newbase>::
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 2c9c0165b5ab..9f3e82b79615 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -1210,6 +1210,10 @@ else
>  revisions=$onto...$orig_head
>  shortrevisions=$shorthead
> fi
> +
> +rebasecmd=pick
> +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p
> +
> format=$(git config --get rebase.instructionFormat)
> # the 'rev-list .. | sed' requires %m to parse; the instruction requires 
> %H to parse
> git rev-list $merges_option --format="%m%H ${format:-%s}" \
> @@ -1228,7 +1232,7 @@ do
>
>  if test t != "$preserve_merges"
>  then
> - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
> + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
>  else
>  if test -z "$rebase_root"
>  then
> @@ -1246,7 +1250,7 @@ do
>  if test f = "$preserve"
>  then
>  touch "$rewritten"/$sha1
> - printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
> + printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
>  fi
>  fi
> done
> -- 
> 2.9.3
>
> 

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Mike Rappazzo]

On Tue, Apr 25, 2017 at 5:57 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Apr 25 2017, Liam Beguin <liambeguin@gmail.com> wrote:
>
>> Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i`
>> to abbreviate the command-names in the instruction list.
>>
>> This means that `git rebase -i` would print:
>>     p deadbee The oneline of this commit
>>     ...
>>
>> instead of:
>>     pick deadbee The oneline of this commit
>>     ...
>>
>> Using a single character command-name allows the lines to remain
>> aligned, making the whole set more readable.
>
> Perhaps there should rather be an option to tell rebase to align the
> columns?
>

You _can_ set a custom instruction format using the config variable:
`rebase.instructionFormat`.  With this, you can align columns using
the normal git log format.

For example, I personally use this as my instruction format:

    [%an%<|(64)%x5d %s

While, this won't always align perfectly, it may help scratch your itch.

Re: [PATCH] rebase -i: add config to abbreviate command name

[Johannes Schindelin]

Hi Liam,

On Mon, 24 Apr 2017, liam BEGUIN wrote:

> On Mon, 2017-04-24 at 12:26 +0200, Johannes Schindelin wrote:
> 
> > On Sun, 23 Apr 2017, Liam Beguin wrote:
> > 
> > > Add the 'rebase.abbrevCmd' boolean config option to allow the user
> > > to abbreviate the default command name while editing the
> > > 'git-rebase-todo' file.
> > 
> > This patch does not handle the `git rebase --edit-todo` subcommand.
> > Intentional?
> 
> After a little more investigation, I'm not sure what should be added for
> the `git rebase --edit-todo` subcommand. It seems like it uses the same
> text that was added the first time (with `git rebase -i`).

Well, it uses whatever the user may have edited. It may surprise users
that their `pick` does not get converted to `p` like all the original
commands.

Ciao,
Johannes

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Johannes Schindelin]

Hi Liam,

On Tue, 25 Apr 2017, Liam Beguin wrote:

> Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i`
> to abbreviate the command-names in the instruction list.
> 
> This means that `git rebase -i` would print:
>     p deadbee The oneline of this commit
>     ...
> 
> instead of:
>     pick deadbee The oneline of this commit
>     ...
> 
> Using a single character command-name allows the lines to remain
> aligned, making the whole set more readable.
> 
> Signed-off-by: Liam Beguin <liambeguin@gmail.com>

Apart from either abbreviating commands after --edit-todo, or documenting
explicitly that the new config option only concerns the initial todo list,
there is another problem that just occurred to me: --exec.

When you call `git rebase -x "make DEVELOPER=1 -j15"`, the idea is to
append an "exec make DEVELOPER=1 -j15" line after every pick line. The
code in question looks like this:

add_exec_commands () {
        {
                first=t
                while read -r insn rest
                do
                        case $insn in
                        pick)
                                test -n "$first" ||
                                printf "%s" "$cmd"
                                ;;
                        esac
                        printf "%s %s\n" "$insn" "$rest"
                        first=
                done
                printf "%s" "$cmd"
        } <"$1" >"$1.new" &&
        mv "$1.new" "$1"
}

Obviously, the git-rebase--interactive script expects at this point that
the command is spelled out, so your patch needs to change the `pick)` case
to `p|pick)`, I think.

In addition, since the rationale for the new option is to align the lines
better, the `exec` would need to be replaced by `x`, and as multiple `-x`
options are allowed, you would need something like this at the beginning
of `add_exec_commands`, too:

	# abbreviate `exec` if rebase.abbrevCmd is true
	test p != "$rebasecmd" ||
	cmd="$(echo "$cmd" | sed 's/^exec/x/')"

Also:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 475e874d5155..8b1877f2df91 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2614,6 +2614,25 @@ rebase.instructionFormat::
>  	the instruction list during an interactive rebase.  The format will automatically
>  	have the long commit hash prepended to the format.
>  
> +rebase.abbrevCmd::

It does not fail to amuse that the term "abbrevCmd" is abbreviated
heavily itself. However, I would strongly suggest to avoid that. It would
be much more pleasant to call the config option rebase.abbreviateCommands

> +rebase.abbrevCmd::
> +	If set to true, `git rebase -i` will abbreviate the command-names in the
> +	instruction list. This means that instead of looking like this,

This is by no means your fault, but it is really horrible by how many
different names Git's documentation refers to the todo script, nothing
short of confusing. It is the todo script (which I called it initially,
maybe not a good name, but it has the merit of the longest tradition at
least), the todo list, the instruction sheet, the rebase script, the
instruction list... etc

However, the thing is called "todo list" elsewhere in the same file,
therefore lets try to avoid even more confusion and use that term instead
of "instruction list" here.

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 2c9c0165b5ab..9f3e82b79615 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -1210,6 +1210,10 @@ else
>  	revisions=$onto...$orig_head
>  	shortrevisions=$shorthead
>  fi
> +
> +rebasecmd=pick
> +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p

A better name would be "pickcmd", as there are more rebase commands than
just `pick` and what we want here is really only associated with one of
those commands.

Ciao,
Johannes

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Johannes Schindelin]

Hi Andreas,

On Tue, 25 Apr 2017, Andreas Schwab wrote:

> On Apr 25 2017, Liam Beguin <liambeguin@gmail.com> wrote:
> 
> > diff --git a/Documentation/config.txt b/Documentation/config.txt
> > index 475e874d5155..8b1877f2df91 100644
> > --- a/Documentation/config.txt
> > +++ b/Documentation/config.txt
> > @@ -2614,6 +2614,25 @@ rebase.instructionFormat::
> >  	the instruction list during an interactive rebase.  The format will automatically
> >  	have the long commit hash prepended to the format.
> >  
> > +rebase.abbrevCmd::
> > +	If set to true, `git rebase -i` will abbreviate the command-names in the
> > +	instruction list. This means that instead of looking like this,
> > +
> > +-------------------------------------------
> > +	pick deadbee The oneline of this commit
> > +	pick fa1afe1 The oneline of the next commit
> > +	...
> > +-------------------------------------------
> > +
> > +	the list would use the short version of the command resulting in
> > +	something like this.
> > +
> > +-------------------------------------------
> > +	p deadbee The oneline of this commit
> > +	p fa1afe1 The oneline of the next commit
> > +	...
> > +-------------------------------------------
> 
> That doesn't explain the point of the option.

And what you forgot to say in order to make this a constructive criticism
is: we probably want to add a sentence like this:


	Using the one-letter abbreviations will align the lines better
	in case that the non-abbreviated commands have different lengths.

Speaking of commands with different lengths, I just thought of fixup and
squash. I do not think those are handled by the patch, but they should be
(the `action` in the first loop of `rearrange_squash` should abbreviate
via `test p != "$pickcmd" || action=${action%${action#?}}`).

Ciao,
Johannes

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[liam BEGUIN]

Hi Johannes,


On Tue, 2017-04-25 at 23:23 +0200, Johannes Schindelin wrote:
> Hi Andreas,
> 
> On Tue, 25 Apr 2017, Andreas Schwab wrote:
> 
> > On Apr 25 2017, Liam Beguin <liambeguin@gmail.com> wrote:
> > 
> > > diff --git a/Documentation/config.txt b/Documentation/config.txt
> > > index 475e874d5155..8b1877f2df91 100644
> > > --- a/Documentation/config.txt
> > > +++ b/Documentation/config.txt
> > > @@ -2614,6 +2614,25 @@ rebase.instructionFormat::
> > >  	the instruction list during an interactive rebase.  The format will automatically
> > >  	have the long commit hash prepended to the format.
> > >  
> > > +rebase.abbrevCmd::
> > > +	If set to true, `git rebase -i` will abbreviate the command-names in the
> > > +	instruction list. This means that instead of looking like this,
> > > +
> > > +-------------------------------------------
> > > +	pick deadbee The oneline of this commit
> > > +	pick fa1afe1 The oneline of the next commit
> > > +	...
> > > +-------------------------------------------
> > > +
> > > +	the list would use the short version of the command resulting in
> > > +	something like this.
> > > +
> > > +-------------------------------------------
> > > +	p deadbee The oneline of this commit
> > > +	p fa1afe1 The oneline of the next commit
> > > +	...
> > > +-------------------------------------------
> > 
> > That doesn't explain the point of the option.
> 
> And what you forgot to say in order to make this a constructive criticism
> is: we probably want to add a sentence like this:
> 
> 
> 	Using the one-letter abbreviations will align the lines better
> 	in case that the non-abbreviated commands have different lengths.
> 
> Speaking of commands with different lengths, I just thought of fixup and
> squash. I do not think those are handled by the patch, but they should be
> (the `action` in the first loop of `rearrange_squash` should abbreviate
> via `test p != "$pickcmd" || action=${action%${action#?}}`).
> 

I just noticed this today, I'll make changes to handle this case. 

> Ciao,
> Johannes

Thanks,
Liam

Re: [PATCH] rebase -i: add config to abbreviate command name

[liam BEGUIN]

Hi Johannes,

On Tue, 2017-04-25 at 21:45 +0200, Johannes Schindelin wrote:
> Hi Liam,
> 
> On Mon, 24 Apr 2017, liam BEGUIN wrote:
> 
> > On Mon, 2017-04-24 at 12:26 +0200, Johannes Schindelin wrote:
> > 
> > > On Sun, 23 Apr 2017, Liam Beguin wrote:
> > > 
> > > > Add the 'rebase.abbrevCmd' boolean config option to allow the user
> > > > to abbreviate the default command name while editing the
> > > > 'git-rebase-todo' file.
> > > 
> > > This patch does not handle the `git rebase --edit-todo` subcommand.
> > > Intentional?
> > 
> > After a little more investigation, I'm not sure what should be added for
> > the `git rebase --edit-todo` subcommand. It seems like it uses the same
> > text that was added the first time (with `git rebase -i`).
> 
> Well, it uses whatever the user may have edited. It may surprise users
> that their `pick` does not get converted to `p` like all the original
> commands.
> 

It makes more sens to me now, I'll add it in next patch

> Ciao,
> Johannes

Thanks, 
Liam

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[liam Beguin]

Hi Jake, 

On Tue, 2017-04-25 at 01:29 -0700, Jacob Keller wrote:
> On Mon, Apr 24, 2017 at 11:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Personally I am happy with the beginning of each instruction line
> > aligned, so from that point of view, this patch is a mild Meh to me,
> > even though I do a fair amount of "rebase -i" myself.  But obviously
> > I am not the only user of Git you need to please, so...
> 
> I would instead justify this as making it easier to change the action,
> since you only need to rewrite a single letter, which at least in vim
> takes "r<letter>" to change the action, vs slightly more keystrokes
> such as "ct <letter" or otherwise.

It's another reason that motivated the change but I didn't think the
vim shortcuts would justify the patch. Since you pointed it out, 
I'll probably add it.

> 
> Also, if you change the default commit hash length, it becomes long
> enough to cover most commits and you see all commits at say 12 digits
> commit hash and everything is nicely aligned.
> 
> Thanks,
> Jake

Thanks,
Liam 

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[liam Beguin]

Hi Johannes, 

On Tue, 2017-04-25 at 22:08 +0200, Johannes Schindelin wrote:
> Hi Liam,
> 
> On Tue, 25 Apr 2017, Liam Beguin wrote:
> 
> > Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i`
> > to abbreviate the command-names in the instruction list.
> > 
> > This means that `git rebase -i` would print:
> >     p deadbee The oneline of this commit
> >     ...
> > 
> > instead of:
> >     pick deadbee The oneline of this commit
> >     ...
> > 
> > Using a single character command-name allows the lines to remain
> > aligned, making the whole set more readable.
> > 
> > Signed-off-by: Liam Beguin <liambeguin@gmail.com>
> 
> Apart from either abbreviating commands after --edit-todo, or documenting
> explicitly that the new config option only concerns the initial todo list,
> there is another problem that just occurred to me: --exec.
> 
> When you call `git rebase -x "make DEVELOPER=1 -j15"`, the idea is to
> append an "exec make DEVELOPER=1 -j15" line after every pick line. The
> code in question looks like this:
> 
> add_exec_commands () {
>         {
>                 first=t
>                 while read -r insn rest
>                 do
>                         case $insn in
>                         pick)
>                                 test -n "$first" ||
>                                 printf "%s" "$cmd"
>                                 ;;
>                         esac
>                         printf "%s %s\n" "$insn" "$rest"
>                         first=
>                 done
>                 printf "%s" "$cmd"
>         } <"$1" >"$1.new" &&
>         mv "$1.new" "$1"
> }
> 
> Obviously, the git-rebase--interactive script expects at this point that
> the command is spelled out, so your patch needs to change the `pick)` case
> to `p|pick)`, I think.
> 
> In addition, since the rationale for the new option is to align the lines
> better, the `exec` would need to be replaced by `x`, and as multiple `-x`
> options are allowed, you would need something like this at the beginning
> of `add_exec_commands`, too:
> 
> 	# abbreviate `exec` if rebase.abbrevCmd is true
> 	test p != "$rebasecmd" ||
> 	cmd="$(echo "$cmd" | sed 's/^exec/x/')"
> 



> Also:
> 
> > diff --git a/Documentation/config.txt b/Documentation/config.txt
> > index 475e874d5155..8b1877f2df91 100644
> > --- a/Documentation/config.txt
> > +++ b/Documentation/config.txt
> > @@ -2614,6 +2614,25 @@ rebase.instructionFormat::
> >  	the instruction list during an interactive rebase.  The format will automatically
> >  	have the long commit hash prepended to the format.
> >  
> > +rebase.abbrevCmd::
> 
> It does not fail to amuse that the term "abbrevCmd" is abbreviated
> heavily itself. However, I would strongly suggest to avoid that. It would
> be much more pleasant to call the config option rebase.abbreviateCommands

I tried to use something similar to the rest of the options but I guess that
would be best.

> 
> > +rebase.abbrevCmd::
> > +	If set to true, `git rebase -i` will abbreviate the command-names in the
> > +	instruction list. This means that instead of looking like this,
> 
> This is by no means your fault, but it is really horrible by how many
> different names Git's documentation refers to the todo script, nothing
> short of confusing. It is the todo script (which I called it initially,
> maybe not a good name, but it has the merit of the longest tradition at
> least), the todo list, the instruction sheet, the rebase script, the
> instruction list... etc
> 
> However, the thing is called "todo list" elsewhere in the same file,
> therefore lets try to avoid even more confusion and use that term instead
> of "instruction list" here.

thanks for pointing this out, I was not quite sure what to call this list.

> 
> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > index 2c9c0165b5ab..9f3e82b79615 100644
> > --- a/git-rebase--interactive.sh
> > +++ b/git-rebase--interactive.sh
> > @@ -1210,6 +1210,10 @@ else
> >  	revisions=$onto...$orig_head
> >  	shortrevisions=$shorthead
> >  fi
> > +
> > +rebasecmd=pick
> > +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p
> 
> A better name would be "pickcmd", as there are more rebase commands than
> just `pick` and what we want here is really only associated with one of
> those commands.

Wouldn't that make it confusing when the patch starts to handle other commands?
A common name across the script would limit further confusion.
I noticed that it is already called `action` in `rearrange_squash`.
would that do? (even though it has no reference to 'command')

> 
> Ciao,
> Johannes

Thanks for the detailed answer,
Liam

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Jeff King]

On Tue, Apr 25, 2017 at 08:13:27PM -0400, liam Beguin wrote:

> > > +rebase.abbrevCmd::
> > > +	If set to true, `git rebase -i` will abbreviate the command-names in the
> > > +	instruction list. This means that instead of looking like this,
> > 
> > This is by no means your fault, but it is really horrible by how many
> > different names Git's documentation refers to the todo script, nothing
> > short of confusing. It is the todo script (which I called it initially,
> > maybe not a good name, but it has the merit of the longest tradition at
> > least), the todo list, the instruction sheet, the rebase script, the
> > instruction list... etc
> > 
> > However, the thing is called "todo list" elsewhere in the same file,
> > therefore lets try to avoid even more confusion and use that term instead
> > of "instruction list" here.
> 
> thanks for pointing this out, I was not quite sure what to call this list.

I think the words "instruction list" may have come from my suggestion. I
used them because that is the term used in the rebase.instructionFormat
documentation directly above the option you are adding.

It may be worth a follow-on patch to convert that one to "todo list" if
that's the preferred name.

-Peff

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Junio C Hamano]

Jacob Keller <jacob.keller@gmail.com> writes:

> On Mon, Apr 24, 2017 at 11:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Personally I am happy with the beginning of each instruction line
>> aligned, so from that point of view, this patch is a mild Meh to me,
>> even though I do a fair amount of "rebase -i" myself.  But obviously
>> I am not the only user of Git you need to please, so...
>
> I would instead justify this as making it easier to change the action,
> since you only need to rewrite a single letter, which at least in vim
> takes "r<letter>" to change the action, vs slightly more keystrokes
> such as "ct <letter" or otherwise.

That makes sense to me too.

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I think the words "instruction list" may have come from my suggestion. I
> used them because that is the term used in the rebase.instructionFormat
> documentation directly above the option you are adding.
>
> It may be worth a follow-on patch to convert that one to "todo list" if
> that's the preferred name.

Running

$ git grep -i -e 'instruction [ls]' -e 'todo l'

lets us count how we call them, and we can see there is only one
instance of 'instruction list'.

Running the above in v1.7.3 tree shows that it was originally called
'todo list', and we can see that an enhancement of cherry-pick in
cd4093b6 ("Merge branch 'rr/revert-cherry-pick-continue'",
2011-10-05)) started calling this instruction sheet around v1.7.8.

A follow-on patch to unify all three would be nice, indeed.

Thanks.

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Johannes Schindelin]

Hi Junio,

On Tue, 25 Apr 2017, Junio C Hamano wrote:

> Running
> 
> $ git grep -i -e 'instruction [ls]' -e 'todo l'
> 
> lets us count how we call them, and we can see there is only one
> instance of 'instruction list'.
> 
> Running the above in v1.7.3 tree shows that it was originally called
> 'todo list', and we can see that an enhancement of cherry-pick in
> cd4093b6 ("Merge branch 'rr/revert-cherry-pick-continue'",
> 2011-10-05)) started calling this instruction sheet around v1.7.8.
> 
> A follow-on patch to unify all three would be nice, indeed.

But we cannot unify them, as the config option's name uses "instruction"
and to keep backwards-compatibility, we are simply unable to resolve the
confusion.

Ciao,
Dscho

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

Hi Liam,

On Tue, 25 Apr 2017, liam Beguin wrote:

> On Tue, 2017-04-25 at 22:08 +0200, Johannes Schindelin wrote:
> > 
> > On Tue, 25 Apr 2017, Liam Beguin wrote:
> > 
> > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > > index 2c9c0165b5ab..9f3e82b79615 100644
> > > --- a/git-rebase--interactive.sh
> > > +++ b/git-rebase--interactive.sh
> > > @@ -1210,6 +1210,10 @@ else
> > >  	revisions=$onto...$orig_head
> > >  	shortrevisions=$shorthead
> > >  fi
> > > +
> > > +rebasecmd=pick
> > > +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p
> > 
> > A better name would be "pickcmd", as there are more rebase commands than
> > just `pick` and what we want here is really only associated with one of
> > those commands.
> 
> Wouldn't that make it confusing when the patch starts to handle other
> commands?

Only if you use that variable to hold other values than `pick` or `p`. But
you do not plan on that, right? You plan to use this variable only to hold
the value `pick` by default and `p` in case the user asked for abbreviated
commands. Therefore, I think it makes sense to reflect in the variable
name that the purpose is really only to reflect the string used for the
`pick` command (as opposed to any other todo command).

Ciao,
Johannes

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Ævar Arnfjörð Bjarmason]

On Tue, Apr 25, 2017 at 6:43 AM, Liam Beguin <liambeguin@gmail.com> wrote:
> Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i`
> to abbreviate the command-names in the instruction list.
>
> This means that `git rebase -i` would print:
>     p deadbee The oneline of this commit
>     ...
>
> instead of:
>     pick deadbee The oneline of this commit
>     ...
>
> Using a single character command-name allows the lines to remain
> aligned, making the whole set more readable.

Aside from the existing comments about the commit message from others,
you should be noting that we *already* have these abbreviations for
all the todo list options, and we note this in append_todo_help.


> +rebase.abbrevCmd::
> +       If set to true, `git rebase -i` will abbreviate the command-names in the
> +       instruction list. This means that instead of looking like this,
> +
> [...]
> +rebase.abbrevCmd::
> +       If set to true, `git rebase -i` will abbreviate the command-names in the
> +       instruction list. This means that instead of looking like this,
> [...]

Better to split this out into a new *.txt file and use the include::*
facility (grep for it) rather than copy/pasting this entirely across
two files.

>  OPTIONS
>  -------
>  --onto <newbase>::
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 2c9c0165b5ab..9f3e82b79615 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -1210,6 +1210,10 @@ else
>         revisions=$onto...$orig_head
>         shortrevisions=$shorthead
>  fi
> +
> +rebasecmd=pick
> +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p

Rather than hardcoding "p" here maybe it would be worthhwile to make
that into a variable used both here and in append_todo_help, maybe
not...

>  format=$(git config --get rebase.instructionFormat)
>  # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
>  git rev-list $merges_option --format="%m%H ${format:-%s}" \
> @@ -1228,7 +1232,7 @@ do
>
>         if test t != "$preserve_merges"
>         then
> -               printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
> +               printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
>         else
>                 if test -z "$rebase_root"
>                 then
> @@ -1246,7 +1250,7 @@ do
>                 if test f = "$preserve"
>                 then
>                         touch "$rewritten"/$sha1
> -                       printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
> +                       printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
>                 fi
>         fi
>  done

I haven't tried applying & running this patch, but it seems you
definitely missed the case where --autosquash will add fixup or
squash, that should be f or s with your patch, but you didn't change
that code. See the rearrange_squash function.

Ditto for turning "exec" into "e" with --exec.

But if the motivation for this entire thing is to make sure the
commands are aligned this doesn't fix that, because the sha1s can be
of different lengths. So as others have pointed out maybe this entire
thing should be dropped & replaced with some bool command to align the
todo list, maybe turning that on by default.

Unless the real unstated reason is to make this easier to edit in vim
or something, in which case this approach seems reasonable.

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Junio,
>
> On Tue, 25 Apr 2017, Junio C Hamano wrote:
>
>> Running
>> 
>> $ git grep -i -e 'instruction [ls]' -e 'todo l'
>> 
>> lets us count how we call them, and we can see there is only one
>> instance of 'instruction list'.
>> 
>> Running the above in v1.7.3 tree shows that it was originally called
>> 'todo list', and we can see that an enhancement of cherry-pick in
>> cd4093b6 ("Merge branch 'rr/revert-cherry-pick-continue'",
>> 2011-10-05)) started calling this instruction sheet around v1.7.8.
>> 
>> A follow-on patch to unify all three would be nice, indeed.
>
> But we cannot unify them, as the config option's name uses "instruction"
> and to keep backwards-compatibility, we are simply unable to resolve the
> confusion.

We can correct historical mistakes by introducing preferred synonym
to misnamed configuration variables, clearly document why we prefer
it over the misnamed one that is now deprecated, and then eventually
dropping it at a major version boundary.

Re: [PATCH v2] rebase -i: add config to abbreviate command-names

[liam Beguin]

Hi Ævar,

On Wed, 2017-04-26 at 17:24 +0200, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Apr 25, 2017 at 6:43 AM, Liam Beguin <liambeguin@gmail.com> wrote:
> > Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i`
> > to abbreviate the command-names in the instruction list.
> > 
> > This means that `git rebase -i` would print:
> >     p deadbee The oneline of this commit
> >     ...
> > 
> > instead of:
> >     pick deadbee The oneline of this commit
> >     ...
> > 
> > Using a single character command-name allows the lines to remain
> > aligned, making the whole set more readable.
> 
> Aside from the existing comments about the commit message from others,
> you should be noting that we *already* have these abbreviations for
> all the todo list options, and we note this in append_todo_help.
> 
> 
> > +rebase.abbrevCmd::
> > +       If set to true, `git rebase -i` will abbreviate the command-names in the
> > +       instruction list. This means that instead of looking like this,
> > +
> > [...]
> > +rebase.abbrevCmd::
> > +       If set to true, `git rebase -i` will abbreviate the command-names in the
> > +       instruction list. This means that instead of looking like this,
> > [...]
> 
> Better to split this out into a new *.txt file and use the include::*
> facility (grep for it) rather than copy/pasting this entirely across
> two files.
> 

Thanks for pointing this out, I'll update the documentation

> >  OPTIONS
> >  -------
> >  --onto <newbase>::
> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > index 2c9c0165b5ab..9f3e82b79615 100644
> > --- a/git-rebase--interactive.sh
> > +++ b/git-rebase--interactive.sh
> > @@ -1210,6 +1210,10 @@ else
> >         revisions=$onto...$orig_head
> >         shortrevisions=$shorthead
> >  fi
> > +
> > +rebasecmd=pick
> > +test "$(git config --bool --get rebase.abbrevCmd)" = true && rebasecmd=p
> 
> Rather than hardcoding "p" here maybe it would be worthhwile to make
> that into a variable used both here and in append_todo_help, maybe
> not...
> 

I'm not sure I understand, do you mean that the option should also affect the
message added by append_todo_help ?

> >  format=$(git config --get rebase.instructionFormat)
> >  # the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse
> >  git rev-list $merges_option --format="%m%H ${format:-%s}" \
> > @@ -1228,7 +1232,7 @@ do
> > 
> >         if test t != "$preserve_merges"
> >         then
> > -               printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
> > +               printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
> >         else
> >                 if test -z "$rebase_root"
> >                 then
> > @@ -1246,7 +1250,7 @@ do
> >                 if test f = "$preserve"
> >                 then
> >                         touch "$rewritten"/$sha1
> > -                       printf '%s\n' "${comment_out}pick $sha1 $rest" >>"$todo"
> > +                       printf '%s\n' "${comment_out}${rebasecmd} $sha1 $rest" >>"$todo"
> >                 fi
> >         fi
> >  done
> 
> I haven't tried applying & running this patch, but it seems you
> definitely missed the case where --autosquash will add fixup or
> squash, that should be f or s with your patch, but you didn't change
> that code. See the rearrange_squash function.
> 
> Ditto for turning "exec" into "e" with --exec.
> 

I noticed this yesterday, I'll add both cases the next iteration.

> But if the motivation for this entire thing is to make sure the
> commands are aligned this doesn't fix that, because the sha1s can be
> of different lengths. So as others have pointed out maybe this entire
> thing should be dropped & replaced with some bool command to align the
> todo list, maybe turning that on by default.
> 
> Unless the real unstated reason is to make this easier to edit in vim
> or something, in which case this approach seems reasonable.

Keeping things aligned was the first motivation but the fact that it also
makes changing the action faster is also nice to have. I didn't think it
would help justify the patch.
The SHA1s not having the same length can easily be 'fixed' by setting a
higher value for 'core.abbrev'. 

Thanks, 
Liam