From: Emily Shaffer <emilyshaffer@google.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Matthew Rogers <mattr94@gmail.com>,
Philip Oakley <philipoakley@iee.email>
Subject: Re: [PATCH v2] config: add string mapping for enum config_scope
Date: Wed, 11 Dec 2019 19:45:47 -0800 [thread overview]
Message-ID: <20191212034547.GA226275@google.com> (raw)
In-Reply-To: <20191212031003.GA1196215@coredump.intra.peff.net>
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
next prev parent reply other threads:[~2019-12-12 3:45 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-11 23:38 [PATCH v2] config: add string mapping for enum config_scope Emily Shaffer
2019-12-11 23:52 ` Matt Rogers
2019-12-12 3:10 ` Jeff King
2019-12-12 3:40 ` mattr94
2019-12-12 3:49 ` Jeff King
2019-12-12 3:45 ` Emily Shaffer [this message]
2019-12-12 3:55 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191212034547.GA226275@google.com \
--to=emilyshaffer@google.com \
--cc=git@vger.kernel.org \
--cc=mattr94@gmail.com \
--cc=peff@peff.net \
--cc=philipoakley@iee.email \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox