> > + switch (scope) {
> > + case CONFIG_SCOPE_LOCAL:
> > + return "local";
> > + case CONFIG_SCOPE_GLOBAL:
> > + return "global";
> > + case CONFIG_SCOPE_SYSTEM:
> > + return "system";
> > + case CONFIG_SCOPE_WORKTREE:
> > + return "worktree";
> > + case CONFIG_SCOPE_COMMAND:
> > + return "command";
> > + case CONFIG_SCOPE_SUBMODULE:
> > + return "submodule";
> > + default:
> > + return "unknown";
>
> Makes me wonder if this wants to be done via a table, which would
> later allow a parser to map in the other direction, taking a string
> and returning the corresponding enum, more easily (imagine a yet to
> be invented feature to "list config settings for only this scope").
I was thinking the same but then I realized such an option already exists,
--local and company.
>
> > + }
> > +}
> > +
> > +static void show_config_scope(struct strbuf *buf) {
> > + const char term = end_nul ? '\0' : '\t';
> > + const char *scope = scope_to_string(current_config_scope());
> > +
> > + strbuf_addstr(buf, N_(scope));
> > + strbuf_addch(buf, term);
> > +}
> > +
> > static int show_all_config(const char *key_, const char *value_, void *cb)
> > {
> > - if (show_origin) {
> > + if (show_origin || show_scope) {
> > struct strbuf buf = STRBUF_INIT;
> > - show_config_origin(&buf);
> > + if (show_scope)
> > + show_config_scope(&buf);
> > + if (show_origin)
> > + show_config_origin(&buf);
> > /* Use fwrite as "buf" can contain \0's if "end_null" is set. */
> > fwrite(buf.buf, 1, buf.len, stdout);
> > strbuf_release(&buf);
> > @@ -213,6 +245,8 @@ struct strbuf_list {
> >
> > static int format_config(struct strbuf *buf, const char *key_, const char *value_)
> > {
> > + if (show_scope)
> > + show_config_origin(buf);
> > if (show_origin)
> > show_config_origin(buf);
>
> Shouldn't one of these show scope instead of origin? I wonder how
> the tests I see later in the patch could have passed with this.
>
Oof, that was a blunder... I think this passes the tests because none of them
actually hits `format_config()` only `show_all_config()` so I'll definitely add
a test case for this. (and fix it while I'm at it)
> > diff --git a/t/helper/test-config.c b/t/helper/test-config.c
> > index 78bbb9eb98..aae2c6fc9e 100644
> > --- a/t/helper/test-config.c
> > +++ b/t/helper/test-config.c
> > @@ -48,8 +48,10 @@ static const char *scope_name(enum config_scope scope)
> > return "repo";
> > case CONFIG_SCOPE_WORKTREE:
> > return "worktree";
> > + case CONFIG_SCOPE_SUBMODULE:
> > + return "submodule";
> > case CONFIG_SCOPE_COMMAND:
> > - return "command";
> > + return "cmdline";
>
> Oh???
>
I definitely got a little too excited to get this patch in, so I
definitely apologize for that,
but thanks for the review.
--
Matthew Rogers
