config: allow user to know scope of config options #478

gitgitgadget · 2019-12-18T19:48:13Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > -static int end_null; > +static int end_nul; > - OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")), > + OPT_BOOL('z', "null", &end_nul, N_("terminate values with NUL byte")), > - const char term = end_null ? '\0' : '\t'; > + const char term = end_nul ? '\0' : '\t'; > - if (end_null) > + if (end_nul) These are correct changes, but is unrelated noise in the context of the theme of the patch, no? > +static const char *scope_to_string(enum config_scope scope) { > + /* > + * --local, --global, and --system work the same as --file so there's > + * no easy way for the parser to tell the difference when it is > + * setting the scope, so we use our information about which options > + * were passed > + */ > + if (use_local_config || scope == CONFIG_SCOPE_REPO) { > + return "local"; > + } else if (use_global_config || scope == CONFIG_SCOPE_GLOBAL) { > + return "global"; > + } else if (use_system_config || scope == CONFIG_SCOPE_SYSTEM) { > + return "system"; The above is slightly tricky; --global/--system/--local are made mutually exclusive in the higher level, so if any of them is set, we do not even need to look at the "scope" to tell what kind of source we are reading from. > + } else if (given_config_source.use_stdin || > + given_config_source.blob || > + given_config_source.file || > + scope == CONFIG_SCOPE_CMDLINE) { > + return "command line"; I am not sure what the implication of saying "they came from the command line" when we read from the standard input or from a blob. > + } else { > + return "unknown"; > + } > +} In any case, the need for such logic that says "scope might not say it is REPO, but when use_local_config is true, we are doing a local config" implies that "scope" parameter the caller of this function has is not set correctly when these options are used---would that be the real bug that needs fixing, rather than getting "worked around" with a code like this? It almost makes me point fingers at config.c::config_with_options() where config_source is inspected and git_config_from_*() helpers are called without setting the current_parsing_scope. Unlike these cases, when do_git_config_sequence() is called from that function, the scope is recorded in the variable before each standard config source file is opened and read. What would we be breaking if we taught the function to set the current_parsing_scope variable correctly even when reading from the config_source? That would certainly simplify this function quite a lot, but if the other parts of the codebase relies on the current behaviour, we cannot make such a change lightly. > +static void show_config_scope(struct strbuf *buf) > +{ > + const char term = end_nul ? '\0' : '\t'; > + const char *scope = scope_to_string(current_config_scope()); > + > + strbuf_addch(buf, '('); > + if (end_nul) > + strbuf_addstr(buf, N_(scope)); > + else > + quote_c_style(scope, buf, NULL, 0); Isn't this overkill? I think this code was copied-and-pasted from the other function that needs to show an arbitrary end-user supplied data which is a pathname, so it makes perfect sense to use c-style quoting, but the token scope_to_string() returns is taken from a bounded set that doesn't require such quoting, no? > + strbuf_addch(buf, ')'); > + strbuf_addch(buf, term); > +} > +

On the Git mailing list, Jeff King wrote (reply to this):

On Wed, Dec 18, 2019 at 11:46:06AM -0800, Junio C Hamano wrote: > It almost makes me point fingers at config.c::config_with_options() > where config_source is inspected and git_config_from_*() helpers are > called without setting the current_parsing_scope. Unlike these > cases, when do_git_config_sequence() is called from that function, > the scope is recorded in the variable before each standard config > source file is opened and read. What would we be breaking if we > taught the function to set the current_parsing_scope variable > correctly even when reading from the config_source? That would > certainly simplify this function quite a lot, but if the other parts > of the codebase relies on the current behaviour, we cannot make such > a change lightly. As the author of the SCOPE enum, I think this is the right direction. The only users of current_config_scope() are in config callbacks (e.g., upload_pack_config() checks it for the "packobjectshook"), which you couldn't trigger via "git config" anyway. The main reason I didn't set the scope before in config_with_options() is that it's not given the scope at all; git-config resolves it to the filename. So if git_config_source grows an enum to select the type, and all that filename-resolution gets pushed down into config_with_options(), the whole thing would fall out naturally, I think. -Peff

On the Git mailing list, Junio C Hamano wrote (reply to this):

Jeff King <[email protected]> writes: > ... So if git_config_source grows an enum to select the type, and > all that filename-resolution gets pushed down into config_with_options(), > the whole thing would fall out naturally, I think. Makes sense.

gitgitgadget · 2019-12-18T22:51:21Z

On the Git mailing list, Philip Oakley wrote (reply to this):

Hi Matthew, On 18/12/2019 01:11, Matthew Rogers via GitGitGadget wrote: > From: Matthew Rogers <[email protected]> > > Add new option --show-scope which allows a user to know what the scope > of listed config options are (local/global/system/etc.). > > Signed-off-by: Matthew Rogers <[email protected]> > --- > builtin/config.c | 60 ++++++++++++++++++++++++++++++++++++++++------- > t/t1300-config.sh | 51 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 103 insertions(+), 8 deletions(-) Doesn't this also need a man page update as well for adding the option to the synopsis. The commit message doesn't fully highlight that the config list will often be all the users config values, so each value will be disambiguated/identified as to it's origin. Philip

On the Git mailing list, wrote (reply to this):

