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