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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 26, 2017, 09:28 UTC
Message-ID: <alpine.DEB.2.20.1704261126130.3480@virtualbox>
In-Reply-To: <1493165607.29673.31.camel@gmail.com>

Hi Liam,

On Tue, 25 Apr 2017, liam Beguin wrote:

Show 22 quoted lines

> 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