Junio, >These are correct changes, but is unrelated noise in the context of >the theme of the patch, no? I think that's the case, would the recommended course of action be to move these changes into its own commit? >> + if (use_local_config || scope == CONFIG_SCOPE_REPO) { >> + return "local"; >> + } else if (use_global_config || scope == CONFIG_SCOPE_GLOBAL) { >> + return "global"; >> + } else if (use_system_config || scope == CONFIG_SCOPE_SYSTEM) { >> + return "system"; > >The above is slightly tricky; --global/--system/--local are made >mutually exclusive in the higher level, so if any of them is set, >we do not even need to look at the "scope" to tell what kind of >source we are reading from. So the way I structured these was to mirror the way other parts of this file check if we should be doing a --local, etc. and mirrored that here. This could definitely be cleaned up if we change the behavior with how --local, etc. set the current_config_scope. >> + } else if (given_config_source.use_stdin || >> + given_config_source.blob || >> + given_config_source.file || >> + scope == CONFIG_SCOPE_CMDLINE) { >> + return "command line"; > >I am not sure what the implication of saying "they came from the >command line" when we read from the standard input or from a blob. I agree with you here, I only put this as "command line" because they came from a place that was ultimately fed in from command line options (--file/--blob). I wouldn't have a problem with calling them out as their own scope ("file", "blob", "stdin"). >> + } else { >> + return "unknown"; >> + } >> +} > >In any case, the need for such logic that says "scope might not say >it is REPO, but when use_local_config is true, we are doing a local >config" implies that "scope" parameter the caller of this function >has is not set correctly when these options are used---would that be >the real bug that needs fixing, rather than getting "worked around" >with a code like this? > >It almost makes me point fingers at config.c::config_with_options() >where config_source is inspected and git_config_from_*() helpers are >called without setting the current_parsing_scope. Unlike these >cases, when do_git_config_sequence() is called from that function, >the scope is recorded in the variable before each standard config >source file is opened and read. What would we be breaking if we >taught the function to set the current_parsing_scope variable >correctly even when reading from the config_source? That would >certainly simplify this function quite a lot, but if the other parts >of the codebase relies on the current behaviour, we cannot make such >a change lightly. From what I can tell from a cursory glance. the only two clients of this function are remote.c and upload-pack.c. The usecase for remote.c mostly seems to be to determine the result of `remote_is_configured()` which (more importantly) seems to be done when that iterates through the relevant configuration options. Similarly for upload-pack.c. I don't think it would be harmful for git config --local, etc. to set that as we would normally intuit. >> +static void show_config_scope(struct strbuf *buf) >> +{ >> + const char term = end_nul ? '\0' : '\t'; >> + const char *scope = scope_to_string(current_config_scope()); >> + >> + strbuf_addch(buf, '('); >> + if (end_nul) >> + strbuf_addstr(buf, N_(scope)); >> + else >> + quote_c_style(scope, buf, NULL, 0); > >Isn't this overkill? I think this code was copied-and-pasted from >the other function that needs to show an arbitrary end-user supplied >data which is a pathname, so it makes perfect sense to use c-style >quoting, but the token scope_to_string() returns is taken from a >bounded set that doesn't require such quoting, no? Yeah, I guess that is a copy+paste mistake. I don't think its necessary since we control the input into this function, So I'll fix that. Philip, >Doesn't this also need a man page update as well for adding the option >to the synopsis. > >The commit message doesn't fully highlight that the config list will >often be all the users config values, so each value will be >disambiguated/identified as to it's origin. I'm agreed on these. So I'll look to readjust that.

On the Git mailing list, Junio C Hamano wrote (reply to this):

<[email protected]> writes: >>These are correct changes, but is unrelated noise in the context of >>the theme of the patch, no? > > I think that's the case, would the recommended course of action be to > move these changes into its own commit? > Yup, and it generally is a good idea to make such a clean-up patch either early in the series, or as a standalone patch (with a note under three-dash lines that another topic is coming on top of the cleanup), because it would be much less likely to introduce new bugs and can be merged quicly to 'next' and then to 'master' to serve as a base to build your other changes on top.

On the Git mailing list, Matt Rogers wrote (reply to this):

Philip, >The commit message doesn't fully highlight that the config list will >often be all the users config values, so each value will be >disambiguated/identified as to it's origin. I don't really understand what you mean by "it's origin" here. When you say origin, do you mean in the "--show-origin" sense of "file/blob/etc." or something else? Because scope is kind of an orthogonal concept to origin in that sense as you can have files with different origins but the same scope. Thanks, Matt On Thu, Dec 19, 2019 at 12:56 PM Junio C Hamano <[email protected]> wrote: > > <[email protected]> writes: > > >>These are correct changes, but is unrelated noise in the context of > >>the theme of the patch, no? > > > > I think that's the case, would the recommended course of action be to > > move these changes into its own commit? > > > > Yup, and it generally is a good idea to make such a clean-up patch > either early in the series, or as a standalone patch (with a note > under three-dash lines that another topic is coming on top of the > cleanup), because it would be much less likely to introduce new bugs > and can be merged quicly to 'next' and then to 'master' to serve as > a base to build your other changes on top. > -- Matthew Rogers

On the Git mailing list, Junio C Hamano wrote (reply to this):

Matt Rogers <[email protected]> writes: > Philip, > >>The commit message doesn't fully highlight that the config list will >>often be all the users config values, so each value will be >>disambiguated/identified as to it's origin. > > I don't really understand what you mean by "it's origin" here. When > you say origin, do you mean in the "--show-origin" sense of "file/blob/etc." > or something else? Because scope is kind of an orthogonal concept to origin > in that sense as you can have files with different origins but the same scope. I do not think origin and scope are orghogonal, though. Can the same file appear as the source for different configuration var-value pair in two different scopes? It is likely that you can _guess_ with high precision that given a pathname reported by --show-origin what scope it is in. It on the other hand is not so trivial given a scope to guess which exact file a var-value pair came from, I would think.\

On the Git mailing list, Matt Rogers wrote (reply to this):

On Fri, Dec 20, 2019 at 9:37 PM Junio C Hamano <[email protected]> wrote: > > I do not think origin and scope are orghogonal, though. Can the > same file appear as the source for different configuration var-value > pair in two different scopes? I meant orthogonal in two senses: That given the current implementation you don't need to have both options active at the same time but can have them active at both times. And that origin and scope correlate, but aren't necessarily one-for-one. For example, --show-origin lists in a known order, but it follows includes and lists the origin as the included file. so if you include a file globally which has includeif "gitdir:..." directives then it can get hairy when all your config files are structured like that. Although, to be fair I doubt that that kind of situation is normal > > It is likely that you can _guess_ with high precision that given a > pathname reported by --show-origin what scope it is in. It on the > other hand is not so trivial given a scope to guess which exact file > a var-value pair came from, I would think.\ Normally yes, but things can get complicating depending on your configuration/include situation.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Matt Rogers <[email protected]> writes: > And that origin and scope correlate, but aren't necessarily > one-for-one. Yes, exactly. When I see "orthogonal", I expect the word describes things that do not correlate. I can see values in the option that gives scope but not path in order to write scripts that limits var-value pairs to be used (e.g. "I do not want to be affected by the per-repository values", "I do not trust settings that comes system-wide"). I also can see values in the option that gives only path when debugging the configuration file. I briefly wondered, for the purpose of such "I do want to see only those settings coming from these scopes" script, it may make more sense to have the command _filter_ the var-value pairs, instead of showing all of them with label, but that feature always exists ;-)

gitgitgadget · 2020-01-09T19:08:45Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > From: Matthew Rogers <[email protected]> > > In git config use of the end_null variable to determine if we should be > null terminating our output. While it is correct to say a string is > "null terminated" the character is actually the "nul" character, so this > malapropism is being fixed. > > Signed-off-by: Matthew Rogers <[email protected]> > --- > builtin/config.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) Obviously correct. Malapropism is a new word in "git log" output in our history ;-) > diff --git a/builtin/config.c b/builtin/config.c > index 98d65bc0ad..52a904cfb1 100644 > --- a/builtin/config.c > +++ b/builtin/config.c > @@ -29,7 +29,7 @@ static int use_worktree_config; > static struct git_config_source given_config_source; > static int actions, type; > static char *default_value; > -static int end_null; > +static int end_nul; > static int respect_includes_opt = -1; > static struct config_options config_options; > static int show_origin; > @@ -151,7 +151,7 @@ static struct option builtin_config_options[] = { > OPT_CALLBACK_VALUE(0, "path", &type, N_("value is a path (file or directory name)"), TYPE_PATH), > OPT_CALLBACK_VALUE(0, "expiry-date", &type, N_("value is an expiry date"), TYPE_EXPIRY_DATE), > OPT_GROUP(N_("Other")), > - OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")), > + OPT_BOOL('z', "null", &end_nul, N_("terminate values with NUL byte")), > OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")), > OPT_BOOL(0, "includes", &respect_includes_opt, N_("respect include directives on lookup")), > OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")), > @@ -178,11 +178,11 @@ static void check_argc(int argc, int min, int max) > > static void show_config_origin(struct strbuf *buf) > { > - const char term = end_null ? '\0' : '\t'; > + const char term = end_nul ? '\0' : '\t'; > > strbuf_addstr(buf, current_config_origin_type()); > strbuf_addch(buf, ':'); > - if (end_null) > + if (end_nul) > strbuf_addstr(buf, current_config_name()); > else > quote_c_style(current_config_name(), buf, NULL, 0); > @@ -678,7 +678,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) > config_options.git_dir = get_git_dir(); > } > > - if (end_null) { > + if (end_nul) { > term = '\0'; > delim = '\n'; > key_delim = '\n';

On the Git mailing list, Matt Rogers wrote (reply to this):

> > Malapropism is a new word in "git log" output in our history ;-) > Yeah, I remember my coworker saying that to me the day I wrote the message and it was just stuck in my head

gitgitgadget · 2020-01-10T11:57:21Z

On the Git mailing list, Jeff King wrote (reply to this):

On Thu, Jan 09, 2020 at 10:16:38AM +0000, Matthew Rogers via GitGitGadget wrote: > - OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")), > + OPT_BOOL('z', "null", &end_nul, N_("terminate values with NUL byte")), The user-facing option is still spelled "null". Obviously we can't just change that, but if we are trying to prefer "nul", should we offer both? I think we actually _do_ allow "--nul" because of parse-option's willingness to accept a partial name. But if we wanted to advertise one over the other in "-h", then we'd have to set up the alias ourselves. Interestingly, "xargs" (and GNU grep) spells it as "--null", so perhaps that's an argument to keep it as-is. Likewise if we accept that "null termination" is a correct name, I suppose. -Peff

gitgitgadget · 2020-02-10T18:17:29Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > From: Matthew Rogers <[email protected]> > > There are many situations where the scope of a config command is known > beforehand, such as passing of '--local', '--file', etc. to an > invocation of git config. However, this information is lost when moving > from builtin/config.c to /config.c. This historically hasn't been a big > deal, but to prepare for the upcoming --show-scope option we teach > git_config_source to keep track of the source and the config machinery > to use that information to set current_parsing_scope appropriately. > > Signed-off-by: Matthew Rogers <[email protected]> > --- > builtin/config.c | 16 +++++++++++++--- > config.c | 3 +++ > config.h | 20 ++++++++++---------- > 3 files changed, 26 insertions(+), 13 deletions(-) This is split from the last step in the previous round, and the splitting makes sense. Thanks.

