All of lore.kernel.org
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Glen Choo via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	"Derrick Stolee" <derrickstolee@github.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Emily Shaffer" <emilyshaffer@google.com>,
	"Jonathan Tan" <jonathantanmy@google.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Glen Choo" <chooglen@google.com>
Subject: Re: [PATCH v6 1/5] Documentation/git-config.txt: add SCOPES section
Date: Thu, 30 Jun 2022 18:32:11 -0400	[thread overview]
Message-ID: <Yr4ka485NzadFTto@nand.local> (raw)
In-Reply-To: <ee9619f6ec0608f399fc924cfe9254df5e7bc431.1656612839.git.gitgitgadget@gmail.com>

On Thu, Jun 30, 2022 at 06:13:55PM +0000, Glen Choo via GitGitGadget wrote:
> From: Glen Choo <chooglen@google.com>
>
> In a subsequent commit, we will introduce "protected configuration",
> which is easiest to describe in terms of configuration scopes (i.e. it's
> the union of the 'system', 'global', and 'command' scopes). This
> description is fine for ML discussions, but it's inadequate for end
> users because we don't provide a good description of "configuration
> scopes" in the public docs.
>
> 145d59f482 (config: add '--show-scope' to print the scope of a config
> value, 2020-02-10) introduced the word "scope" to our public docs, but
> that only enumerates the scopes and assumes the user can figure out
> those values mean.

Thanks, I think that "scope" is an appropriate term here. When I
originally read this patch, I was thinking that "origin" would be more
appropriate, since I was recalling the `--show-origin` option to `git
config`. But that shows the file name, and `--show-scope` is a separate
option entirely.

The latter is definitely more appropriate here, so I think this choice
of naming is good and makes sense.

> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> index 9376e39aef2..f93d437b898 100644
> --- a/Documentation/git-config.txt
> +++ b/Documentation/git-config.txt
> @@ -297,8 +297,8 @@ The default is to use a pager.
>  FILES
>  -----
>
> -If not set explicitly with `--file`, there are four files where
> -'git config' will search for configuration options:
> +By default, 'git config' will read configuration options from multiple
> +files:
>
>  $(prefix)/etc/gitconfig::
>  	System-wide configuration file.
> @@ -322,27 +322,63 @@ $GIT_DIR/config.worktree::
>  	This is optional and is only searched when
>  	`extensions.worktreeConfig` is present in $GIT_DIR/config.
>
> -If no further options are given, all reading options will read all of these
> -files that are available. If the global or the system-wide configuration
> -file are not available they will be ignored. If the repository configuration
> -file is not available or readable, 'git config' will exit with a non-zero
> -error code. However, in neither case will an error message be issued.
> +You may also provide additional configuration parameters when running any
> +git command by using the `-c` option. See linkgit:git[1] for details.
> +
> +Options will be read from all of these files that are available. If the
> +global or the system-wide configuration file are not available they will be
> +ignored. If the repository configuration file is not available or readable,
> +'git config' will exit with a non-zero error code. However, in neither case
> +will an error message be issued.

Nit: the last sentence is a little awkwardly worded. Perhaps just:
"Note that neither case produces an error message".

> -All writing options will per default write to the repository specific
> +By default, options are only written to the repository specific
>  configuration file. Note that this also affects options like `--replace-all`

Should we mention that this is the same as the "local" scope below?

>  and `--unset`. *'git config' will only ever change one file at a time*.
>
> -You can override these rules using the `--global`, `--system`,
> -`--local`, `--worktree`, and `--file` command-line options; see
> -<<OPTIONS>> above.
> +You can change the way options are read/written by specifying the path to a
> +file (`--file`), or by specifying a configuration scope (`--system`,
> +`--global`, `--local`, `--worktree`); see <<OPTIONS>> above.

I think this paragraph could be slightly more descriptive about what
`--file` does while still linking out to <<OPTIONS>> above for more
detailed information. In the pre-image, we say:

    If not set explicitly with `--file`, there are four files will `git
    config will search`.

So I wonder if something more descriptive in this section might be:

    You can limit which configuration sources are read to or written
    from by specifying the path of a file with the `--file` option, or
    by specifying a scope with `--system`, `--global`, `--local`, or
    `--worktree`. For more, see <<OPTIONS>> above.

