Re: [PATCH 1/2] parse-options: allow for configuring option abbreviation

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 25, 2019, 22:47 UTC
Message-ID: <87o95ybmgf.fsf@evledraar.gmail.com>
In-Reply-To: <CAPig+cR0Ldt3EpQ683ZFNFXggfsTrdeZ3R-V6pDBZNA1N3c+xg@mail.gmail.com>

On Mon, Mar 25 2019, Eric Sunshine wrote:

Show 33 quoted lines

> On Mon, Mar 25, 2019 at 4:23 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
>> @@ -1,3 +1,15 @@
>> +core.abbreviatedOptions::
>> +       Defaults to `true` which allows options to be abbreviated as
>> +       long as they aren't ambiguous, e.g. for linkgit:git-init[1]
>> +       the `--bare` option can be abbreviated as `--bar`, `--ba` or
>> +       even `--b` since no other option starts with those
>> +       prefixes. However, if such an option were added in the future
>> +       any use of these abbreviations would break.
>> ++
>> +By setting this to false (e.g. in scripts) you can defend against such
>> +future breakages by enforcing that options must always be fully
>> +provided.
>
> I don't get why having a configuration option is better for defending
> scripts against this problem than a simple environment variable. It
> seems easier for the script prologue to contain:
>
>     GIT_TEST_ABBREVIATED_OPTIONS=false
>     export GIT_TEST_ABBREVIATED_OPTIONS
>
> than for it to muck about with git-config or use "git -c
> core.abbreviatedOptions=false ..." everywhere. The commit message
> doesn't do a good enough job of justifying the configuration option
> over the environment variable.
>
> Also, if this is now intended to be more general (aiding script
> writers) than just being for our test suite, then dropping "TEST" from
> the name seems warranted:
>
>     GIT_ABBREVIATED_OPTIONS

If we want to make something user-configurable we tend to add config variables. The GIT_TEST_* variables are only intended for our own test suite, see t/README.

I don't mind documenting this, but it's a well-established pattern, so if we're going to describe how this works/why use one or the other it should probably be some other series to t/README and/or git-config.txt

We traditionally *only* expose this sort of thing to users via config, and not via env variables.

The config system is more flexible in every way. You can set it system-wide, user-wide, repo-wide etc., and if you want exactly the scope of an env variable you can do that too, just start your script with:

    # These "''" quotes are not a mistake, it needs to be like this
    export GIT_CONFIG_PARAMETERS="'core.abbreviatedOptions=false'"
    git <some-cmd> [...]

So the reason we have GIT_TEST_* is pretty much because we can't just whitelist GIT_CONFIG_PARAMETERS for the test suite, and to make it obvious what test modes we have available.