[v7,05/10] config: split repo scope to local and worktree
diff mbox series

Message ID 1af0237b8e76f0fd84ccac1e29d0cdc326303d7e.1581294660.git.gitgitgadget@gmail.com
State New
Headers show
Series
  • config: allow user to know scope of config options
Related show

Commit Message

Elijah Newren via GitGitGadget Feb. 10, 2020, 12:30 a.m. UTC
From: Matthew Rogers <mattr94@gmail.com>

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 <mattr94@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 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(-)

Comments

Junio C Hamano Feb. 10, 2020, 6:07 p.m. UTC | #1
"Matthew Rogers via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Matthew Rogers <mattr94@gmail.com>
>
> 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 <mattr94@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  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);
>  	}

Patch
diff mbox series

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);
 	}