I don't think that's so different form what you wrote, but I think it's
a little clearer particularly what `--file` does (instead of "change the
way options are read/written" it "limit[s] which configuration sources
are read to or written from").

> +
> +SCOPES
> +------
> +
> +Each configuration source falls within a configuration scope. The scopes
> +are:
> +
> +system::
> +	$(prefix)/etc/gitconfig
> +
> +global::
> +	$XDG_CONFIG_HOME/git/config
> ++
> +~/.gitconfig
> +
> +local::
> +	$GIT_DIR/config
> +
> +worktree::
> +	$GIT_DIR/config.worktree
> +
> +command::
> +	environment variables
> ++
> +the `-c` option
> +
> +With the exception of 'command', each scope corresponds to a command line
> +option - `--system`, `--global`, `--local`, `--worktree`.

I think a colon after "option" is more appropriate than a single "-"
dash character, but this is definitely a trivial matter that I have no
strong opinion on.

One thing that this reminds me of (which I don't think is worth taking
up here, but perhaps in a future series, or as #leftoverbits) would be
promoting these scopes behind a single option. Back in the day, you
could ask for values out of `git config` by specifying their type with
`--int`, `--bool`, or similar. In e3e042b185 (Merge branch
'tb/config-type', 2018-05-08), we changed to
`--type=<int|bool|color|etc>`, which unified things and made it clearer
which options were grouped together by a single concept.

I think a similar change would make sense here, that is to replace
`--system`, `--global` (and so on) with `--scope=system`,
`--scope=global`, etc.

But that's not material to this series, and just something to think
about for later on if you end up thinking it's a good idea.

> +
> +When reading options, specifying a scope will only read options from the
> +files within that scope. When writing options, specifying a scope will write
> +to the files within that scope (instead of the repository specific
> +configuration file). See <<OPTIONS>> above for a complete description.
>
> +Most configuration options are respected regardless of the scope it is
> +defined in, but some options are only respected in certain scopes. See the
> +option's documentation for the full details.

I assume "the option's" is referring to whichever configuration variable
we're talking about. So it may be clearer to say "See the *respective*
option's documentation for more information" or similar.

Thanks,
Taylor

  reply	other threads:[~2022-06-30 22:32 UTC|newest]

Thread overview: 113+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-06 18:30 [PATCH] [RFC] setup.c: make bare repo discovery optional Glen Choo via GitGitGadget
2022-05-06 20:33 ` Junio C Hamano
2022-05-09 21:42 ` Taylor Blau
2022-05-09 22:54   ` Junio C Hamano
2022-05-09 23:57     ` Taylor Blau
2022-05-10  0:23       ` Junio C Hamano
2022-05-10 22:00   ` Glen Choo
2022-05-13 23:37 ` [PATCH v2 0/2] " Glen Choo via GitGitGadget
2022-05-13 23:37   ` [PATCH v2 1/2] " Glen Choo via GitGitGadget
2022-05-16 18:12     ` Glen Choo
2022-05-16 18:46     ` Derrick Stolee
2022-05-16 22:25       ` Taylor Blau
2022-05-17 20:24       ` Glen Choo
2022-05-17 21:51         ` Glen Choo
2022-05-13 23:37   ` [PATCH v2 2/2] setup.c: learn discovery.bareRepository=cwd Glen Choo via GitGitGadget
2022-05-16 18:49     ` Derrick Stolee
2022-05-16 16:40   ` [PATCH v2 0/2] setup.c: make bare repo discovery optional Junio C Hamano
2022-05-16 18:36     ` Glen Choo
2022-05-16 19:16       ` Junio C Hamano
2022-05-16 20:27         ` Glen Choo
2022-05-16 22:16           ` Junio C Hamano
2022-05-16 16:43   ` Junio C Hamano
2022-05-16 19:07   ` Derrick Stolee
2022-05-16 22:43     ` Taylor Blau
2022-05-16 23:19     ` Junio C Hamano
2022-05-17 18:56     ` Glen Choo
2022-05-27 21:09   ` [PATCH v3 0/5] config: introduce discovery.bare and protected config Glen Choo via GitGitGadget
2022-05-27 21:09     ` [PATCH v3 1/5] Documentation: define protected configuration Glen Choo via GitGitGadget
2022-05-27 23:29       ` Junio C Hamano
2022-06-02 12:42         ` Derrick Stolee
2022-06-02 16:53           ` Junio C Hamano
2022-06-02 17:39             ` Glen Choo
2022-06-03 15:57         ` Glen Choo
2022-05-27 21:09     ` [PATCH v3 2/5] config: read protected config with `git_protected_config()` Glen Choo via GitGitGadget
2022-05-28  0:28       ` Junio C Hamano
2022-05-31 17:43         ` Glen Choo
2022-06-01 15:58           ` Junio C Hamano
2022-06-02 12:56       ` Derrick Stolee
2022-05-27 21:09     ` [PATCH v3 3/5] setup.c: create `discovery.bare` Glen Choo via GitGitGadget
2022-05-28  0:59       ` Junio C Hamano
2022-06-02 13:11       ` Derrick Stolee
2022-05-27 21:09     ` [PATCH v3 4/5] config: include "-c" in protected config Glen Choo via GitGitGadget
2022-06-02 13:15       ` Derrick Stolee
2022-05-27 21:09     ` [PATCH v3 5/5] upload-pack: make uploadpack.packObjectsHook protected Glen Choo via GitGitGadget
2022-06-02 13:18       ` Derrick Stolee
2022-06-07 20:57     ` [PATCH v4 0/5] config: introduce discovery.bare and protected config Glen Choo via GitGitGadget
2022-06-07 20:57       ` [PATCH v4 1/5] Documentation/git-config.txt: add SCOPES section Glen Choo via GitGitGadget
2022-06-07 20:57       ` [PATCH v4 2/5] Documentation: define protected configuration Glen Choo via GitGitGadget
2022-06-22 21:58         ` Jonathan Tan
2022-06-23 18:21           ` Glen Choo
2022-06-07 20:57       ` [PATCH v4 3/5] config: read protected config with `git_protected_config()` Glen Choo via GitGitGadget
2022-06-07 22:49         ` Junio C Hamano
2022-06-08  0:22           ` Glen Choo
2022-06-07 20:57       ` [PATCH v4 4/5] safe.directory: use git_protected_config() Glen Choo via GitGitGadget
2022-06-07 20:57       ` [PATCH v4 5/5] setup.c: create `discovery.bare` Glen Choo via GitGitGadget
2022-06-07 21:37         ` Glen Choo
2022-06-22 22:03       ` [PATCH v4 0/5] config: introduce discovery.bare and protected config Jonathan Tan
2022-06-23 17:13         ` Glen Choo
2022-06-23 18:32           ` Junio C Hamano
2022-06-27 17:34             ` Glen Choo
2022-06-27 18:19       ` Glen Choo
2022-06-27 18:36       ` [PATCH v5 " Glen Choo via GitGitGadget
2022-06-27 18:36         ` [PATCH v5 1/5] Documentation/git-config.txt: add SCOPES section Glen Choo via GitGitGadget
2022-06-27 18:36         ` [PATCH v5 2/5] Documentation: define protected configuration Glen Choo via GitGitGadget
2022-06-27 18:36         ` [PATCH v5 3/5] config: learn `git_protected_config()` Glen Choo via GitGitGadget
2022-06-27 18:36         ` [PATCH v5 4/5] safe.directory: use git_protected_config() Glen Choo via GitGitGadget
2022-06-27 18:36         ` [PATCH v5 5/5] setup.c: create `discovery.bare` Glen Choo via GitGitGadget
2022-06-30 13:20           ` Ævar Arnfjörð Bjarmason
2022-06-30 17:28             ` Glen Choo
2022-06-30 18:13         ` [PATCH v6 0/5] config: introduce discovery.bare and protected config Glen Choo via GitGitGadget
2022-06-30 18:13           ` [PATCH v6 1/5] Documentation/git-config.txt: add SCOPES section Glen Choo via GitGitGadget
2022-06-30 22:32             ` Taylor Blau [this message]
2022-07-06 17:44               ` Glen Choo
2022-06-30 18:13           ` [PATCH v6 2/5] Documentation: define protected configuration Glen Choo via GitGitGadget
2022-06-30 23:49             ` Taylor Blau
2022-07-06 18:21               ` Glen Choo
2022-06-30 18:13           ` [PATCH v6 3/5] config: learn `git_protected_config()` Glen Choo via GitGitGadget
2022-07-01  1:22             ` Taylor Blau
2022-07-06 22:42               ` Glen Choo
2022-06-30 18:13           ` [PATCH v6 4/5] safe.directory: use git_protected_config() Glen Choo via GitGitGadget
2022-06-30 18:13           ` [PATCH v6 5/5] setup.c: create `discovery.bare` Glen Choo via GitGitGadget
2022-07-01  1:30             ` Taylor Blau
2022-07-07 19:55               ` Glen Choo
2022-06-30 22:13           ` [PATCH v6 0/5] config: introduce discovery.bare and protected config Taylor Blau
2022-06-30 23:07           ` Ævar Arnfjörð Bjarmason
2022-07-01 17:37             ` Glen Choo
2022-07-08 21:58               ` Ævar Arnfjörð Bjarmason
2022-07-12 20:47                 ` Glen Choo
2022-07-12 23:53                   ` Ævar Arnfjörð Bjarmason
2022-07-07 23:01           ` [PATCH v7 " Glen Choo via GitGitGadget
2022-07-07 23:01             ` [PATCH v7 1/5] Documentation/git-config.txt: add SCOPES section Glen Choo via GitGitGadget
2022-07-07 23:43               ` Junio C Hamano
2022-07-08 17:01                 ` Glen Choo
2022-07-08 19:01                   ` Junio C Hamano
2022-07-08 21:38                     ` Glen Choo
2022-07-07 23:01             ` [PATCH v7 2/5] Documentation: define protected configuration Glen Choo via GitGitGadget
2022-07-08  0:39               ` Junio C Hamano
2022-07-07 23:01             ` [PATCH v7 3/5] config: learn `git_protected_config()` Glen Choo via GitGitGadget
2022-07-07 23:01             ` [PATCH v7 4/5] safe.directory: use git_protected_config() Glen Choo via GitGitGadget
2022-07-07 23:01             ` [PATCH v7 5/5] setup.c: create `discovery.bare` Glen Choo via GitGitGadget
2022-07-08  1:07             ` [PATCH v7 0/5] config: introduce discovery.bare and protected config Junio C Hamano
2022-07-08 20:35               ` Glen Choo
2022-07-12 22:11                 ` Glen Choo
2022-07-14 21:27             ` [PATCH v8 0/5] config: introduce safe.bareRepository " Glen Choo via GitGitGadget
2022-07-14 21:27               ` [PATCH v8 1/5] Documentation/git-config.txt: add SCOPES section Glen Choo via GitGitGadget
2022-07-14 21:27               ` [PATCH v8 2/5] Documentation: define protected configuration Glen Choo via GitGitGadget
2022-07-14 21:27               ` [PATCH v8 3/5] config: learn `git_protected_config()` Glen Choo via GitGitGadget
2022-07-25 18:26                 ` SANITIZE=address failure on master (was: [PATCH v8 3/5] config: learn `git_protected_config()`) Ævar Arnfjörð Bjarmason
2022-07-25 20:15                   ` Glen Choo
2022-07-25 20:41                     ` Ævar Arnfjörð Bjarmason
2022-07-25 20:56                       ` Glen Choo
2022-07-14 21:28               ` [PATCH v8 4/5] safe.directory: use git_protected_config() Glen Choo via GitGitGadget
2022-07-14 21:28               ` [PATCH v8 5/5] setup.c: create `safe.bareRepository` Glen Choo via GitGitGadget

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

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

  git send-email \
    --in-reply-to=Yr4ka485NzadFTto@nand.local \
    --to=me@ttaylorr.com \
    --cc=avarab@gmail.com \
    --cc=chooglen@google.com \
    --cc=derrickstolee@github.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    --cc=sandals@crustytoothpaste.net \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.