Re: [PATCH v2] config: add string mapping for enum config_scope

From: Emily Shaffer <emilyshaffer@google.com>
Date: Dec 12, 2019, 03:45 UTC
Message-ID: <20191212034547.GA226275@google.com>
In-Reply-To: <20191212031003.GA1196215@coredump.intra.peff.net>

On Wed, Dec 11, 2019 at 10:10:03PM -0500, Jeff King wrote:

Show 25 quoted lines

> On Wed, Dec 11, 2019 at 03:38:20PM -0800, Emily Shaffer wrote:
> 
> > If a user is interacting with their config files primarily by the 'git
> > config' command, using the location flags (--global, --system, etc) then
> > they may be more interested to see the scope of the config file they are
> > editing, rather than the filepath.
> 
> I don't mind adding this in, but I think we did discuss this same
> concept when we did "config --show-origin", and ended up showing the
> whole path, to help with a few cases:
> 
>   - you know you're getting a value from the "system" config, but you
>     don't know where that is (e.g., because the baked-in sysconfdir path
>     is something you didn't expect)
> 
>   - you're in a scope like "global", but the value actually comes from
>     an included file
> 
> Of course there's a flip-side, which is that showing "/etc/gitconfig"
> doesn't tell you that this is the "--system" file; the user has to infer
> that themselves.
> 
> There are no callers added here, so I'm not sure exactly how the new
> function is meant to be used. But I'd caution that it might be worth
> showing the scope _and_ the path, instead of one or the other.

Yeah, I hear you - I had added this originally to the config-based hooks topic to get an output like this:

$ git hook --list pre-commit 001 global ~/foo.sh 002 local ~/bar.sh

That's a scenario where it might be handy to add the path, especially if it's coming in via an import, sure. (For brevity I think I'd want to turn it on via an argument.)

As I was working through the comments on v3 of git-bugreport, though, I saw a request to add the origin of the configs - and that's a case where I don't necessarily want someone to see, say:

[Selected Configs] user.name (/home/emily/robot-revolution/stairclimber/.git/config) : Emily Shaffer

when I mail that bugreport to the Git list.

So, I think I hear what you're saying - use wisely - but I think it's OK for a user to say:

  printf("%s (%s): %s = %s\n",
         current_config_name(),
	 config_scope_to_string(current_config_scope()),
	 var,
	 value);

That is, I don't think the right solution is to make current_config_name() provide a stringification of current_config_scope() as well.

Or, I guess we can decide that the bugreport scenario is different enough that this helper should exist only there, and everybody else should use current_config_name().

 - Emily