gitgitgadget · 2020-02-10T18:14:29Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > From: Matthew Rogers <[email protected]> > > do_git_config_sequence operated under the assumption that it was correct > to set current_parsing_scope to CONFIG_SCOPE_UNKNOWN as part of the > cleanup it does after it finishes execution. This is incorrect, as it > blows away the current_parsing_scope if do_git_config_sequence is called > recursively. As such situations are rare (git config running with the > '--blob' option is one example) this has yet to cause a problem, but the > upcoming '--show-scope' option will experience issues in that case, lets > teach do_git_config_sequence to preserve the current_parsing_scope from > before it started execution. > > Signed-off-by: Matthew Rogers <[email protected]> > --- > config.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) This step is new and it does make sense. Thanks. > diff --git a/config.c b/config.c > index fe1e44a43a..0e2c693e78 100644 > --- a/config.c > +++ b/config.c > @@ -1702,6 +1702,7 @@ static int do_git_config_sequence(const struct config_options *opts, > char *xdg_config = xdg_config_home("config"); > char *user_config = expand_user_path("~/.gitconfig", 0); > char *repo_config; > + enum config_scope prev_parsing_scope = current_parsing_scope; > > if (opts->commondir) > repo_config = mkpathdup("%s/config", opts->commondir); > @@ -1741,7 +1742,7 @@ static int do_git_config_sequence(const struct config_options *opts, > if (!opts->ignore_cmdline && git_config_from_parameters(fn, data) < 0) > die(_("unable to parse command-line config")); > > - current_parsing_scope = CONFIG_SCOPE_UNKNOWN; > + current_parsing_scope = prev_parsing_scope; > free(xdg_config); > free(user_config); > free(repo_config);

gitgitgadget · 2020-02-10T18:17:30Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > From: Matthew Rogers <[email protected]> > > Before the changes to teach git_config_source to remember scope > information submodule-config.c never needed to consider the question of > config scope. Even though zeroing out git_config_source is still > correct and preserved the previous behavior of setting the scope to > CONFIG_SCOPE_UNKNOWN, it's better to be explicit about such situations > by explicitly setting the scope. As none of the current config_scope > enumerations make sense we create CONFIG_SCOPE_SUBMODULE to describe the > situation. > > Signed-off-by: Matthew Rogers <[email protected]> > --- Other than a typo on the title, I think this one is perfect ;-) Will queue. > config.c | 2 ++ > config.h | 1 + > submodule-config.c | 4 +++- > 3 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/config.c b/config.c > index 9b6afca210..18a6bdd9ff 100644 > --- a/config.c > +++ b/config.c > @@ -3311,6 +3311,8 @@ const char *config_scope_name(enum config_scope scope) > return "worktree"; > case CONFIG_SCOPE_COMMAND: > return "command"; > + case CONFIG_SCOPE_SUBMODULE: > + return "submodule"; > default: > return "unknown"; > } > diff --git a/config.h b/config.h > index 165cacb7da..fe0addb0dc 100644 > --- a/config.h > +++ b/config.h > @@ -42,6 +42,7 @@ enum config_scope { > CONFIG_SCOPE_LOCAL, > CONFIG_SCOPE_WORKTREE, > CONFIG_SCOPE_COMMAND, > + CONFIG_SCOPE_SUBMODULE, > }; > const char *config_scope_name(enum config_scope scope); > > diff --git a/submodule-config.c b/submodule-config.c > index 85064810b2..b8e97d8ae8 100644 > --- a/submodule-config.c > +++ b/submodule-config.c > @@ -635,7 +635,9 @@ static void submodule_cache_check_init(struct repository *repo) > static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void *data) > { > if (repo->worktree) { > - struct git_config_source config_source = { 0 }; > + struct git_config_source config_source = { > + 0, .scope = CONFIG_SCOPE_SUBMODULE > + }; > const struct config_options opts = { 0 }; > struct object_id oid; > char *file;

gitgitgadget · 2020-01-09T19:08:43Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > From: Matthew Rogers <[email protected]> > > Previously when iterating through git config variables, worktree config > and local config were both considered "CONFIG_SCOPE_REPO". This was > never a problem before as no one had needed to differentiate between the > two cases. Hmph, then "fix" on the title is a bit misleading, no? The enum may not have been as fine grained as you would have liked, but if there was nothing broken, then we are doing this not to "fix" anything. A more important thing to explain would probably be why we (i.e. those who propose this change, those who give favoriable reviews to it, and those who accept it change to the system) would want to see finer-grained classification. What do we expect to be able to do with the resulting finer-grained set that we wouldn't be able to with the original, and why is it a good thing? That is what readers of the proposed log message of this change would want to see, I would think. > Additionally we rename what was CONFIG_SCOPE_REPO to CONFIG_SCOPE_LOCAL > to reflect its new, more specific meaning. > > The clients of 'current_config_scope()' who cared about > CONFIG_SCOPE_REPO are also modified to similarly care about > CONFIG_SCOPE_WORKTREE and CONFIG_SCOPE_LOCAL to preserve previous behavior. > > Signed-off-by: Matthew Rogers <[email protected]> > --- > config.c | 7 ++----- > config.h | 3 ++- > remote.c | 3 ++- > t/helper/test-config.c | 4 +++- > upload-pack.c | 3 ++- > 5 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/config.c b/config.c > index d75f88ca0c..447a013a15 100644 > --- a/config.c > +++ b/config.c > @@ -1724,15 +1724,12 @@ static int do_git_config_sequence(const struct config_options *opts, > if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) > ret += git_config_from_file(fn, user_config, data); > > - current_parsing_scope = CONFIG_SCOPE_REPO; > + current_parsing_scope = CONFIG_SCOPE_LOCAL; > if (!opts->ignore_repo && repo_config && > !access_or_die(repo_config, R_OK, 0)) > ret += git_config_from_file(fn, repo_config, data); > > - /* > - * Note: this should have a new scope, CONFIG_SCOPE_WORKTREE. > - * But let's not complicate things before it's actually needed. > - */ > + current_parsing_scope = CONFIG_SCOPE_WORKTREE; > if (!opts->ignore_worktree && repository_format_worktree_config) { > char *path = git_pathdup("config.worktree"); > if (!access_or_die(path, R_OK, 0)) > diff --git a/config.h b/config.h > index 91fd4c5e96..284d92fb0e 100644 > --- a/config.h > +++ b/config.h > @@ -298,7 +298,8 @@ enum config_scope { > CONFIG_SCOPE_UNKNOWN = 0, > CONFIG_SCOPE_SYSTEM, > CONFIG_SCOPE_GLOBAL, > - CONFIG_SCOPE_REPO, > + CONFIG_SCOPE_LOCAL, > + CONFIG_SCOPE_WORKTREE, > CONFIG_SCOPE_CMDLINE, > }; > > diff --git a/remote.c b/remote.c > index 5c4666b53a..593ce297ed 100644 > --- a/remote.c > +++ b/remote.c > @@ -369,7 +369,8 @@ static int handle_config(const char *key, const char *value, void *cb) > } > remote = make_remote(name, namelen); > remote->origin = REMOTE_CONFIG; > - if (current_config_scope() == CONFIG_SCOPE_REPO) > + if (current_config_scope() == CONFIG_SCOPE_LOCAL || > + current_config_scope() == CONFIG_SCOPE_WORKTREE) > remote->configured_in_repo = 1; > if (!strcmp(subkey, "mirror")) > remote->mirror = git_config_bool(key, value); > diff --git a/t/helper/test-config.c b/t/helper/test-config.c > index 214003d5b2..6695e463eb 100644 > --- a/t/helper/test-config.c > +++ b/t/helper/test-config.c > @@ -44,8 +44,10 @@ static const char *scope_name(enum config_scope scope) > return "system"; > case CONFIG_SCOPE_GLOBAL: > return "global"; > - case CONFIG_SCOPE_REPO: > + case CONFIG_SCOPE_LOCAL: > return "repo"; > + case CONFIG_SCOPE_WORKTREE: > + return "worktree"; > case CONFIG_SCOPE_CMDLINE: > return "cmdline"; > default: > diff --git a/upload-pack.c b/upload-pack.c > index a00d7ece6b..c53249cac1 100644 > --- a/upload-pack.c > +++ b/upload-pack.c > @@ -1073,7 +1073,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused) > precomposed_unicode = git_config_bool(var, value); > } > > - if (current_config_scope() != CONFIG_SCOPE_REPO) { > + if (current_config_scope() != CONFIG_SCOPE_LOCAL && > + current_config_scope() != CONFIG_SCOPE_WORKTREE) { > if (!strcmp("uploadpack.packobjectshook", var)) > return git_config_string(&pack_objects_hook, var, value); > }

On the Git mailing list, Matt Rogers wrote (reply to this):

On Thu, Jan 9, 2020 at 2:06 PM Junio C Hamano <[email protected]> wrote: > > "Matthew Rogers via GitGitGadget" <[email protected]> writes: > > > From: Matthew Rogers <[email protected]> > > > > Previously when iterating through git config variables, worktree config > > and local config were both considered "CONFIG_SCOPE_REPO". This was > > never a problem before as no one had needed to differentiate between the > > two cases. > > Hmph, then "fix" on the title is a bit misleading, no? > > The enum may not have been as fine grained as you would have liked, > but if there was nothing broken, then we are doing this not to "fix" > anything. > I see where you're coming from, but I would definitely consider this a "fix" in that it's something that (as explained in the deleted comment) should have been this way from the start but was unnecessary as no one had a need for it yet. But I definitely wouldn't be against changing the phrasing to something like "clean up" or whatever your preferred wording would be. > A more important thing to explain would probably be why we > (i.e. those who propose this change, those who give favoriable > reviews to it, and those who accept it change to the system) would > want to see finer-grained classification. What do we expect to be > able to do with the resulting finer-grained set that we wouldn't be > able to with the original, and why is it a good thing? That is what > readers of the proposed log message of this change would want to > see, I would think. > This is really more prep for patch 4 later on in this series, as a user who ran the proposed '--show-scope' later on in the series would care what was "worktree" vs "local". Regardless, I think the two options I have would be to amend the commit message with that extra information or roll it together with patch 4

gitgitgadget · 2020-01-09T19:20:46Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > From: Matthew Rogers <[email protected]> > > CONFIG_SCOPE_CMDLINE is generally used in the code to refer to config > values passed in via the -c option. This is a little bit too specific > as there are other methods to pass config values so that the last for a > single command (namely --file and --blob). Sorry, but I cannot parse this, especially around "so that the last for ..." part. > As the "visibility" of config > values passed by these situations is common, we unify them as having a > scope of "command" rather than "command line". Is the "unification" something done by this patch? It does not appear to be so. The existing code already was using CMDLINE to call "git -c VAR=VAL" and also something else (which is not clear to me, unfortunately, probably because I failed to parse the above X-<), and this step renames CMDLINE to COMMAND perhaps because CMDLINE has too strong connotation with the "-c" thing and much less with the other thing (which is not clear to me, unfortunately) and you found that renaming it to COMMAND would cover both cases better? I also do not get what you meant by "visibility", but it probably is primarily because it is not clear what "the other thing" is. > Signed-off-by: Matthew Rogers <[email protected]> > --- > config.c | 2 +- > config.h | 2 +- > t/helper/test-config.c | 4 ++-- > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/config.c b/config.c > index 447a013a15..f319a3d6a0 100644 > --- a/config.c > +++ b/config.c > @@ -1737,7 +1737,7 @@ static int do_git_config_sequence(const struct config_options *opts, > free(path); > } > > - current_parsing_scope = CONFIG_SCOPE_CMDLINE; > + current_parsing_scope = CONFIG_SCOPE_COMMAND; > if (!opts->ignore_cmdline && git_config_from_parameters(fn, data) < 0) > die(_("unable to parse command-line config")); > > diff --git a/config.h b/config.h > index 284d92fb0e..f383a71404 100644 > --- a/config.h > +++ b/config.h > @@ -300,7 +300,7 @@ enum config_scope { > CONFIG_SCOPE_GLOBAL, > CONFIG_SCOPE_LOCAL, > CONFIG_SCOPE_WORKTREE, > - CONFIG_SCOPE_CMDLINE, > + CONFIG_SCOPE_COMMAND, > }; > > enum config_scope current_config_scope(void); > diff --git a/t/helper/test-config.c b/t/helper/test-config.c > index 6695e463eb..78bbb9eb98 100644 > --- a/t/helper/test-config.c > +++ b/t/helper/test-config.c > @@ -48,8 +48,8 @@ static const char *scope_name(enum config_scope scope) > return "repo"; > case CONFIG_SCOPE_WORKTREE: > return "worktree"; > - case CONFIG_SCOPE_CMDLINE: > - return "cmdline"; > + case CONFIG_SCOPE_COMMAND: > + return "command"; > default: > return "unknown"; > }

On the Git mailing list, Matt Rogers wrote (reply to this):

On Thu, Jan 9, 2020 at 2:14 PM Junio C Hamano <[email protected]> wrote: > > "Matthew Rogers via GitGitGadget" <[email protected]> writes: > > > From: Matthew Rogers <[email protected]> > > > > CONFIG_SCOPE_CMDLINE is generally used in the code to refer to config > > values passed in via the -c option. This is a little bit too specific > > as there are other methods to pass config values so that the last for a > > single command (namely --file and --blob). > > Sorry, but I cannot parse this, especially around "so that the last > for ..." part. > My bad, I guess I must have not read as carefully as I thought I did. It should read: "... there are other methods to pass config values so that _they_ last for a single command ..." > > As the "visibility" of config > > values passed by these situations is common, we unify them as having a > > scope of "command" rather than "command line". > > Is the "unification" something done by this patch? It does not > appear to be so. The existing code already was using CMDLINE to > call "git -c VAR=VAL" and also something else (which is not clear to > me, unfortunately, probably because I failed to parse the above > X-<), and this step renames CMDLINE to COMMAND perhaps because > CMDLINE has too strong connotation with the "-c" thing and much less > with the other thing (which is not clear to me, unfortunately) and > you found that renaming it to COMMAND would cover both cases better? Essentially, the "unification" was referring to more the unification of all the things that affect configuration only for the duration of a specific command. The gist of this patch was to say "There are a few ways besides -c to pass a configuration option that lasts for a single command, so it makes sense to broaden that scope.". The change is definitely justified in that COMMAND communicates that much clearer than CMDLINE as REPO, GLOBAL, SYSTEM all describe the thing that can see the configuration options, and it's specifically the command the can see the -c options and not the command line as a whole. > > I also do not get what you meant by "visibility", but it probably is > primarily because it is not clear what "the other thing" is. I was using visibility as another way to conceptualize scoping. A scope more or less determines who can "see" a thing, so maybe that language was a little bit too much in my head. The issue is that currently the code doesn't care about any of that (as only -c options actually have COMMAND scoping), so maybe I should roll it into the next patch of the series? As that introduces code that actually cares about the difference?

gitgitgadget · 2020-01-17T20:47:49Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > diff --git a/config.h b/config.h > index 91fd4c5e96..284d92fb0e 100644 > --- a/config.h > +++ b/config.h > @@ -298,7 +298,8 @@ enum config_scope { > CONFIG_SCOPE_UNKNOWN = 0, > CONFIG_SCOPE_SYSTEM, > CONFIG_SCOPE_GLOBAL, > - CONFIG_SCOPE_REPO, > + CONFIG_SCOPE_LOCAL, > + CONFIG_SCOPE_WORKTREE, > CONFIG_SCOPE_CMDLINE, > }; So the gist of the change is to split REPO into two, LOCAL and WORKTREE. If we can find a way to state that concisely, perhaps we can improve "refine enum" and make it more informative. > diff --git a/t/helper/test-config.c b/t/helper/test-config.c > index 214003d5b2..6695e463eb 100644 > --- a/t/helper/test-config.c > +++ b/t/helper/test-config.c > @@ -44,8 +44,10 @@ static const char *scope_name(enum config_scope scope) > return "system"; > case CONFIG_SCOPE_GLOBAL: > return "global"; > - case CONFIG_SCOPE_REPO: > + case CONFIG_SCOPE_LOCAL: > return "repo"; > + case CONFIG_SCOPE_WORKTREE: > + return "worktree"; > case CONFIG_SCOPE_CMDLINE: > return "cmdline"; > default: > diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh > index 7b4e1a63eb..535b2a73f7 100755 > --- a/t/t1308-config-set.sh > +++ b/t/t1308-config-set.sh > @@ -265,7 +265,7 @@ test_expect_success 'iteration shows correct origins' ' > value=from-cmdline > origin=command line > name= > - scope=cmdline > + scope=command Why??? > EOF > GIT_CONFIG_PARAMETERS=$cmdline_config test-tool config iterate >actual && > test_cmp expect actual > diff --git a/upload-pack.c b/upload-pack.c > index a00d7ece6b..c53249cac1 100644 > --- a/upload-pack.c > +++ b/upload-pack.c > @@ -1073,7 +1073,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused) > precomposed_unicode = git_config_bool(var, value); > } > > - if (current_config_scope() != CONFIG_SCOPE_REPO) { > + if (current_config_scope() != CONFIG_SCOPE_LOCAL && > + current_config_scope() != CONFIG_SCOPE_WORKTREE) { > if (!strcmp("uploadpack.packobjectshook", var)) > return git_config_string(&pack_objects_hook, var, value); > }

On the Git mailing list, Matt Rogers wrote (reply to this):

> So the gist of the change is to split REPO into two, LOCAL and > WORKTREE. > > If we can find a way to state that concisely, perhaps we can improve > "refine enum" and make it more informative. > Should I just say "split repo scope" Or is that too on the nose? > > diff --git a/t/helper/test-config.c b/t/helper/test-config.c > > index 214003d5b2..6695e463eb 100644 > > --- a/t/helper/test-config.c > > +++ b/t/helper/test-config.c > > @@ -44,8 +44,10 @@ static const char *scope_name(enum config_scope scope) > > return "system"; > > case CONFIG_SCOPE_GLOBAL: > > return "global"; > > - case CONFIG_SCOPE_REPO: > > + case CONFIG_SCOPE_LOCAL: > > return "repo"; > > + case CONFIG_SCOPE_WORKTREE: > > + return "worktree"; > > case CONFIG_SCOPE_CMDLINE: > > return "cmdline"; > > default: > > diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh > > index 7b4e1a63eb..535b2a73f7 100755 > > --- a/t/t1308-config-set.sh > > +++ b/t/t1308-config-set.sh > > @@ -265,7 +265,7 @@ test_expect_success 'iteration shows correct origins' ' > > value=from-cmdline > > origin=command line > > name= > > - scope=cmdline > > + scope=command > > Why??? I meant to put this change into the next series in the patch, but I must have messed up, so I'll readjust that. -- Matthew Rogers

On the Git mailing list, Junio C Hamano wrote (reply to this):

Matt Rogers <[email protected]> writes: >> So the gist of the change is to split REPO into two, LOCAL and >> WORKTREE. >> >> If we can find a way to state that concisely, perhaps we can improve >> "refine enum" and make it more informative. >> > > Should I just say "split repo scope" Or is that too on the nose? Yeah, I think phrasing along that line would work well. config: split repo scope to local and worktree That's 46 chars.

gitgitgadget · 2020-01-17T21:02:52Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > From: Matthew Rogers <[email protected]> > > CONFIG_SCOPE_CMDLINE is generally used in the code to refer to config > values passed in via the -c option. Options passed in using this > mechanism share similar scoping characteristics with the --file and > --blob options of the 'config' command, namely that they are only in use > for that single invocation of git, and that they supersede the normal > system/global/local hierarchy. All of the above justifies why it makes sense to treat --file, --blob and "git -c VAR=VAL" as the same scope (i.e. it would have been a nice part of log message of the commit that introduced SCOPE_CMDLINE), but that is not something we need to justify now---we have such a scope for one-shot settings and these three sources are already treated as the same class. > This patch introduces > CONFIG_SCOPE_COMMAND to reflect this new idea, which also makes > CONFIG_SCOPE_CMDLINE redundant. The change in this commit is to rename CMDLINE to COMMAND. That is what the proposed log message for this step must justify. We internally use CONFIG_SCOPE_CMDLINE for the scope for the configuration variables that come from "git -c VAR=VAL", "git config --file=FILE" and "git config --blob=BLOB". As we are going to expose the scope names to end-users in the next step, let's rethink the half-cryptic "cmdline" and instead use a proper word "command" (the settings from three sources in this scope are all in effect only during a single invocation of a git command). or something like that. > --- a/t/helper/test-config.c > +++ b/t/helper/test-config.c > @@ -48,8 +48,8 @@ static const char *scope_name(enum config_scope scope) > return "repo"; > case CONFIG_SCOPE_WORKTREE: > return "worktree"; > - case CONFIG_SCOPE_CMDLINE: > - return "cmdline"; > + case CONFIG_SCOPE_COMMAND: > + return "command"; The only externally observable effect of this patch is this output string from test-tool and we are not breaking end-user experience, but I am not sure if this churn is worth it. I dunno. In any case, the change to t1308 we saw in the previous step belongs to this step, I think.

On the Git mailing list, Matt Rogers wrote (reply to this):

> we have such a scope for one-shot settings and these three > sources are already treated as the same class. > So this isn't technically correct, as before the final patch of this series we technically don't set the current_parsing_scope for those options. This doesn't actually affect anything though, because before then nothing actually checks the current_parsing_scope during such calls to git config. As such, it makes sense to me to keep that part of the commit message. > > This patch introduces > > CONFIG_SCOPE_COMMAND to reflect this new idea, which also makes > > CONFIG_SCOPE_CMDLINE redundant. > > The change in this commit is to rename CMDLINE to COMMAND. That is > what the proposed log message for this step must justify. > > We internally use CONFIG_SCOPE_CMDLINE for the scope for the > configuration variables that come from "git -c VAR=VAL", > "git config --file=FILE" and "git config --blob=BLOB". As > we are going to expose the scope names to end-users in the > next step, let's rethink the half-cryptic "cmdline" and > instead use a proper word "command" (the settings from three > sources in this scope are all in effect only during a single > invocation of a git command). > > or something like that. I like this explanation much better so I'll roll that into the commit message. > > > > --- a/t/helper/test-config.c > > +++ b/t/helper/test-config.c > > @@ -48,8 +48,8 @@ static const char *scope_name(enum config_scope scope) > > return "repo"; > > case CONFIG_SCOPE_WORKTREE: > > return "worktree"; > > - case CONFIG_SCOPE_CMDLINE: > > - return "cmdline"; > > + case CONFIG_SCOPE_COMMAND: > > + return "command"; > > The only externally observable effect of this patch is this output > string from test-tool and we are not breaking end-user experience, > but I am not sure if this churn is worth it. I dunno. > > In any case, the change to t1308 we saw in the previous step belongs > to this step, I think. Yeah, this was my bad... -- Matthew Rogers

gitgitgadget · 2020-01-24T18:52:18Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > From: Matthew Rogers <[email protected]> > > Previously when iterating through git config variables, worktree config > and local config were both considered "CONFIG_SCOPE_REPO". This was > never a problem before as no one had needed to differentiate between the > two cases, but future functionality may care whether or not the config > options come from a worktree or from the repository's actual local > config file. For example, the planned feature to add a '--show-scope' > to config to allow a user to see which scope listed config options come > from would confuse users if it just printed 'repo' rather than 'local' > or 'worktree' as the documentation would lead them to expect. As well > as the additional benefit of making the implementation look more like > how the documentation describes the interface. > > To accomplish this we split out what was previously considered repo > scope to be local and worktree. > > The clients of 'current_config_scope()' who cared about > CONFIG_SCOPE_REPO are also modified to similarly care about > CONFIG_SCOPE_WORKTREE and CONFIG_SCOPE_LOCAL to preserve previous behavior. > > Signed-off-by: Matthew Rogers <[email protected]> > --- > config.c | 7 ++----- > config.h | 3 ++- > remote.c | 3 ++- > t/helper/test-config.c | 4 +++- > t/t1308-config-set.sh | 2 +- > upload-pack.c | 3 ++- > 6 files changed, 12 insertions(+), 10 deletions(-) Nicely done. > diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh > index 7b4e1a63eb..90196e2862 100755 > --- a/t/t1308-config-set.sh > +++ b/t/t1308-config-set.sh > @@ -259,7 +259,7 @@ test_expect_success 'iteration shows correct origins' ' > value=from-repo > origin=file > name=.git/config > - scope=repo > + scope=local > > key=foo.bar > value=from-cmdline OK.

gitgitgadget · 2020-01-24T19:16:21Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > diff --git a/t/helper/test-config.c b/t/helper/test-config.c > index 214003d5b2..6695e463eb 100644 > --- a/t/helper/test-config.c > +++ b/t/helper/test-config.c > @@ -44,8 +44,10 @@ static const char *scope_name(enum config_scope scope) > return "system"; > case CONFIG_SCOPE_GLOBAL: > return "global"; > - case CONFIG_SCOPE_REPO: > + case CONFIG_SCOPE_LOCAL: > return "repo"; This must be updated to "local", no, to match the change to t1308? > + case CONFIG_SCOPE_WORKTREE: > + return "worktree"; > case CONFIG_SCOPE_CMDLINE: > return "cmdline"; > default: > diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh > index 7b4e1a63eb..90196e2862 100755 > --- a/t/t1308-config-set.sh > +++ b/t/t1308-config-set.sh > @@ -259,7 +259,7 @@ test_expect_success 'iteration shows correct origins' ' > value=from-repo > origin=file > name=.git/config > - scope=repo > + scope=local

gitgitgadget · 2020-01-27T23:16:25Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > diff --git a/t/helper/test-config.c b/t/helper/test-config.c > index 214003d5b2..6695e463eb 100644 > --- a/t/helper/test-config.c > +++ b/t/helper/test-config.c > @@ -44,8 +44,10 @@ static const char *scope_name(enum config_scope scope) > return "system"; > case CONFIG_SCOPE_GLOBAL: > return "global"; > - case CONFIG_SCOPE_REPO: > + case CONFIG_SCOPE_LOCAL: > return "repo"; > + case CONFIG_SCOPE_WORKTREE: > + return "worktree"; It used to be only "repo"; now we give either "repo" or "worktree". > case CONFIG_SCOPE_CMDLINE: > return "cmdline"; > default: > diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh > index 7b4e1a63eb..90196e2862 100755 > --- a/t/t1308-config-set.sh > +++ b/t/t1308-config-set.sh > @@ -259,7 +259,7 @@ test_expect_success 'iteration shows correct origins' ' > value=from-repo > origin=file > name=.git/config > - scope=repo > + scope=local But the test expects "local". This patch cannot be correct. I recall fixing this manually when I queued the previous round to 'pu'. Please double check.

gitgitgadget · 2020-02-10T18:11:30Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > From: Matthew Rogers <[email protected]> > > Previously when iterating through git config variables, worktree config > and local config were both considered "CONFIG_SCOPE_REPO". This was > never a problem before as no one had needed to differentiate between the > two cases, but future functionality may care whether or not the config > options come from a worktree or from the repository's actual local > config file. For example, the planned feature to add a '--show-scope' > to config to allow a user to see which scope listed config options come > from would confuse users if it just printed 'repo' rather than 'local' > or 'worktree' as the documentation would lead them to expect. As well > as the additional benefit of making the implementation look more like > how the documentation describes the interface. > > To accomplish this we split out what was previously considered repo > scope to be local and worktree. > > The clients of 'current_config_scope()' who cared about > CONFIG_SCOPE_REPO are also modified to similarly care about > CONFIG_SCOPE_WORKTREE and CONFIG_SCOPE_LOCAL to preserve previous behavior. > > Signed-off-by: Matthew Rogers <[email protected]> > Signed-off-by: Junio C Hamano <[email protected]> > --- > config.c | 13 ++++++------- > config.h | 3 ++- > remote.c | 3 ++- > t/t1308-config-set.sh | 2 +- > upload-pack.c | 3 ++- > 5 files changed, 13 insertions(+), 11 deletions(-) Makes sense. And this step does exactly what it claims to do and nothing else, which is very good ;-) > diff --git a/config.c b/config.c > index 83bb98d65e..7422bdebb1 100644 > --- a/config.c > +++ b/config.c > @@ -1724,15 +1724,12 @@ static int do_git_config_sequence(const struct config_options *opts, > if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) > ret += git_config_from_file(fn, user_config, data); > > - current_parsing_scope = CONFIG_SCOPE_REPO; > + current_parsing_scope = CONFIG_SCOPE_LOCAL; > if (!opts->ignore_repo && repo_config && > !access_or_die(repo_config, R_OK, 0)) > ret += git_config_from_file(fn, repo_config, data); > > - /* > - * Note: this should have a new scope, CONFIG_SCOPE_WORKTREE. > - * But let's not complicate things before it's actually needed. > - */ > + current_parsing_scope = CONFIG_SCOPE_WORKTREE; > if (!opts->ignore_worktree && repository_format_worktree_config) { > char *path = git_pathdup("config.worktree"); > if (!access_or_die(path, R_OK, 0)) > @@ -3304,8 +3301,10 @@ const char *config_scope_name(enum config_scope scope) > return "system"; > case CONFIG_SCOPE_GLOBAL: > return "global"; > - case CONFIG_SCOPE_REPO: > - return "repo"; > + case CONFIG_SCOPE_LOCAL: > + return "local"; > + case CONFIG_SCOPE_WORKTREE: > + return "worktree"; > case CONFIG_SCOPE_CMDLINE: > return "command line"; > default: > diff --git a/config.h b/config.h > index dcb8c274d4..d3ed41ef8e 100644 > --- a/config.h > +++ b/config.h > @@ -299,7 +299,8 @@ enum config_scope { > CONFIG_SCOPE_UNKNOWN = 0, > CONFIG_SCOPE_SYSTEM, > CONFIG_SCOPE_GLOBAL, > - CONFIG_SCOPE_REPO, > + CONFIG_SCOPE_LOCAL, > + CONFIG_SCOPE_WORKTREE, > CONFIG_SCOPE_CMDLINE, > }; > const char *config_scope_name(enum config_scope scope); > diff --git a/remote.c b/remote.c > index 5c4666b53a..593ce297ed 100644 > --- a/remote.c > +++ b/remote.c > @@ -369,7 +369,8 @@ static int handle_config(const char *key, const char *value, void *cb) > } > remote = make_remote(name, namelen); > remote->origin = REMOTE_CONFIG; > - if (current_config_scope() == CONFIG_SCOPE_REPO) > + if (current_config_scope() == CONFIG_SCOPE_LOCAL || > + current_config_scope() == CONFIG_SCOPE_WORKTREE) > remote->configured_in_repo = 1; > if (!strcmp(subkey, "mirror")) > remote->mirror = git_config_bool(key, value); > diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh > index 5f3e71a160..728a2b87ce 100755 > --- a/t/t1308-config-set.sh > +++ b/t/t1308-config-set.sh > @@ -259,7 +259,7 @@ test_expect_success 'iteration shows correct origins' ' > value=from-repo > origin=file > name=.git/config > - scope=repo > + scope=local > > key=foo.bar > value=from-cmdline > diff --git a/upload-pack.c b/upload-pack.c > index a00d7ece6b..c53249cac1 100644 > --- a/upload-pack.c > +++ b/upload-pack.c > @@ -1073,7 +1073,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused) > precomposed_unicode = git_config_bool(var, value); > } > > - if (current_config_scope() != CONFIG_SCOPE_REPO) { > + if (current_config_scope() != CONFIG_SCOPE_LOCAL && > + current_config_scope() != CONFIG_SCOPE_WORKTREE) { > if (!strcmp("uploadpack.packobjectshook", var)) > return git_config_string(&pack_objects_hook, var, value); > }

gitgitgadget · 2020-02-10T18:11:31Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > From: Matthew Rogers <[email protected]> > > CONFIG_SCOPE_CMDLINE is generally used in the code to refer to config > values passed in via the -c option. Options passed in using this > mechanism share similar scoping characteristics with the --file and > --blob options of the 'config' command, namely that they are only in use > for that single invocation of git, and that they supersede the normal > system/global/local hierarchy. This patch introduces > CONFIG_SCOPE_COMMAND to reflect this new idea, which also makes > CONFIG_SCOPE_CMDLINE redundant. > > Signed-off-by: Matthew Rogers <[email protected]> > Signed-off-by: Junio C Hamano <[email protected]> > --- > config.c | 6 +++--- > config.h | 2 +- > t/t1308-config-set.sh | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) This makes sense, even though it is halfway undoing what 04/10 unnecessarily did ;-) I think I can just wiggle minor fixes in to 04/10 and then adjust this one to get the series into a better shape myself before queuing them to expedite the process. Let me finish reading the remainder of the topic. Thanks. > diff --git a/config.c b/config.c > index 7422bdebb1..fe1e44a43a 100644 > --- a/config.c > +++ b/config.c > @@ -1737,7 +1737,7 @@ static int do_git_config_sequence(const struct config_options *opts, > free(path); > } > > - current_parsing_scope = CONFIG_SCOPE_CMDLINE; > + current_parsing_scope = CONFIG_SCOPE_COMMAND; > if (!opts->ignore_cmdline && git_config_from_parameters(fn, data) < 0) > die(_("unable to parse command-line config")); > > @@ -3305,8 +3305,8 @@ const char *config_scope_name(enum config_scope scope) > return "local"; > case CONFIG_SCOPE_WORKTREE: > return "worktree"; > - case CONFIG_SCOPE_CMDLINE: > - return "command line"; > + case CONFIG_SCOPE_COMMAND: > + return "command"; > default: > return "unknown"; > } > diff --git a/config.h b/config.h > index d3ed41ef8e..b570f4ce43 100644 > --- a/config.h > +++ b/config.h > @@ -301,7 +301,7 @@ enum config_scope { > CONFIG_SCOPE_GLOBAL, > CONFIG_SCOPE_LOCAL, > CONFIG_SCOPE_WORKTREE, > - CONFIG_SCOPE_CMDLINE, > + CONFIG_SCOPE_COMMAND, > }; > const char *config_scope_name(enum config_scope scope); > > diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh > index 728a2b87ce..fba0abe429 100755 > --- a/t/t1308-config-set.sh > +++ b/t/t1308-config-set.sh > @@ -265,7 +265,7 @@ test_expect_success 'iteration shows correct origins' ' > value=from-cmdline > origin=command line > name= > - scope=command line > + scope=command > EOF > GIT_CONFIG_PARAMETERS=$cmdline_config test-tool config iterate >actual && > test_cmp expect actual

