Re: [PATCH v3 3/4] config: clarify meaning of command line scoping

2 messages, 2 authors, 2020-01-18 · open the first message on its own page

Re: [PATCH v3 3/4] config: clarify meaning of command line scoping

From: Junio C Hamano <hidden>
Date: 2020-01-17 21:00:53

"Matthew Rogers via GitGitGadget" [off-list ref] writes:
From: Matthew Rogers <redacted>

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.

quoted hunk
--- 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.

Re: [PATCH v3 3/4] config: clarify meaning of command line scoping

From: Matt Rogers <hidden>
Date: 2020-01-18 15:33:21

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.
quoted
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.
quoted
--- 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help