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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- 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