gitgitgadget · 2020-02-10T18:05:30Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Matthew Rogers via GitGitGadget" <[email protected]> writes: > From: Matthew Rogers <[email protected]> > > To prepare for the upcoming --show-scope option, we require the ability > to convert a config_scope enum to a string. As this was originally > implemented as a static function 'scope_name()' in > t/helper/test-config.c, we expose it via config.h and give it a less > ambiguous name 'config_scope_name()' > Signed-off-by: Matthew Rogers <[email protected]> > --- > config.c | 16 ++++++++++++++++ > config.h | 2 ++ > t/helper/test-config.c | 17 +---------------- > t/t1308-config-set.sh | 2 +- > 4 files changed, 20 insertions(+), 17 deletions(-) > > diff --git a/config.c b/config.c > index d75f88ca0c..83bb98d65e 100644 > --- a/config.c > +++ b/config.c > @@ -3297,6 +3297,22 @@ const char *current_config_origin_type(void) > } > } > > +const char *config_scope_name(enum config_scope scope) > +{ > + switch (scope) { > + case CONFIG_SCOPE_SYSTEM: > + return "system"; > + case CONFIG_SCOPE_GLOBAL: > + return "global"; > + case CONFIG_SCOPE_REPO: > + return "repo"; > + case CONFIG_SCOPE_CMDLINE: > + return "command line"; The change from "cmdline" to "command line" does need to happen before the end of the series, but I do not think it should happen here, especialy given that the proposed log message explains that this step is to expose scope_name() under a better name (which is a very good split point). How are you reviewing the patches in your own series before sending them out? This round is better than the previous rounds where we didn't have a matching change to the tests so "make test" may not have passed in the middle of the series, though... > + default: > + return "unknown"; > + } > +} > + > const char *current_config_name(void) > { > const char *name; > diff --git a/config.h b/config.h > index 91fd4c5e96..dcb8c274d4 100644 > --- a/config.h > +++ b/config.h > @@ -35,6 +35,7 @@ struct object_id; > > #define CONFIG_REGEX_NONE ((void *)1) > > + > struct git_config_source { > unsigned int use_stdin:1; > const char *file; > @@ -301,6 +302,7 @@ enum config_scope { > CONFIG_SCOPE_REPO, > CONFIG_SCOPE_CMDLINE, > }; > +const char *config_scope_name(enum config_scope scope); > > enum config_scope current_config_scope(void); > const char *current_config_origin_type(void); > diff --git a/t/helper/test-config.c b/t/helper/test-config.c > index 214003d5b2..1e3bc7c8f4 100644 > --- a/t/helper/test-config.c > +++ b/t/helper/test-config.c > @@ -37,21 +37,6 @@ > * > */ > > -static const char *scope_name(enum config_scope scope) > -{ > - switch (scope) { > - case CONFIG_SCOPE_SYSTEM: > - return "system"; > - case CONFIG_SCOPE_GLOBAL: > - return "global"; > - case CONFIG_SCOPE_REPO: > - return "repo"; > - case CONFIG_SCOPE_CMDLINE: > - return "cmdline"; > - default: > - return "unknown"; > - } > -} > static int iterate_cb(const char *var, const char *value, void *data) > { > static int nr; > @@ -63,7 +48,7 @@ static int iterate_cb(const char *var, const char *value, void *data) > printf("value=%s\n", value ? value : "(null)"); > printf("origin=%s\n", current_config_origin_type()); > printf("name=%s\n", current_config_name()); > - printf("scope=%s\n", scope_name(current_config_scope())); > + printf("scope=%s\n", config_scope_name(current_config_scope())); > > return 0; > } > diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh > index 7b4e1a63eb..5f3e71a160 100755 > --- a/t/t1308-config-set.sh > +++ b/t/t1308-config-set.sh > @@ -265,7 +265,7 @@ test_expect_success 'iteration shows correct origins' ' > value=from-cmdline > origin=command line > name= > - scope=cmdline > + scope=command line > EOF > GIT_CONFIG_PARAMETERS=$cmdline_config test-tool config iterate >actual && > test_cmp expect actual

