Re: [PATCH] advice: use global config for default branch name

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

Vsevolod Myalitsin <ub4nal@mail.ru> writes:

> I considered using an "is_global(key)" helper, but I think adding
> a field to "advice_setting" is cleaner.
>
> The change is quite small:
>
>  struct advice_setting {
>      const char *key;
> +    int global_hint;
>      enum advice_level level;
>  };

Should it only about "global vs local"?  I am wondering if we ever
want to suggest "system".  In any case, these three things are
called "scope" in "git config --help", so perhaps rename the new
member to "config_scope" or "scope_hint" or something?

> Then the scope is specified directly for the relevant advice:
>
> 	-[ADVICE_DEFAULT_BRANCH_NAME] = { "defaultBranchName" },
> 	+[ADVICE_DEFAULT_BRANCH_NAME] = { "defaultBranchName", 1 },
>
> And used when building the hint:
>
> 	 static void vadvise(const char *advice, int display_instructions,
> 	-                    const char *key, va_list params)
> 	+                    const char *key, int global, va_list params)

Have you considered going in the other direction to narrow the
interface instead of widening?  Instead of passing .level and .key
separately from the caller to this function, I wonder if it makes
it more future-proof to pass &advice_setting[type].  A call in
advise_if_enabled() then would become

	vadvise(advice, &advice_settings[type], params);

and vadvise() is the only thing that needs to know what members are
in the advice_setting struct and how they affect the output.

> 	 {
> 	     ...
>  
> 	     if (display_instructions)
> 	-        strbuf_addf(&buf, turn_off_instructions, key);
> 	+        strbuf_addf(&buf, turn_off_instructions,
> 	+                    global ? "--global" : "", key);
> 	 }
>
> This keeps the information about the intended config scope in "advice_setting", rather than making "vadvise()" depend on specific advice keys.