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.
next prev parent 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 3/8] t7810: do not abbreviate `--no-exclude-standard` nor `--invert-match` 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 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