From: Junio C Hamano Date: Tue, 08 Sep 2026 16:31:20 GMT Subject: Re: [PATCH] advice: use global config for default branch name Message-ID: In-Reply-To: <20260908185653.34702-1-ub4nal@mail.ru> Vsevolod Myalitsin 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.