git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Git List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Denton Liu <liu.denton@gmail.com>
Subject: Re: [PATCH 1/2] parse-options: allow for configuring option abbreviation
Date: Tue, 26 Mar 2019 07:28:58 +0100
Message-ID: <87lg12b13p.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <CACsJy8Bk=Z8BaVeAhKzF4PWYLLG76cADooHKNw+Xy3EztZL1DQ@mail.gmail.com>


On Tue, Mar 26 2019, Duy Nguyen wrote:

> On Tue, Mar 26, 2019 at 5:48 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>>
>> On Mon, Mar 25 2019, Eric Sunshine wrote:
>>
>> > 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.

FWIW I replied so quickly to this with patches since I had something WIP
to do this, it's annoyed me as a user in the past that I couldn't turn
this off, IIRC some upgrade of git broke my "bad" muscle memory /
scripts.

> If this is mostly useful for scripts then I agree with Eric an
> environment variable is the way to go. A configuration variable does
> not make it more convenient.

I think both of you might be assuming that when you want to configure
something it's as easy to tweak every run time environment (set an env
var) as it is to set a global config. See the trace2.* config discussion
for similar use-cases.

> And no we don't only export via config. There are a bunch of public
> env variables in git.txt. "core" namespace is already very crowded. If
> this one is only rarely used, I'd rather not add a new config
> variable.

I don't see how a new "abbreviatedOptions" is plausibly going to crowd
out anything else, sounds pretty unambiguous to me.

By my count out of the the existing GIT_* variables in git.txt around
1/3 are already configurable via config, another 1/3 (all the GIT_TRACE
stuff) is something we've wanted to have configurable in the past (but
nobody's gotten around to writing patches for).

I think it's fair to say that when we normally add user-configurable
stuff we do it as config, not as new env vars.

  reply	other threads:[~2019-03-26  6:29 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-25 18:14 [PATCH 0/8] Do not use abbreviated options in tests Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 1/8] tests (rebase): spell out the `--keep-empty` option Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 2/8] tests (rebase): spell out the `--force-rebase` option Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 3/8] t7810: do not abbreviate `--no-exclude-standard` nor `--invert-match` Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 4/8] t5531: avoid using an abbreviated option Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 5/8] tests (push): do not abbreviate the `--follow-tags` option Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 6/8] tests (status): spell out the `--find-renames` option in full Johannes Schindelin via GitGitGadget
2019-03-25 18:14 ` [PATCH 8/8] tests: disallow the use of abbreviated options (by default) Johannes Schindelin via GitGitGadget
2019-03-25 18:35   ` Denton Liu
2019-03-25 20:26     ` Ævar Arnfjörð Bjarmason
2019-04-12  8:48       ` Johannes Schindelin
2019-04-12  8:50     ` Johannes Schindelin
2019-03-25 19:47   ` Ævar Arnfjörð Bjarmason
2019-04-12  8:59     ` Johannes Schindelin
2019-03-25 18:14 ` [PATCH 7/8] tests (pack-objects): use the full, unabbreviated `--revs` option Johannes Schindelin via GitGitGadget
2019-03-25 20:23 ` [PATCH 0/2] allow for configuring option abbreviation + fix Ævar Arnfjörð Bjarmason
2019-03-25 20:23 ` [PATCH 1/2] parse-options: allow for configuring option abbreviation Ævar Arnfjörð Bjarmason
2019-03-25 21:23   ` Eric Sunshine
2019-03-25 22:47     ` Ævar Arnfjörð Bjarmason
2019-03-26  4:14       ` Duy Nguyen
2019-03-26  6:28         ` Ævar Arnfjörð Bjarmason [this message]
2019-03-26  7:13           ` Duy Nguyen
2019-03-26 11:00             ` Ævar Arnfjörð Bjarmason
2019-04-01 10:47     ` Junio C Hamano
2019-04-12  9:06       ` Johannes Schindelin
2019-03-25 20:23 ` [PATCH 2/2] parse-options: don't emit "ambiguous option" for aliases Ævar Arnfjörð Bjarmason
2019-04-17 12:44   ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2019-04-17 16:04     ` Duy Nguyen
2019-04-18  0:48       ` Junio C Hamano
2019-04-18  9:29         ` Duy Nguyen
2019-04-19  4:39           ` Junio C Hamano
2019-04-22 12:22             ` [PATCH] " Nguyễn Thái Ngọc Duy
2019-04-22 12:34               ` Duy Nguyen
2019-04-29 10:05               ` [PATCH v2] " Nguyễn Thái Ngọc Duy
2019-05-07  3:43                 ` Junio C Hamano
2019-05-07 11:58                   ` Duy Nguyen
2019-04-02  0:58 ` [PATCH 0/8] Do not use abbreviated options in tests Junio C Hamano
2019-04-12  9:37 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 1/8] tests (rebase): spell out the `--keep-empty` option Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 2/8] tests (rebase): spell out the `--force-rebase` option Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 3/8] t7810: do not abbreviate `--no-exclude-standard` nor `--invert-match` Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 4/8] t5531: avoid using an abbreviated option Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 5/8] tests (push): do not abbreviate the `--follow-tags` option Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 7/8] tests (pack-objects): use the full, unabbreviated `--revs` option Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 6/8] tests (status): spell out the `--find-renames` option in full Johannes Schindelin via GitGitGadget
2019-04-12  9:37   ` [PATCH v2 8/8] tests: disallow the use of abbreviated options (by default) Johannes Schindelin via GitGitGadget
2019-04-14  2:35     ` Junio C Hamano
2019-04-15  2:55       ` Junio C Hamano
2019-04-15 13:09         ` Johannes Schindelin
2019-04-15 13:45           ` Junio C Hamano
2019-04-17 12:07             ` Johannes Schindelin
2019-04-15 13:08       ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lg12b13p.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=liu.denton@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git