On Tue, Apr 25, 2017 at 6:43 AM, Liam Beguin <liambeguin@xxxxxxxxx> 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.