On the Git mailing list, Junio C Hamano wrote (reply to this):

Junio C Hamano <[email protected]> writes: > The change from "cmdline" to "command line" does need to happen > before the end of the series, but I do not think it should happen > here, especialy given that the proposed log message explains that > this step is to expose scope_name() under a better name (which is a > very good split point). I'll tweak this step with the attached patch, and then adjust 06/10 as needed, while queuing. config.c | 2 +- config.h | 1 - t/t1308-config-set.sh | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/config.c b/config.c index 83bb98d65e..a922b136e5 100644 --- a/config.c +++ b/config.c @@ -3307,7 +3307,7 @@ const char *config_scope_name(enum config_scope scope) case CONFIG_SCOPE_REPO: return "repo"; case CONFIG_SCOPE_CMDLINE: - return "command line"; + return "cmdline"; default: return "unknown"; } diff --git a/config.h b/config.h index dcb8c274d4..c063f33ff6 100644 --- a/config.h +++ b/config.h @@ -35,7 +35,6 @@ struct object_id; #define CONFIG_REGEX_NONE ((void *)1) - struct git_config_source { unsigned int use_stdin:1; const char *file; diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index 5f3e71a160..7b4e1a63eb 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -265,7 +265,7 @@ test_expect_success 'iteration shows correct origins' ' value=from-cmdline origin=command line name= - scope=command line + scope=cmdline EOF GIT_CONFIG_PARAMETERS=$cmdline_config test-tool config iterate >actual && test_cmp expect actual

