Re: [PATCH] advice: use global config for default branch name
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 8, 2026, 16:31 UTC
- Message-ID
- <xmqqik4fyaav.fsf@gitster.g>
- In-Reply-To
- <20260908185653.34702-1-ub4nal@mail.ru>
Vsevolod Myalitsin <ub4nal@mail.ru> writes:
Show 10 quoted lines
> 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?
Show 10 quoted lines
> 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.
Show 10 quoted lines
> {
> ...
>
> 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.