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 [off-list ref] wrote:

quoted

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.

quoted

+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

quoted

 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 ?

quoted

 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