{"thread":{"id":"66285","subject":"[PATCH] advice: use global config for default branch name","startedAt":"2026-09-07T12:56:29Z","lastAt":"2026-09-10T16:31:23Z","messageCount":19,"participants":["Vsevolod Myalitsin","Ben Knoble","R4NC","D. Ben Knoble","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"552138","messageId":"20260907125610.23458-1-ub4nal@mail.ru","threadId":"66285","inReplyTo":null,"subject":"[PATCH] advice: use global config for default branch name","fromName":"Vsevolod Myalitsin","fromEmail":"ub4nal@mail.ru","sentAt":"2026-09-07T12:56:09Z","receivedAt":"2026-09-07T12:56:29Z","isPatch":true,"body":"The advice for configuring the default branch name\nsuggests disabling it with \"git config set\nadvice.defaultBranchName false\". This setting is\nuseless because it neither affects the current\nrepository nor newly created repositories.\n\nSuggest using \"git config --global\" instead.\n\nSigned-off-by: Vsevolod Myalitsin \n---\n advice.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/advice.c b/advice.c\nindex 63bf8b0c5f..64ca4613b4 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -96,7 +96,7 @@ static struct {\n \n static const char turn_off_instructions[] =\n N_(\"\\n\"\n- \"Disable this message with \\\"git config set advice.%s false\\\"\");\n+ \"Disable this message with \\\"git config %s advice.%s false\\\"\");\n \n static void vadvise(const char *advice, int display_instructions,\n \t\t const char *key, va_list params)\n@@ -107,7 +107,8 @@ static void vadvise(const char *advice, int display_instructions,\n \tstrbuf_vaddf(&buf, advice, params);\n \n \tif (display_instructions)\n-\t\tstrbuf_addf(&buf, turn_off_instructions, key);\n+\t\tstrbuf_addf(&buf, turn_off_instructions,\n+\t\t\tstrcmp(key, \"defaultBranchName\") ? \"set\" : \"--global\", key);\n \n \tfor (cp = buf.buf; *cp; cp = np) {\n \t\tnp = strchrnul(cp, '\\n');\n-- \n2.50.1\n\n"},{"id":"552211","messageId":"90671DEB-7A41-47DA-B865-AB963AEC11D1@gmail.com","threadId":"66285","inReplyTo":"20260907125610.23458-1-ub4nal@mail.ru","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-09-08T13:35:04Z","receivedAt":"2026-09-08T13:35:23Z","isPatch":true,"body":"\n> Le 7 sept. 2026 à 09:02, Vsevolod Myalitsin a écrit :\n> \n> The advice for configuring the default branch name\n> suggests disabling it with \"git config set\n> advice.defaultBranchName false\". This setting is\n> useless because it neither affects the current\n> repository nor newly created repositories.\n\nMakes sense.\n\n> Suggest using \"git config --global\" instead.\n\nI think we should probably say “git config set --global …”\nusing the modern forms, no?\n\n> Signed-off-by: Vsevolod Myalitsin \n> ---\n> advice.c | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/advice.c b/advice.c\n> index 63bf8b0c5f..64ca4613b4 100644\n> --- a/advice.c\n> +++ b/advice.c\n> @@ -96,7 +96,7 @@ static struct {\n> \n> static const char turn_off_instructions[] =\n> N_(\"\\n\"\n> - \"Disable this message with \\\"git config set advice.%s false\\\"\");\n> + \"Disable this message with \\\"git config %s advice.%s false\\\"\");\n> \n> static void vadvise(const char *advice, int display_instructions,\n> const char *key, va_list params)\n> @@ -107,7 +107,8 @@ static void vadvise(const char *advice, int display_instructions,\n> strbuf_vaddf(&buf, advice, params);\n> \n> if (display_instructions)\n> - strbuf_addf(&buf, turn_off_instructions, key);\n> + strbuf_addf(&buf, turn_off_instructions,\n> + strcmp(key, \"defaultBranchName\") ? \"set\" : \"--global\", key);\n\nThis would be hard to extend later for other advice options that also make more sense at the global level. Perhaps extract a little helper is_global(key)?\n\nThanks"},{"id":"552214","messageId":"20260908185653.34702-1-ub4nal@mail.ru","threadId":"66285","inReplyTo":"90671DEB-7A41-47DA-B865-AB963AEC11D1@gmail.com","subject":"[PATCH] advice: use global config for default branch name","fromName":"Vsevolod Myalitsin","fromEmail":"ub4nal@mail.ru","sentAt":"2026-09-08T18:56:52Z","receivedAt":"2026-09-08T14:59:01Z","isPatch":true,"body":"\nI considered using an \"is_global(key)\" helper, but I think adding a field to \"advice_setting\" is cleaner.\n\nThe change is quite small:\n\n struct advice_setting {\n const char *key;\n+ int global_hint;\n enum advice_level level;\n };\n\nThen the scope is specified directly for the relevant advice:\n\n\t-[ADVICE_DEFAULT_BRANCH_NAME] = { \"defaultBranchName\" },\n\t+[ADVICE_DEFAULT_BRANCH_NAME] = { \"defaultBranchName\", 1 },\n\nAnd used when building the hint:\n\n\t static void vadvise(const char *advice, int display_instructions,\n\t- const char *key, va_list params)\n\t+ const char *key, int global, va_list params)\n\t {\n\t ...\n \n\t if (display_instructions)\n\t- strbuf_addf(&buf, turn_off_instructions, key);\n\t+ strbuf_addf(&buf, turn_off_instructions,\n\t+ global ? \"--global\" : \"\", key);\n\t }\n\nThis keeps the information about the intended config scope in \"advice_setting\", rather than making \"vadvise()\" depend on specific advice keys.\n"},{"id":"552215","messageId":"7D54AA3C-0724-4C8A-9CB8-64150CD3A051@gmail.com","threadId":"66285","inReplyTo":"20260908185653.34702-1-ub4nal@mail.ru","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-09-08T14:59:53Z","receivedAt":"2026-09-08T15:00:09Z","isPatch":true,"body":"\n> Le 8 sept. 2026 à 10:43, Vsevolod Myalitsin a écrit :\n> \n> \n> I considered using an \"is_global(key)\" helper, but I think adding a field to \"advice_setting\" is cleaner.\n> \n> The change is quite small:\n> \n> struct advice_setting {\n> const char *key;\n> + int global_hint;\n> enum advice_level level;\n> };\n> \n> Then the scope is specified directly for the relevant advice:\n> \n> -[ADVICE_DEFAULT_BRANCH_NAME] = { \"defaultBranchName\" },\n> +[ADVICE_DEFAULT_BRANCH_NAME] = { \"defaultBranchName\", 1 },\n> \n> And used when building the hint:\n> \n> static void vadvise(const char *advice, int display_instructions,\n> - const char *key, va_list params)\n> + const char *key, int global, va_list params)\n> {\n> ...\n> \n> if (display_instructions)\n> - strbuf_addf(&buf, turn_off_instructions, key);\n> + strbuf_addf(&buf, turn_off_instructions,\n> + global ? \"--global\" : \"\", key);\n> }\n> \n> This keeps the information about the intended config scope in \"advice_setting\", rather than making \"vadvise()\" depend on specific advice keys.\n\nThat also seems good to me. I think I prefer it. \n\nPS it is normal here to bottom-post and quote at least the\nrelevant parts of the message to which you reply ;)"},{"id":"552221","messageId":"7a77ce52-b7d4-4818-9b9b-052d5922db2f@mail.ru","threadId":"66285","inReplyTo":"7D54AA3C-0724-4C8A-9CB8-64150CD3A051@gmail.com","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"R4NC","fromEmail":"ub4nal@mail.ru","sentAt":"2026-09-08T19:56:05Z","receivedAt":"2026-09-08T16:00:06Z","isPatch":true,"body":"> That also seems good to me. I think I prefer it.\n>\n> PS it is normal here to bottom-post and quote at least the\n> relevant parts of the message to which you reply 😉\n\n\nThank you for the review and for the formatting advice.\nI will send v2 of the patch with the global_hint field added as suggested.\nBy the way, is my reply formatting correct this time?\n\n"},{"id":"552223","messageId":"CALnO6CAHZXT5rZtwTTXwCFvBELRjzxUmRW3pB6KVyE7ERFJqHg@mail.gmail.com","threadId":"66285","inReplyTo":"7a77ce52-b7d4-4818-9b9b-052d5922db2f@mail.ru","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-09-08T16:24:33Z","receivedAt":"2026-09-08T16:24:45Z","isPatch":true,"body":"On Tue, Sep 8, 2026 at 11:59 AM R4NC wrote:\n>\n> > PS it is normal here to bottom-post and quote at least the\n> > relevant parts of the message to which you reply 😉\n\n[snip]\n\n> By the way, is my reply formatting correct this time?\n\nI think so, anyway :)\n\nThanks again!\n\n-- \nD. Ben Knoble\n"},{"id":"552224","messageId":"xmqqik4fyaav.fsf@gitster.g","threadId":"66285","inReplyTo":"20260908185653.34702-1-ub4nal@mail.ru","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-08T16:31:20Z","receivedAt":"2026-09-08T16:31:25Z","isPatch":true,"body":"Vsevolod Myalitsin writes:\n\n> I considered using an \"is_global(key)\" helper, but I think adding\n> a field to \"advice_setting\" is cleaner.\n>\n> The change is quite small:\n>\n> struct advice_setting {\n> const char *key;\n> + int global_hint;\n> enum advice_level level;\n> };\n\nShould it only about \"global vs local\"? I am wondering if we ever\nwant to suggest \"system\". In any case, these three things are\ncalled \"scope\" in \"git config --help\", so perhaps rename the new\nmember to \"config_scope\" or \"scope_hint\" or something?\n\n> Then the scope is specified directly for the relevant advice:\n>\n> \t-[ADVICE_DEFAULT_BRANCH_NAME] = { \"defaultBranchName\" },\n> \t+[ADVICE_DEFAULT_BRANCH_NAME] = { \"defaultBranchName\", 1 },\n>\n> And used when building the hint:\n>\n> \t static void vadvise(const char *advice, int display_instructions,\n> \t- const char *key, va_list params)\n> \t+ const char *key, int global, va_list params)\n\nHave you considered going in the other direction to narrow the\ninterface instead of widening? Instead of passing .level and .key\nseparately from the caller to this function, I wonder if it makes\nit more future-proof to pass &advice_setting[type]. A call in\nadvise_if_enabled() then would become\n\n\tvadvise(advice, &advice_settings[type], params);\n\nand vadvise() is the only thing that needs to know what members are\nin the advice_setting struct and how they affect the output.\n\n> \t {\n> \t ...\n> \n> \t if (display_instructions)\n> \t- strbuf_addf(&buf, turn_off_instructions, key);\n> \t+ strbuf_addf(&buf, turn_off_instructions,\n> \t+ global ? \"--global\" : \"\", key);\n> \t }\n>\n> This keeps the information about the intended config scope in \"advice_setting\", rather than making \"vadvise()\" depend on specific advice keys.\n"},{"id":"552239","messageId":"20260908213840.37833-1-ub4nal@mail.ru","threadId":"66285","inReplyTo":"xmqqik4fyaav.fsf@gitster.g","subject":"[PATCH] advice: use global config for default branch name","fromName":"Vsevolod Myalitsin","fromEmail":"ub4nal@mail.ru","sentAt":"2026-09-08T21:38:39Z","receivedAt":"2026-09-08T17:50:49Z","isPatch":true,"body":"Hi Junio,\n\n> Should it only about \"global vs local\"? I am wondering if we ever\n> want to suggest \"system\". In any case, these three things are\n> called \"scope\" in \"git config --help\", so perhaps rename the new\n> member to \"config_scope\" or \"scope_hint\" or something?\n\nAgreed. I will rename \"global_hint\" to \"scope_hint\" so that the field\ndescribes the configuration scope rather than just the global case.\n\n> Have you considered going in the other direction to narrow the\n> interface instead of widening? Instead of passing .level and .key\n> separately from the caller to this function, I wonder if it makes\n> it more future-proof to pass &advice_setting[type].\n\nYes, I agree that passing the \"advice_setting\" itself is cleaner and\nmore future-proof. I will change \"vadvise()\" to take a pointer to the\ncorresponding \"advice_setting\" instead.\n\nUnfortunately, I did not notice your message in time and had already\nsent v2. I will implement these changes in v3.\n\nThanks for the suggestions.\n\nBest,\nVsevolod R4NC\n"},{"id":"552244","messageId":"xmqqik4fwoz5.fsf@gitster.g","threadId":"66285","inReplyTo":"20260908213840.37833-1-ub4nal@mail.ru","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-08T18:57:18Z","receivedAt":"2026-09-08T18:57:21Z","isPatch":true,"body":"Vsevolod Myalitsin writes:\n\n> Yes, I agree that passing the \"advice_setting\" itself is cleaner and\n> more future-proof. I will change \"vadvise()\" to take a pointer to the\n> corresponding \"advice_setting\" instead.\n\nOne minor glitch is that there is an ad-hoc vadvise() call in\nadvise() that is not tied to any particular entry in the\nadvise_setting[] table. I think we'd need to give a name to the\nadvice_setting struct type, instanciate an ad-hoc instance on stack,\nand pass it down the callchain, perhaps like so:\n\n\tvoid advise(const char *advice, ...)\n\t{\n\t\tstruct advice_setting ad_hoc = {\n\t\t\t.key = \"\",\n\t\t\t.scope = CONFIG_SCOPE_UNKNOWN,\n\t\t\t.level = 0,\n\t\t};\n\t\tva_list params;\n\n\t\tva_start(params, advise);\n\t\tvadvise(advise, &ad_hoc, params);\n\t\tva_end(params);\n\t}\n\n\n"},{"id":"552294","messageId":"9b4f43d7-ba77-4859-8efe-facdec6ec5aa@mail.ru","threadId":"66285","inReplyTo":"xmqqik4fwoz5.fsf@gitster.g","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"R4NC","fromEmail":"ub4nal@mail.ru","sentAt":"2026-09-09T06:49:17Z","receivedAt":"2026-09-09T06:49:37Z","isPatch":true,"body":" > One minor glitch is that there is an ad-hoc vadvise() call in\n > advise() that is not tied to any particular entry in the\n > advise_setting[] table.\n\nI agree that we should use a separate \"advice_setting\" structure for this.\n\n > I think we'd need to give a name to the advice_setting struct type,\n > instanciate an ad-hoc instance on stack, and pass it down the callchain.\n\nI agree. However, \"advise()\" originally passed \"0\" for \n\"display_instructions\",\nwhile \"advise_if_enabled()\" passed the negation of \"level\". With the new \ninterface,\nwe need a non-zero value for the ad-hoc setting to suppress the \ninstructions.\nUsing \"ADVICE_LEVEL_ENABLED\" or \"ADVICE_LEVEL_DISABLED\" would be a hack.\n\n\nI suggest adding a dedicated \"ADVICE_LEVEL_UNKNOWN\" value to \"enum \nadvice_level\" for this case.\n"},{"id":"552337","messageId":"20260909155440.GA94069@coredump.intra.peff.net","threadId":"66285","inReplyTo":"xmqqik4fwoz5.fsf@gitster.g","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-09-09T15:54:40Z","receivedAt":"2026-09-09T15:54:48Z","isPatch":true,"body":"On Tue, Sep 08, 2026 at 11:57:18AM -0700, Junio C Hamano wrote:\n\n> Vsevolod Myalitsin writes:\n> \n> > Yes, I agree that passing the \"advice_setting\" itself is cleaner and\n> > more future-proof. I will change \"vadvise()\" to take a pointer to the\n> > corresponding \"advice_setting\" instead.\n> \n> One minor glitch is that there is an ad-hoc vadvise() call in\n> advise() that is not tied to any particular entry in the\n> advise_setting[] table. I think we'd need to give a name to the\n> advice_setting struct type, instanciate an ad-hoc instance on stack,\n> and pass it down the callchain, perhaps like so:\n\nIsn't this a natural fit for NULL? That ad-hoc call wants to pass the\nnotion that there is no matching advice config (or at least not that it\nknows about). And then vadvise() can check:\n\n if (conf && !conf->level)\n\t...show instructions...\n\nwhich seems natural to me.\n\nAs a side note, I think this is revealing some existing shortcomings in\nthe callers. Most of the calls to advise() are doing something like:\n\n if (advice_is_enabled(ADVICE_FOO))\n\tadvise(\"ask your doctor about foo\");\n\nThose won't get the \"turn this off with advice.foo instructions\". Only:\n\n advise_if_enabled(ADVICE_FOO, \"ask your doctor about foo\");\n\nwill. So there are many missed opportunities for offering the turn-off\ninstructions. Nobody seems to have complained, which makes me wonder if\nthe turn-off instructions would be annoyingly chatty if we printed them\nall the time. Most of those calls predate the addition if the turn-off\ninstructions and advise_if_enabled(), which was added in 2020. I wonder\nhow people would feel if we converted them all and started printing the\nturn-off instructions everywhere.\n\nAnyway, UI philosophizing aside, another obvious pattern for advise()\nis:\n\n if (advice_is_enabled(ADVICE_FOO)) {\n\t/* do lots of work */\n\tadvise(\"try %s\", results_of_work);\n }\n\nwhich _wouldn't_ want to convert to advise_if_enabled(). If that wants\nthe turn-off message, we'd want to be able to pass the advice enum to\nadvise(), like:\n\n advise(ADVICE_FOO, \"try %s\", results_of_work);\n\nat which point we might need a way to pass the NULL advice marker\nsomehow (for those cases which really aren't tied to a config value,\nthough arguably that is an anti-pattern in itself).\n\nI guess the caller could just do:\n\n advise_if_enabled(ADVICE_FOO, ...);\n\ninside the block. We know that it's enabled, but it's not like the check\nis expensive.\n\n-Peff\n"},{"id":"552346","messageId":"xmqqv78eqmw8.fsf@gitster.g","threadId":"66285","inReplyTo":"20260909155440.GA94069@coredump.intra.peff.net","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-09T18:51:03Z","receivedAt":"2026-09-09T18:51:05Z","isPatch":true,"body":"Jeff King writes:\n\n> On Tue, Sep 08, 2026 at 11:57:18AM -0700, Junio C Hamano wrote:\n>\n>> Vsevolod Myalitsin writes:\n>> \n>> > Yes, I agree that passing the \"advice_setting\" itself is cleaner and\n>> > more future-proof. I will change \"vadvise()\" to take a pointer to the\n>> > corresponding \"advice_setting\" instead.\n>> \n>> One minor glitch is that there is an ad-hoc vadvise() call in\n>> advise() that is not tied to any particular entry in the\n>> advise_setting[] table. I think we'd need to give a name to the\n>> advice_setting struct type, instanciate an ad-hoc instance on stack,\n>> and pass it down the callchain, perhaps like so:\n>\n> Isn't this a natural fit for NULL?\n\nPerfect.\n\n> As a side note, I think this is revealing some existing shortcomings in\n> the callers. Most of the calls to advise() are doing something like:\n>\n> if (advice_is_enabled(ADVICE_FOO))\n> \tadvise(\"ask your doctor about foo\");\n\nYes, but all of these callers call advice_enabled() without _is ;-)\n\n>\n> Those won't get the \"turn this off with advice.foo instructions\". Only:\n>\n> advise_if_enabled(ADVICE_FOO, \"ask your doctor about foo\");\n>\n> will. So there are many missed opportunities for offering the turn-off\n> instructions. Nobody seems to have complained, which makes me wonder if\n> the turn-off instructions would be annoyingly chatty if we printed them\n> all the time. Most of those calls predate the addition if the turn-off\n> instructions and advise_if_enabled(), which was added in 2020. I wonder\n> how people would feel if we converted them all and started printing the\n> turn-off instructions everywhere.\n\nDepends on how we do so, I guess. Do you mean we should rewrite\nadvise() call above to advice_if_enabled(), even though the check\nfor ADVICE_FOO token appear redundant?\n\n> Anyway, UI philosophizing aside, another obvious pattern for advise()\n> is:\n>\n> if (advice_is_enabled(ADVICE_FOO)) {\n> \t/* do lots of work */\n> \tadvise(\"try %s\", results_of_work);\n> }\n\nYes, checking with is-enabled primarily for the purpose of skipping\n\"do lots of work\" is a very typical use. I do not know why you\nassume ...\n\n>\n> which _wouldn't_ want to convert to advise_if_enabled().\n\n... this \"try X\" is something the users would not want to learn how\nto disable, but assuming it is not, the existing code above as-is\nshould be what we want.\n\n> If that wants\n> the turn-off message, we'd want to be able to pass the advice enum to\n> advise(), like:\n>\n> advise(ADVICE_FOO, \"try %s\", results_of_work);\n>\n> at which point we might need a way to pass the NULL advice marker\n> somehow (for those cases which really aren't tied to a config value,\n> though arguably that is an anti-pattern in itself).\n>\n> I guess the caller could just do:\n>\n> advise_if_enabled(ADVICE_FOO, ...);\n>\n> inside the block. We know that it's enabled, but it's not like the check\n> is expensive.\n\nYes, I think we already have some callers that do so, in a pattern\nwhere they want to skip the \"do lots of work\" part. Or at least I\nthink I suggested the pattern in the past for somebody who wanted to\ndo that.\n\n"},{"id":"552370","messageId":"20260909195132.GA182066@coredump.intra.peff.net","threadId":"66285","inReplyTo":"xmqqv78eqmw8.fsf@gitster.g","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-09-09T19:51:32Z","receivedAt":"2026-09-09T19:51:35Z","isPatch":true,"body":"On Wed, Sep 09, 2026 at 11:51:03AM -0700, Junio C Hamano wrote:\n\n> > will. So there are many missed opportunities for offering the turn-off\n> > instructions. Nobody seems to have complained, which makes me wonder if\n> > the turn-off instructions would be annoyingly chatty if we printed them\n> > all the time. Most of those calls predate the addition if the turn-off\n> > instructions and advise_if_enabled(), which was added in 2020. I wonder\n> > how people would feel if we converted them all and started printing the\n> > turn-off instructions everywhere.\n> \n> Depends on how we do so, I guess. Do you mean we should rewrite\n> advise() call above to advice_if_enabled(), even though the check\n> for ADVICE_FOO token appear redundant?\n\nI mean we could mechanically rewrite:\n\n if (advice_enabled(ADVICE_FOO))\n\tadvise(...);\n\nto:\n\n advise_if_enabled(ADVICE_FOO, ...);\n\nSo the check wouldn't be redundant, but rather folded into the helper\nfunction. The code becomes shorter, and the user-visible behavior\nchanges to produce the extra \"turn-off\" message.\n\n> > Anyway, UI philosophizing aside, another obvious pattern for advise()\n> > is:\n> >\n> > if (advice_is_enabled(ADVICE_FOO)) {\n> > \t/* do lots of work */\n> > \tadvise(\"try %s\", results_of_work);\n> > }\n> \n> Yes, checking with is-enabled primarily for the purpose of skipping\n> \"do lots of work\" is a very typical use. I do not know why you\n> assume ...\n> \n> >\n> > which _wouldn't_ want to convert to advise_if_enabled().\n> \n> ... this \"try X\" is something the users would not want to learn how\n> to disable, but assuming it is not, the existing code above as-is\n> should be what we want.\n\nI meant only that they would not want the same mechanical conversion\nabove, because that would lose the ability to avoid the extra work.\n\n> > I guess the caller could just do:\n> >\n> > advise_if_enabled(ADVICE_FOO, ...);\n> >\n> > inside the block. We know that it's enabled, but it's not like the check\n> > is expensive.\n> \n> Yes, I think we already have some callers that do so, in a pattern\n> where they want to skip the \"do lots of work\" part. Or at least I\n> think I suggested the pattern in the past for somebody who wanted to\n> do that.\n\nI think we do the same thing with trace_want() in a few spots.\n\n-Peff\n"},{"id":"552374","messageId":"20270829005902.91081-1-ub4nal@mail.ru","threadId":"66285","inReplyTo":"xmqqv78eqmw8.fsf@gitster.g","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Vsevolod Myalitsin","fromEmail":"ub4nal@mail.ru","sentAt":"2027-08-29T00:59:01Z","receivedAt":"2026-09-09T20:05:33Z","isPatch":true,"body":"Hi,\n\nI've sent v3 with the suggested changes.\n\nIn particular, v3 uses NULL for advise() calls that are not associated with an advice_setting entry, as suggested by Peff.\n\nPlease continue the discussion based on v3.\n\nThanks,\nVsevolod\n"},{"id":"552392","messageId":"xmqq8q594tvs.fsf@gitster.g","threadId":"66285","inReplyTo":"20260909195132.GA182066@coredump.intra.peff.net","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-10T04:23:19Z","receivedAt":"2026-09-10T04:23:21Z","isPatch":true,"body":"Jeff King writes:\n\n> On Wed, Sep 09, 2026 at 11:51:03AM -0700, Junio C Hamano wrote:\n>\n>> > will. So there are many missed opportunities for offering the turn-off\n>> > instructions. Nobody seems to have complained, which makes me wonder if\n>> > the turn-off instructions would be annoyingly chatty if we printed them\n>> > all the time. Most of those calls predate the addition if the turn-off\n>> > instructions and advise_if_enabled(), which was added in 2020. I wonder\n>> > how people would feel if we converted them all and started printing the\n>> > turn-off instructions everywhere.\n>> \n>> Depends on how we do so, I guess. Do you mean we should rewrite\n>> advise() call above to advice_if_enabled(), even though the check\n>> for ADVICE_FOO token appear redundant?\n>\n> I mean we could mechanically rewrite:\n>\n> if (advice_enabled(ADVICE_FOO))\n> \tadvise(...);\n>\n> to:\n>\n> advise_if_enabled(ADVICE_FOO, ...);\n\nSurely, and I think we are pretty much on the same page. Such a\nmechanical rewrite is not too bad. Here is what I came up with:\n\n $ edit tools/coccinelle/advice.cocci\n $ make coccicheck\n $ git add -N tools/coccinelle/advice.cocci\n $ git apply .build/tools/coccinelle/ALL.cocci.patch\n $ git add -p\n\n Some of the hunks I simply accepted with (y), but most of them\n needed (e)dit to make them presentable; otherwise we ended up\n with too many overly long lines.\n\n\n\n tools/coccinelle/advice.cocci | 7 +++++++\n advice.c | 13 ++++---------\n branch.c | 6 +++---\n builtin/am.c | 4 ++--\n builtin/checkout.c | 4 ++--\n builtin/submodule--helper.c | 4 ++--\n sequencer.c | 9 ++++-----\n 7 files changed, 24 insertions(+), 23 deletions(-)\n\ndiff --git c/tools/coccinelle/advice.cocci w/tools/coccinelle/advice.cocci\nnew file mode 100644\nindex 0000000000..da4851c5d0\n--- /dev/null\n+++ w/tools/coccinelle/advice.cocci\n@@ -0,0 +1,7 @@\n+@@\n+expression A;\n+expression list args;\n+@@\n+-if (advice_enabled(A))\n+-\tadvise(args);\n++advice_if_enabled(A, args);\ndiff --git c/advice.c w/advice.c\nindex 63bf8b0c5f..c60b33ee33 100644\n--- c/advice.c\n+++ w/advice.c\n@@ -216,13 +216,8 @@ int error_resolve_conflict(const char *me)\n \telse\n \t\tBUG(\"Unhandled conflict reason '%s'\", me);\n \n-\tif (advice_enabled(ADVICE_RESOLVE_CONFLICT))\n-\t\t/*\n-\t\t * Message used both when 'git commit' fails and when\n-\t\t * other commands doing a merge do.\n-\t\t */\n-\t\tadvise(_(\"Fix them up in the work tree, and then use 'git add/rm '\\n\"\n-\t\t\t \"as appropriate to mark resolution and make a commit.\"));\n+\tadvice_if_enabled(ADVICE_RESOLVE_CONFLICT,\n+\t\t\t _(\"Fix them up in the work tree, and then use 'git add/rm '\\n\" \"as appropriate to mark resolution and make a commit.\"));\n \treturn -1;\n }\n \n@@ -235,8 +230,8 @@ void NORETURN die_resolve_conflict(const char *me)\n void NORETURN die_conclude_merge(void)\n {\n \terror(_(\"You have not concluded your merge (MERGE_HEAD exists).\"));\n-\tif (advice_enabled(ADVICE_RESOLVE_CONFLICT))\n-\t\tadvise(_(\"Please, commit your changes before merging.\"));\n+\tadvice_if_enabled(ADVICE_RESOLVE_CONFLICT,\n+\t\t\t _(\"Please, commit your changes before merging.\"));\n \tdie(_(\"Exiting because of unfinished merge.\"));\n }\n \ndiff --git c/branch.c w/branch.c\nindex 22f4f46b96..a87facd311 100644\n--- c/branch.c\n+++ w/branch.c\n@@ -812,9 +812,9 @@ void create_branches_recursively(struct repository *r, const char *name,\n \t\t\tint code = die_message(\n \t\t\t\t_(\"submodule '%s': unable to find submodule\"),\n \t\t\t\tsubmodule_entry_list.entries[i].submodule->name);\n-\t\t\tif (advice_enabled(ADVICE_SUBMODULES_NOT_UPDATED))\n-\t\t\t\tadvise(_(\"You may try updating the submodules using 'git checkout --no-recurse-submodules %s && git submodule update --init'\"),\n-\t\t\t\t start_committish);\n+\t\t\tadvice_if_enabled(ADVICE_SUBMODULES_NOT_UPDATED,\n+\t\t\t\t\t _(\"You may try updating the submodules using 'git checkout --no-recurse-submodules %s && git submodule update --init'\"),\n+\t\t\t\t\t start_committish);\n \t\t\texit(code);\n \t\t}\n \ndiff --git c/builtin/am.c w/builtin/am.c\nindex e9623b8307..6039b69475 100644\n--- c/builtin/am.c\n+++ w/builtin/am.c\n@@ -1910,8 +1910,8 @@ static void am_run(struct am_state *state, int resume)\n \t\t\tprintf_ln(_(\"Patch failed at %s %.*s\"), msgnum(state),\n \t\t\t\tlinelen(state->msg), state->msg);\n \n-\t\t\tif (advice_enabled(ADVICE_AM_WORK_DIR))\n-\t\t\t\tadvise(_(\"Use 'git am --show-current-patch=diff' to see the failed patch\"));\n+\t\t\tadvice_if_enabled(ADVICE_AM_WORK_DIR,\n+\t\t\t\t\t _(\"Use 'git am --show-current-patch=diff' to see the failed patch\"));\n \n \t\t\tdie_user_resolve(state);\n \t\t}\ndiff --git c/builtin/checkout.c w/builtin/checkout.c\nindex 2bc21aa49b..34f05d2381 100644\n--- c/builtin/checkout.c\n+++ w/builtin/checkout.c\n@@ -1612,8 +1612,8 @@ static void die_expecting_a_branch(const struct branch_info *branch_info)\n \t\t */\n \t\tcode = die_message(_(\"a branch is expected, got '%s'\"), branch_info->name);\n \n-\tif (advice_enabled(ADVICE_SUGGEST_DETACHING_HEAD))\n-\t\tadvise(_(\"If you want to detach HEAD at the commit, try again with the --detach option.\"));\n+\tadvice_if_enabled(ADVICE_SUGGEST_DETACHING_HEAD,\n+\t\t\t _(\"If you want to detach HEAD at the commit, try again with the --detach option.\"));\n \n \texit(code);\n }\ndiff --git c/builtin/submodule--helper.c w/builtin/submodule--helper.c\nindex e7cd3225fa..5e4989a9aa 100644\n--- c/builtin/submodule--helper.c\n+++ w/builtin/submodule--helper.c\n@@ -1806,8 +1806,8 @@ static int add_possible_reference_from_superproject(\n \t\t} else {\n \t\t\tswitch (sas->error_mode) {\n \t\t\tcase SUBMODULE_ALTERNATE_ERROR_DIE:\n-\t\t\t\tif (advice_enabled(ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE))\n-\t\t\t\t\tadvise(_(alternate_error_advice));\n+\t\t\t\tadvice_if_enabled(ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,\n+\t\t\t\t\t\t _(alternate_error_advice));\n \t\t\t\tdie(_(\"submodule '%s' cannot add alternate: %s\"),\n \t\t\t\t sas->submodule_name, err.buf);\n \t\t\tcase SUBMODULE_ALTERNATE_ERROR_INFO:\ndiff --git c/sequencer.c w/sequencer.c\nindex 65afd100d9..6d8be0c036 100644\n--- c/sequencer.c\n+++ w/sequencer.c\n@@ -624,8 +624,8 @@ static int error_dirty_index(struct repository *repo, struct replay_opts *opts)\n \terror(_(\"your local changes would be overwritten by %s.\"),\n \t\t_(action_name(opts)));\n \n-\tif (advice_enabled(ADVICE_COMMIT_BEFORE_MERGE))\n-\t\tadvise(_(\"commit your changes or stash them to proceed.\"));\n+\tadvice_if_enabled(ADVICE_COMMIT_BEFORE_MERGE,\n+\t\t\t _(\"commit your changes or stash them to proceed.\"));\n \treturn -1;\n }\n \n@@ -3497,9 +3497,8 @@ static int create_seq_dir(struct repository *r)\n \t}\n \tif (in_progress_error) {\n \t\terror(\"%s\", in_progress_error);\n-\t\tif (advice_enabled(ADVICE_SEQUENCER_IN_USE))\n-\t\t\tadvise(in_progress_advice,\n-\t\t\t\tadvise_skip ? \"--skip | \" : \"\");\n+\t\tadvice_if_enabled(ADVICE_SEQUENCER_IN_USE, in_progress_advice,\n+\t\t\t\t advise_skip ? \"--skip | \" : \"\");\n \t\treturn -1;\n \t}\n \tif (mkdir(git_path_seq_dir(), 0777) < 0)\n"},{"id":"552393","messageId":"xmqq5x0d4tmh.fsf@gitster.g","threadId":"66285","inReplyTo":"20260909195132.GA182066@coredump.intra.peff.net","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-10T04:28:54Z","receivedAt":"2026-09-10T04:28:57Z","isPatch":true,"body":"Jeff King writes:\n\n> On Wed, Sep 09, 2026 at 11:51:03AM -0700, Junio C Hamano wrote:\n>\n>> > will. So there are many missed opportunities for offering the turn-off\n>> > instructions. Nobody seems to have complained, which makes me wonder if\n>> > the turn-off instructions would be annoyingly chatty if we printed them\n>> > all the time. Most of those calls predate the addition if the turn-off\n>> > instructions and advise_if_enabled(), which was added in 2020. I wonder\n>> > how people would feel if we converted them all and started printing the\n>> > turn-off instructions everywhere.\n>> \n>> Depends on how we do so, I guess. Do you mean we should rewrite\n>> advise() call above to advice_if_enabled(), even though the check\n>> for ADVICE_FOO token appear redundant?\n>\n> I mean we could mechanically rewrite:\n>\n> if (advice_enabled(ADVICE_FOO))\n> \tadvise(...);\n>\n> to:\n>\n> advise_if_enabled(ADVICE_FOO, ...);\n\nSurely, and I think we are pretty much on the same page. Such a\nmechanical rewrite is not too bad. Here is what I came up with:\n\n $ edit tools/coccinelle/advice.cocci\n $ make coccicheck\n $ git add -N tools/coccinelle/advice.cocci\n $ git apply .build/tools/coccinelle/ALL.cocci.patch\n $ git add -p\n\n Some of the hunks I simply accepted with (y), but most of them\n needed (e)dit to make them presentable; otherwise we ended up\n with too many overly long lines and losing some comments.\n\n--- >8 ---\nSubject: [PATCH] advice: use advise_if_enabled() more\n\nOne very common pattern is\n\n\tif (advice_enabled(ADVICE_FOO))\n\t\tadvise(_(\"MESSAGE FOR FOO\"));\n\nbut we have a perfect short-hand for that. Using coccinelle,\nrewrite the above as\n\n\tadvise_if_enabled(ADVICE_FOO, _(\"MESSAGE FOR FOR\"));\n\nSigned-off-by: Junio C Hamano \n---\n advice.c | 13 ++++---------\n branch.c | 6 +++---\n builtin/am.c | 4 ++--\n builtin/checkout.c | 4 ++--\n builtin/submodule--helper.c | 4 ++--\n sequencer.c | 9 ++++-----\n tools/coccinelle/advice.cocci | 7 +++++++\n 7 files changed, 24 insertions(+), 23 deletions(-)\n create mode 100644 tools/coccinelle/advice.cocci\n\ndiff --git a/advice.c b/advice.c\nindex 63bf8b0c5f..c60b33ee33 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -216,13 +216,8 @@ int error_resolve_conflict(const char *me)\n \telse\n \t\tBUG(\"Unhandled conflict reason '%s'\", me);\n \n-\tif (advice_enabled(ADVICE_RESOLVE_CONFLICT))\n-\t\t/*\n-\t\t * Message used both when 'git commit' fails and when\n-\t\t * other commands doing a merge do.\n-\t\t */\n-\t\tadvise(_(\"Fix them up in the work tree, and then use 'git add/rm '\\n\"\n-\t\t\t \"as appropriate to mark resolution and make a commit.\"));\n+\tadvice_if_enabled(ADVICE_RESOLVE_CONFLICT,\n+\t\t\t _(\"Fix them up in the work tree, and then use 'git add/rm '\\n\" \"as appropriate to mark resolution and make a commit.\"));\n \treturn -1;\n }\n \n@@ -235,8 +230,8 @@ void NORETURN die_resolve_conflict(const char *me)\n void NORETURN die_conclude_merge(void)\n {\n \terror(_(\"You have not concluded your merge (MERGE_HEAD exists).\"));\n-\tif (advice_enabled(ADVICE_RESOLVE_CONFLICT))\n-\t\tadvise(_(\"Please, commit your changes before merging.\"));\n+\tadvice_if_enabled(ADVICE_RESOLVE_CONFLICT,\n+\t\t\t _(\"Please, commit your changes before merging.\"));\n \tdie(_(\"Exiting because of unfinished merge.\"));\n }\n \ndiff --git a/branch.c b/branch.c\nindex 22f4f46b96..a87facd311 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -812,9 +812,9 @@ void create_branches_recursively(struct repository *r, const char *name,\n \t\t\tint code = die_message(\n \t\t\t\t_(\"submodule '%s': unable to find submodule\"),\n \t\t\t\tsubmodule_entry_list.entries[i].submodule->name);\n-\t\t\tif (advice_enabled(ADVICE_SUBMODULES_NOT_UPDATED))\n-\t\t\t\tadvise(_(\"You may try updating the submodules using 'git checkout --no-recurse-submodules %s && git submodule update --init'\"),\n-\t\t\t\t start_committish);\n+\t\t\tadvice_if_enabled(ADVICE_SUBMODULES_NOT_UPDATED,\n+\t\t\t\t\t _(\"You may try updating the submodules using 'git checkout --no-recurse-submodules %s && git submodule update --init'\"),\n+\t\t\t\t\t start_committish);\n \t\t\texit(code);\n \t\t}\n \ndiff --git a/builtin/am.c b/builtin/am.c\nindex e9623b8307..6039b69475 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1910,8 +1910,8 @@ static void am_run(struct am_state *state, int resume)\n \t\t\tprintf_ln(_(\"Patch failed at %s %.*s\"), msgnum(state),\n \t\t\t\tlinelen(state->msg), state->msg);\n \n-\t\t\tif (advice_enabled(ADVICE_AM_WORK_DIR))\n-\t\t\t\tadvise(_(\"Use 'git am --show-current-patch=diff' to see the failed patch\"));\n+\t\t\tadvice_if_enabled(ADVICE_AM_WORK_DIR,\n+\t\t\t\t\t _(\"Use 'git am --show-current-patch=diff' to see the failed patch\"));\n \n \t\t\tdie_user_resolve(state);\n \t\t}\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 2bc21aa49b..34f05d2381 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1612,8 +1612,8 @@ static void die_expecting_a_branch(const struct branch_info *branch_info)\n \t\t */\n \t\tcode = die_message(_(\"a branch is expected, got '%s'\"), branch_info->name);\n \n-\tif (advice_enabled(ADVICE_SUGGEST_DETACHING_HEAD))\n-\t\tadvise(_(\"If you want to detach HEAD at the commit, try again with the --detach option.\"));\n+\tadvice_if_enabled(ADVICE_SUGGEST_DETACHING_HEAD,\n+\t\t\t _(\"If you want to detach HEAD at the commit, try again with the --detach option.\"));\n \n \texit(code);\n }\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex e7cd3225fa..5e4989a9aa 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1806,8 +1806,8 @@ static int add_possible_reference_from_superproject(\n \t\t} else {\n \t\t\tswitch (sas->error_mode) {\n \t\t\tcase SUBMODULE_ALTERNATE_ERROR_DIE:\n-\t\t\t\tif (advice_enabled(ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE))\n-\t\t\t\t\tadvise(_(alternate_error_advice));\n+\t\t\t\tadvice_if_enabled(ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,\n+\t\t\t\t\t\t _(alternate_error_advice));\n \t\t\t\tdie(_(\"submodule '%s' cannot add alternate: %s\"),\n \t\t\t\t sas->submodule_name, err.buf);\n \t\t\tcase SUBMODULE_ALTERNATE_ERROR_INFO:\ndiff --git a/sequencer.c b/sequencer.c\nindex 65afd100d9..6d8be0c036 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -624,8 +624,8 @@ static int error_dirty_index(struct repository *repo, struct replay_opts *opts)\n \terror(_(\"your local changes would be overwritten by %s.\"),\n \t\t_(action_name(opts)));\n \n-\tif (advice_enabled(ADVICE_COMMIT_BEFORE_MERGE))\n-\t\tadvise(_(\"commit your changes or stash them to proceed.\"));\n+\tadvice_if_enabled(ADVICE_COMMIT_BEFORE_MERGE,\n+\t\t\t _(\"commit your changes or stash them to proceed.\"));\n \treturn -1;\n }\n \n@@ -3497,9 +3497,8 @@ static int create_seq_dir(struct repository *r)\n \t}\n \tif (in_progress_error) {\n \t\terror(\"%s\", in_progress_error);\n-\t\tif (advice_enabled(ADVICE_SEQUENCER_IN_USE))\n-\t\t\tadvise(in_progress_advice,\n-\t\t\t\tadvise_skip ? \"--skip | \" : \"\");\n+\t\tadvice_if_enabled(ADVICE_SEQUENCER_IN_USE, in_progress_advice,\n+\t\t\t\t advise_skip ? \"--skip | \" : \"\");\n \t\treturn -1;\n \t}\n \tif (mkdir(git_path_seq_dir(), 0777) < 0)\ndiff --git a/tools/coccinelle/advice.cocci b/tools/coccinelle/advice.cocci\nnew file mode 100644\nindex 0000000000..da4851c5d0\n--- /dev/null\n+++ b/tools/coccinelle/advice.cocci\n@@ -0,0 +1,7 @@\n+@@\n+expression A;\n+expression list args;\n+@@\n+-if (advice_enabled(A))\n+-\tadvise(args);\n++advice_if_enabled(A, args);\n-- \n2.55.0-967-gab67bff200\n\n"},{"id":"552394","messageId":"20260910043356.GB241223@coredump.intra.peff.net","threadId":"66285","inReplyTo":"xmqq8q594tvs.fsf@gitster.g","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-09-10T04:33:56Z","receivedAt":"2026-09-10T04:33:58Z","isPatch":true,"body":"On Wed, Sep 09, 2026 at 09:23:19PM -0700, Junio C Hamano wrote:\n\n> Surely, and I think we are pretty much on the same page. Such a\n> mechanical rewrite is not too bad. Here is what I came up with:\n> \n> $ edit tools/coccinelle/advice.cocci\n> $ make coccicheck\n> $ git add -N tools/coccinelle/advice.cocci\n> $ git apply .build/tools/coccinelle/ALL.cocci.patch\n> $ git add -p\n> \n> Some of the hunks I simply accepted with (y), but most of them\n> needed (e)dit to make them presentable; otherwise we ended up\n> with too many overly long lines.\n> \n> \n> tools/coccinelle/advice.cocci | 7 +++++++\n> advice.c | 13 ++++---------\n> branch.c | 6 +++---\n> builtin/am.c | 4 ++--\n> builtin/checkout.c | 4 ++--\n> builtin/submodule--helper.c | 4 ++--\n> sequencer.c | 9 ++++-----\n> 7 files changed, 24 insertions(+), 23 deletions(-)\n\nThis misses a few that have more complex conditionals like:\n\ndiff --git a/commit.c b/commit.c\nindex ad26f0b40a..5eedad6a2c 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -290,9 +290,9 @@ static int read_graft_file(struct repository *r, const char *graft_file)\n \tstruct strbuf buf = STRBUF_INIT;\n \tif (!fp)\n \t\treturn -1;\n-\tif (!no_graft_file_deprecated_advice &&\n-\t advice_enabled(ADVICE_GRAFT_FILE_DEPRECATED))\n-\t\tadvise(_(\"Support for /info/grafts is deprecated\\n\"\n+\tif (!no_graft_file_deprecated_advice)\n+\t\tadvise_if_enabled(ADVICE_GRAFT_FILE_DEPRECATED,\n+\t\t\t_(\"Support for /info/grafts is deprecated\\n\"\n \t\t\t \"and will be removed in a future Git version.\\n\"\n \t\t\t \"\\n\"\n \t\t\t \"Please use \\\"git replace --convert-graft-file\\\"\\n\"\n\nBut I think the bigger question remains: if we did this, would people\nfind the extra lines giving the turn-off instructions ugly/overwhelming?\nI'm not sure.\n\n-Peff\n"},{"id":"552439","messageId":"xmqq1pb147w2.fsf@gitster.g","threadId":"66285","inReplyTo":"20260910043356.GB241223@coredump.intra.peff.net","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-10T12:18:21Z","receivedAt":"2026-09-10T12:18:24Z","isPatch":true,"body":"Jeff King writes:\n\n> But I think the bigger question remains: if we did this, would people\n> find the extra lines giving the turn-off instructions ugly/overwhelming?\n> I'm not sure.\n\nWell, if they find them unnecessary then they would want to turn it\noff and the instruction is already there ;-)\n\nMore seriously, if an advice item is found as such, then the item\neither must (1) be beneficial enough to be always shown, or (2) be\nso rarely shown that the turn-off instruction is unneeded. It would\ninherently be case-by-case basis but I do think we would converge\nbetween unconditional advise() calls or advise_if_enabled() calls.\n\n"},{"id":"552465","messageId":"20260910163120.GD251185@coredump.intra.peff.net","threadId":"66285","inReplyTo":"xmqq1pb147w2.fsf@gitster.g","subject":"Re: [PATCH] advice: use global config for default branch name","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-09-10T16:31:20Z","receivedAt":"2026-09-10T16:31:23Z","isPatch":true,"body":"On Thu, Sep 10, 2026 at 05:18:21AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > But I think the bigger question remains: if we did this, would people\n> > find the extra lines giving the turn-off instructions ugly/overwhelming?\n> > I'm not sure.\n> \n> Well, if they find them unnecessary then they would want to turn it\n> off and the instruction is already there ;-)\n\nWell, it would certainly increase my desire to turn each one off. ;) I\nguess you can set it to \"true\" to suppress the turn-off instructions\n(but keep the advice itself).\n\n> More seriously, if an advice item is found as such, then the item\n> either must (1) be beneficial enough to be always shown, or (2) be\n> so rarely shown that the turn-off instruction is unneeded. It would\n> inherently be case-by-case basis but I do think we would converge\n> between unconditional advise() calls or advise_if_enabled() calls.\n\nRight, I was wondering specifically if there are items in (1), but you\nsaid it much better than I did. I guess we wouldn't know until we try\nit and see people's reactions, though.\n\n-Peff\n"}]}