Re: [PATCH] completion: optionally disable checkout DWIM

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Fri, Apr 21, 2017 at 10:19 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Apr 21, 2017 at 10:14:48PM +0200, SZEDER Gábor wrote:
>
>> >> This is flexible enough for me, but it's possible somebody would want
>> >> this on a per-repo basis. I don't know that we want to read from `git
>> >> config`, though, because it's relatively expensive to do so. People who
>> >> want per-repo settings are probably better off with a hook that triggers
>> >> when they "cd" around, and sets up their preferences.
>>
>> We could discern between more than just empty vs. non-empty state of
>> the environment variable, e.g.:
>>
>>   - if empty/unset, then include "DWIM" suggestions.
>>   - if set to 'config', then query the 'completion.checkoutNoGuess'
>>     configuration variable, and omit "DWIM" suggestions if its true.
>>   - if set to something else, then omit "DWIM" suggestions.
>>
>> Then users can themselves decide, whether the per-repo configurability
>> is worth the overhead of running 'git config'.
>
> Yep, that would work. I wasn't going to bother with that complexity
> unless somebody really wanted it. The important thing is not to paint
> ourselves into a corner. By the rules you gave above, it would probably
> be fine to extend my patch later to match. But we could also be more
> specific (e.g., look for some positive value like "1").

That's fine with me.

We should have done this when we introduced env variables controlling
the prompt script, we could spare a 'git config' execution in a
subshell or two.

>> >> @@ -1248,7 +1256,8 @@ _git_checkout ()
>> >>               # check if --track, --no-track, or --no-guess was specified
>> >>               # if so, disable DWIM mode
>> >>               local flags="--track --no-track --no-guess" track_opt="--track"
>> >> -             if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
>> >> +             if [ -n "$GIT_COMPLETION_CHECKOUT_NO_GUESS" -o \
>> >> +                  -n "$(__git_find_on_cmdline "$flags")" ]; then
>>
>> || would be better than '-o', because the former short-circuits when
>> the first condition is true, but the latter doesn't.
>
> Ah, I didn't know that. Usually I use "||", but I thought "-o" was
> generally preferred in bash-specific scripts. We definitely want to
> short circuit here.

No, we use || and && fairly consistently in conditions in Bash
scripts.  There is no '-o' in the completion or prompt scripts, and
there is only a single '-a' in the former, but it only checks two
one-letter variables, so it doesn't matter.  (Besides, it's from my
second ever commit, so it has great sentimental value ;)

There are some inconsistencies using 'if [ ... ]' and 'if [[ ... ]]',
but oh well.


Anyway, v2 looks good to me.