On the Git mailing list, Matt Rogers wrote (reply to this):

> > How are you reviewing the patches in your own series before sending > them out? This round is better than the previous rounds where we > didn't have a matching change to the tests so "make test" may not > have passed in the middle of the series, though... > I went through each patch individually using rebase -i and built/tested it. Although just to save time I only did t1300 and t1308 since I believe those were the only ones that should be affected. I can write a script that would run the whole test suite overnight for me and make sure the series shakes out okay, if you'd like.

On the Git mailing list, Emily Shaffer wrote (reply to this):

On Mon, Feb 10, 2020 at 07:30:22PM -0500, Matt Rogers wrote: > > > > How are you reviewing the patches in your own series before sending > > them out? This round is better than the previous rounds where we > > didn't have a matching change to the tests so "make test" may not > > have passed in the middle of the series, though... > > > > I went through each patch individually using rebase -i and built/tested it. > Although just to save time I only did t1300 and t1308 since I believe those were > the only ones that should be affected. I can write a script that > would run the whole > test suite overnight for me and make sure the series shakes out okay, > if you'd like. Not sure whether you intend to do this or not, but to maybe save you some scripting, I do this like so: git rebase -x \ "make -j16 && (cd t && prove -j16 -v --shuffle t[0-9]*.sh)" master If the tests fail, the rebase is paused, which can be understandably disappointing if you walk away from it overnight only to have it fail after 10 minutes, though. :) - Emily

