Re: [PATCH v7 05/10] config: split repo scope to local and worktree

[Junio C Hamano <gitster@pobox.com>]

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