Re: [PATCH v3] advice: use global config for default branch name
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 9, 2026, 20:50 UTC
- Message-ID
- <xmqqse3ip2s5.fsf@gitster.g>
- In-Reply-To
- <20270829004959.90983-1-ub4nal@mail.ru>
Vsevolod Myalitsin <ub4nal@mail.ru> writes:
Show 10 quoted lines
> Some advice messages suggest disabling the advice with > "git config set advice.<name> false", even when the > corresponding configuration should be set at a different scope. > > Add a scope hint to advice settings so that the suggested > command uses the appropriate config scope. > > Pass the advice setting itself to vadvise() instead of passing > its fields separately. Use NULL for advise() calls that are not > associated with an advice setting.
"""Use this new mechanism to suggest setting advice.defaultBranchName in per-user configuration, not in per-repository configuration, as it is way too late once a repository is initialized.""" or something along that line is missing here.
Show 12 quoted lines
> +enum advice_scope {
> + ADVICE_SCOPE_LOCAL = 0,
> + ADVICE_SCOPE_GLOBAL,
> + ADVICE_SCOPE_SYSTEM,
> +};
> +
> +struct advice_setting {
> const char *key;
> + enum advice_scope scope_hint;
> enum advice_level level;
> -} advice_setting[] = {
> +};Looking good.
Show 44 quoted lines
> +static struct advice_setting advice_setting[] = {
> [ADVICE_ADD_EMBEDDED_REPO] = { "addEmbeddedRepo" },
> [ADVICE_ADD_EMPTY_PATHSPEC] = { "addEmptyPathspec" },
> [ADVICE_ADD_IGNORED_FILE] = { "addIgnoredFile" },
> @@ -51,7 +60,7 @@ static struct {
> [ADVICE_AM_WORK_DIR] = { "amWorkDir" },
> [ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME] = { "checkoutAmbiguousRemoteBranchName" },
> [ADVICE_COMMIT_BEFORE_MERGE] = { "commitBeforeMerge" },
> - [ADVICE_DEFAULT_BRANCH_NAME] = { "defaultBranchName" },
> + [ADVICE_DEFAULT_BRANCH_NAME] = { "defaultBranchName", ADVICE_SCOPE_GLOBAL },
> [ADVICE_DETACHED_HEAD] = { "detachedHead" },
> [ADVICE_DIVERGING] = { "diverging" },
> [ADVICE_FETCH_SET_HEAD_WARN] = { "fetchRemoteHEADWarn" },
> @@ -96,18 +105,31 @@ static struct {
>
> static const char turn_off_instructions[] =
> N_("\n"
> - "Disable this message with \"git config set advice.%s false\"");
> + "Disable this message with \"git config set%s advice.%s false\"");
>
> -static void vadvise(const char *advice, int display_instructions,
> - const char *key, va_list params)
> +static void vadvise(const char *advice,
> + const struct advice_setting *setting, va_list params)
> {
> struct strbuf buf = STRBUF_INIT;
> const char *cp, *np;
>
> strbuf_vaddf(&buf, advice, params);
>
> - if (display_instructions)
> - strbuf_addf(&buf, turn_off_instructions, key);
> + if (setting && setting->level == 0) {
> + const char *scope = "";
> + switch (setting->scope_hint) {
> + case ADVICE_SCOPE_LOCAL:
> + break;
> + case ADVICE_SCOPE_GLOBAL:
> + scope = " --global";
> + break;
> + case ADVICE_SCOPE_SYSTEM:
> + scope = " --system";
> + break;
> + }Style. In our codebase, switch and case are indented to the same tabstop.
Show 24 quoted lines
> + strbuf_addf(&buf, turn_off_instructions,
> + scope, setting->key);
> + }
>
> for (cp = buf.buf; *cp; cp = np) {
> np = strchrnul(cp, '\n');
> @@ -126,7 +148,7 @@ void advise(const char *advice, ...)
> {
> va_list params;
> va_start(params, advice);
> - vadvise(advice, 0, "", params);
> + vadvise(advice, NULL, params);
> va_end(params);
> }
>
> @@ -155,8 +177,7 @@ void advise_if_enabled(enum advice_type type, const char *advice, ...)
> return;
>
> va_start(params, advice);
> - vadvise(advice, !advice_setting[type].level, advice_setting[type].key,
> - params);
> + vadvise(advice, &advice_setting[type], params);
> va_end(params);
> }The change to narrow the interface into vadvise() needs to be described in the proposed log message.
Ideally, this would be a three-patch series. API change to vadvise() would come first, and then the introduction of advice scope mechanism, and finally making defaultBranchName a global scope variable.
Other than that, the end shape looks good to me.
Thanks.