On the Git mailing list, Junio C Hamano wrote (reply to this):

Matt Rogers <[email protected]> writes: >> How are you reviewing the patches in your own series before sending >> them out? This round is better than the previous rounds where we >> didn't have a matching change to the tests so "make test" may not >> have passed in the middle of the series, though... >> > > I went through each patch individually using rebase -i and built/tested it. > Although just to save time I only did t1300 and t1308 since I believe those were > the only ones that should be affected. I can write a script that > would run the whole > test suite overnight for me and make sure the series shakes out okay, > if you'd like. What I like does not matter. What I pointed out for 04/10 wouldn't have been caught by your testing anyway, as both the code and the test had matching unnecessry changes. I was wondering if you are relying too heavily on just tests and without actually proofreading the changes to see if they still make sense in the context of the updated series, and if my suspicion was correct, if there are something reviewers can do to help the authors.

On the Git mailing list, Matt Rogers wrote (reply to this):

On Tue, Feb 11, 2020 at 1:10 AM Junio C Hamano <[email protected]> wrote: > > Matt Rogers <[email protected]> writes: > > >> How are you reviewing the patches in your own series before sending > >> them out? This round is better than the previous rounds where we > >> didn't have a matching change to the tests so "make test" may not > >> have passed in the middle of the series, though... > >> > > > > I went through each patch individually using rebase -i and built/tested it. > > Although just to save time I only did t1300 and t1308 since I believe those were > > the only ones that should be affected. I can write a script that > > would run the whole > > test suite overnight for me and make sure the series shakes out okay, > > if you'd like. > > What I like does not matter. > > What I pointed out for 04/10 wouldn't have been caught by your > testing anyway, as both the code and the test had matching > unnecessry changes. I was wondering if you are relying too heavily > on just tests and without actually proofreading the changes to see > if they still make sense in the context of the updated series, and > if my suspicion was correct, if there are something reviewers can do > to help the authors. > > I do try to proofread patches, I'm just not the most careful of reviewers at times, partially as a personal problem and partially as this is a new workflow for me. As for the particular issue, I just thought it was a good idea at the time and I didn't think it all the way through -- Matthew Rogers

-Original file line number
+Diff line change
@@ Expand Up / @@ -29,7 +29,7 @@ static int use_worktree_config; @@
     static struct git_config_source given_config_source;
     static int actions, type;
     static char *default_value;
-    static int end_null;
+    static int end_nul;
     static int respect_includes_opt = -1;
     static struct config_options config_options;
     static int show_origin;
