Message ID | 97b8a7641d1ae76b9b4766a8f96c77af1aff55f5.1579275102.git.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | config: allow user to know scope of config options | expand |
"Matthew Rogers via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Matthew Rogers <mattr94@gmail.com> > > 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.
> 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...
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"; }