On Wed, Dec 11, 2019 at 10:10:03PM -0500, Jeff King wrote:
> 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