@@ Expand Down Expand Up / @@ -151,7 +151,7 @@ static struct option builtin_config_options[] = { @@
     	OPT_CALLBACK_VALUE(0, "path", &type, N_("value is a path (file or directory name)"), TYPE_PATH),
     	OPT_CALLBACK_VALUE(0, "expiry-date", &type, N_("value is an expiry date"), TYPE_EXPIRY_DATE),
     	OPT_GROUP(N_("Other")),
-    	OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
+    	OPT_BOOL('z', "null", &end_nul, N_("terminate values with NUL byte")),
     	OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
     	OPT_BOOL(0, "includes", &respect_includes_opt, N_("respect include directives on lookup")),
     	OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")),
@@ Expand All / @@ -178,11 +178,11 @@ static void check_argc(int argc, int min, int max) @@
     static void show_config_origin(struct strbuf *buf)
     {
-    	const char term = end_null ? '\0' : '\t';
+    	const char term = end_nul ? '\0' : '\t';
     	strbuf_addstr(buf, current_config_origin_type());
     	strbuf_addch(buf, ':');
-    	if (end_null)
+    	if (end_nul)
     		strbuf_addstr(buf, current_config_name());
     	else
     		quote_c_style(current_config_name(), buf, NULL, 0);
@@ Expand Down Expand Up @@
     			!strcmp(given_config_source.file, "-")) {
     		given_config_source.file = NULL;
     		given_config_source.use_stdin = 1;
+    		given_config_source.scope = CONFIG_SCOPE_COMMAND;
     	}
     	if (use_global_config) {
@@ Expand All @@
     			 */
     			die(_("$HOME not set"));
+    		given_config_source.scope = CONFIG_SCOPE_GLOBAL;
     		if (access_or_warn(user_config, R_OK, 0) &&
     		    xdg_config && !access_or_warn(xdg_config, R_OK, 0)) {
     			given_config_source.file = xdg_config;
@@ Expand All @@
     			free(xdg_config);
     		}
     	}
-    	else if (use_system_config)
+    	else if (use_system_config) {
     		given_config_source.file = git_etc_gitconfig();
-    	else if (use_local_config)
+    		given_config_source.scope = CONFIG_SCOPE_SYSTEM;
+    	} else if (use_local_config) {
     		given_config_source.file = git_pathdup("config");
-    	else if (use_worktree_config) {
+    		given_config_source.scope = CONFIG_SCOPE_LOCAL;
+    	} else if (use_worktree_config) {
     		struct worktree **worktrees = get_worktrees(0);
     		if (repository_format_worktree_config)
     			given_config_source.file = git_pathdup("config.worktree");
@@ Expand All @@
     			      "section in \"git help worktree\" for details"));
     		else
     			given_config_source.file = git_pathdup("config");
+    		given_config_source.scope = CONFIG_SCOPE_LOCAL;
     		free_worktrees(worktrees);
     	} else if (given_config_source.file) {
     		if (!is_absolute_path(given_config_source.file) && prefix)
     			given_config_source.file =
     				prefix_filename(prefix, given_config_source.file);
+    		given_config_source.scope = CONFIG_SCOPE_COMMAND;
+    	} else if (given_config_source.blob) {
+    		given_config_source.scope = CONFIG_SCOPE_COMMAND;
     	}
     	if (respect_includes_opt == -1)
     		config_options.respect_includes = !given_config_source.file;
     	else
@@ Expand All @@
     		config_options.git_dir = get_git_dir();
     	}
-    	if (end_null) {
+    	if (end_nul) {
     		term = '\0';
     		delim = '\n';
     		key_delim = '\n';
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
     	char *xdg_config = xdg_config_home("config");
     	char *user_config = expand_user_path("~/.gitconfig", 0);
     	char *repo_config;
+    	enum config_scope prev_parsing_scope = current_parsing_scope;
     	if (opts->commondir)
     		repo_config = mkpathdup("%s/config", opts->commondir);
@@ Expand All @@
     	if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK))
     		ret += git_config_from_file(fn, user_config, data);
-    	current_parsing_scope = CONFIG_SCOPE_REPO;
+    	current_parsing_scope = CONFIG_SCOPE_LOCAL;
     	if (!opts->ignore_repo && repo_config &&
     	    !access_or_die(repo_config, R_OK, 0))
     		ret += git_config_from_file(fn, repo_config, data);
-    	/*
-    	 * Note: this should have a new scope, CONFIG_SCOPE_WORKTREE.
-    	 * But let's not complicate things before it's actually needed.
-    	 */
+    	current_parsing_scope = CONFIG_SCOPE_WORKTREE;
     	if (!opts->ignore_worktree && repository_format_worktree_config) {
     		char *path = git_pathdup("config.worktree");
     		if (!access_or_die(path, R_OK, 0))
     			ret += git_config_from_file(fn, path, data);
     		free(path);
     	}
-    	current_parsing_scope = CONFIG_SCOPE_CMDLINE;
+    	current_parsing_scope = CONFIG_SCOPE_COMMAND;
     	if (!opts->ignore_cmdline && git_config_from_parameters(fn, data) < 0)
     		die(_("unable to parse command-line config"));
-    	current_parsing_scope = CONFIG_SCOPE_UNKNOWN;
+    	current_parsing_scope = prev_parsing_scope;
     	free(xdg_config);
     	free(user_config);
     	free(repo_config);
@@ Expand All / @@ -1765,6 +1763,9 @@ int config_with_options(config_fn_t fn, void *data, @@
     		data = &inc;
     	}
+    	if (config_source)
+    		current_parsing_scope = config_source->scope;
     	/*
     	 * If we have a specific filename, use it. Otherwise, follow the
     	 * regular lookup sequence.
@@ Expand Down Expand Up / @@ -3297,6 +3298,26 @@ const char *current_config_origin_type(void) @@
     	}
     }
+    const char *config_scope_name(enum config_scope scope)
+    {
+    	switch (scope) {
+    	case CONFIG_SCOPE_SYSTEM:
+    		return "system";
+    	case CONFIG_SCOPE_GLOBAL:
+    		return "global";
+    	case CONFIG_SCOPE_LOCAL:
+    		return "local";
+    	case CONFIG_SCOPE_WORKTREE:
+    		return "worktree";
+    	case CONFIG_SCOPE_COMMAND:
+    		return "command";
+    	case CONFIG_SCOPE_SUBMODULE:
+    		return "submodule";
+    	default:
+    		return "unknown";
+    	}
+    }
     const char *current_config_name(void)
     {
     	const char *name;
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -35,10 +35,22 @@ struct object_id; @@
     #define CONFIG_REGEX_NONE ((void *)1)
+    enum config_scope {
+    	CONFIG_SCOPE_UNKNOWN = 0,
+    	CONFIG_SCOPE_SYSTEM,
+    	CONFIG_SCOPE_GLOBAL,
+    	CONFIG_SCOPE_LOCAL,
+    	CONFIG_SCOPE_WORKTREE,
+    	CONFIG_SCOPE_COMMAND,
+    	CONFIG_SCOPE_SUBMODULE,
+    };
+    const char *config_scope_name(enum config_scope scope);
     struct git_config_source {
     	unsigned int use_stdin:1;
     	const char *file;
     	const char *blob;
+    	enum config_scope scope;
     };
     enum config_origin_type {
@@ Expand Down Expand Up / @@ -294,14 +306,6 @@ int config_error_nonbool(const char *); @@
     int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
-    enum config_scope {
-    	CONFIG_SCOPE_UNKNOWN = 0,
-    	CONFIG_SCOPE_SYSTEM,
-    	CONFIG_SCOPE_GLOBAL,
-    	CONFIG_SCOPE_REPO,
-    	CONFIG_SCOPE_CMDLINE,
-    };
     enum config_scope current_config_scope(void);
     const char *current_config_origin_type(void);
     const char *current_config_name(void);
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
     	}
     	remote = make_remote(name, namelen);
     	remote->origin = REMOTE_CONFIG;
-    	if (current_config_scope() == CONFIG_SCOPE_REPO)
+    	if (current_config_scope() == CONFIG_SCOPE_LOCAL ||
+    	current_config_scope() == CONFIG_SCOPE_WORKTREE)
     		remote->configured_in_repo = 1;
     	if (!strcmp(subkey, "mirror"))
     		remote->mirror = git_config_bool(key, value);
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
     static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void *data)
     {
     	if (repo->worktree) {
-    		struct git_config_source config_source = { 0 };
+    		struct git_config_source config_source = {
+, .scope = CONFIG_SCOPE_SUBMODULE
+    		};
     		const struct config_options opts = { 0 };
     		struct object_id oid;
     		char *file;
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -37,21 +37,6 @@ @@
      *
      */
-    static const char *scope_name(enum config_scope scope)
-    {
-    	switch (scope) {
-    	case CONFIG_SCOPE_SYSTEM:
-    		return "system";
-    	case CONFIG_SCOPE_GLOBAL:
-    		return "global";
-    	case CONFIG_SCOPE_REPO:
-    		return "repo";
-    	case CONFIG_SCOPE_CMDLINE:
-    		return "cmdline";
-    	default:
-    		return "unknown";
-    	}
-    }
     static int iterate_cb(const char *var, const char *value, void *data)
     {
     	static int nr;
@@ Expand All @@
     	printf("value=%s\n", value ? value : "(null)");
     	printf("origin=%s\n", current_config_origin_type());
     	printf("name=%s\n", current_config_name());
-    	printf("scope=%s\n", scope_name(current_config_scope()));
+    	printf("scope=%s\n", config_scope_name(current_config_scope()));
     	return 0;
     }
@@ Expand Down @@

config: allow user to know scope of config options #478

Uh oh!

config: allow user to know scope of config options #478

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

gitgitgadget bot Dec 18, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Dec 19, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Dec 19, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Dec 18, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Dec 19, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Dec 19, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Dec 20, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Dec 21, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Dec 21, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Dec 21, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Jan 9, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Jan 9, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Jan 10, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Feb 10, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Feb 10, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Feb 10, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Jan 9, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Jan 9, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Jan 9, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Jan 9, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Jan 17, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Jan 18, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Jan 18, 2020

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Jan 17, 2020

Choose a reason for hiding this comment

Uh oh!