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

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

Jeff King <peff@peff.net> writes:

> On Wed, Sep 09, 2026 at 11:51:03AM -0700, Junio C Hamano wrote:
>
>> > will. So there are many missed opportunities for offering the turn-off
>> > instructions. Nobody seems to have complained, which makes me wonder if
>> > the turn-off instructions would be annoyingly chatty if we printed them
>> > all the time. Most of those calls predate the addition if the turn-off
>> > instructions and advise_if_enabled(), which was added in 2020. I wonder
>> > how people would feel if we converted them all and started printing the
>> > turn-off instructions everywhere.
>> 
>> Depends on how we do so, I guess.  Do you mean we should rewrite
>> advise() call above to advice_if_enabled(), even though the check
>> for ADVICE_FOO token appear redundant?
>
> I mean we could mechanically rewrite:
>
>   if (advice_enabled(ADVICE_FOO))
> 	advise(...);
>
> to:
>
>   advise_if_enabled(ADVICE_FOO, ...);

Surely, and I think we are pretty much on the same page.  Such a
mechanical rewrite is not too bad.  Here is what I came up with:

    $ edit tools/coccinelle/advice.cocci
    $ make coccicheck
    $ git add -N tools/coccinelle/advice.cocci
    $ git apply .build/tools/coccinelle/ALL.cocci.patch
    $ git add -p

    Some of the hunks I simply accepted with (y), but most of them
    needed (e)dit to make them presentable; otherwise we ended up
    with too many overly long lines and losing some comments.

--- >8 ---
Subject: [PATCH] advice: use advise_if_enabled() more

One very common pattern is

	if (advice_enabled(ADVICE_FOO))
		advise(_("MESSAGE FOR FOO"));

but we have a perfect short-hand for that.  Using coccinelle,
rewrite the above as

	advise_if_enabled(ADVICE_FOO, _("MESSAGE FOR FOR"));

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 advice.c                      | 13 ++++---------
 branch.c                      |  6 +++---
 builtin/am.c                  |  4 ++--
 builtin/checkout.c            |  4 ++--
 builtin/submodule--helper.c   |  4 ++--
 sequencer.c                   |  9 ++++-----
 tools/coccinelle/advice.cocci |  7 +++++++
 7 files changed, 24 insertions(+), 23 deletions(-)
 create mode 100644 tools/coccinelle/advice.cocci

diff --git a/advice.c b/advice.c
index 63bf8b0c5f..c60b33ee33 100644
--- a/advice.c
+++ b/advice.c
@@ -216,13 +216,8 @@ int error_resolve_conflict(const char *me)
 	else
 		BUG("Unhandled conflict reason '%s'", me);
 
-	if (advice_enabled(ADVICE_RESOLVE_CONFLICT))
-		/*
-		 * Message used both when 'git commit' fails and when
-		 * other commands doing a merge do.
-		 */
-		advise(_("Fix them up in the work tree, and then use 'git add/rm <file>'\n"
-			 "as appropriate to mark resolution and make a commit."));
+	advice_if_enabled(ADVICE_RESOLVE_CONFLICT,
+			  _("Fix them up in the work tree, and then use 'git add/rm <file>'\n" "as appropriate to mark resolution and make a commit."));
 	return -1;
 }
 
@@ -235,8 +230,8 @@ void NORETURN die_resolve_conflict(const char *me)
 void NORETURN die_conclude_merge(void)
 {
 	error(_("You have not concluded your merge (MERGE_HEAD exists)."));
-	if (advice_enabled(ADVICE_RESOLVE_CONFLICT))
-		advise(_("Please, commit your changes before merging."));
+	advice_if_enabled(ADVICE_RESOLVE_CONFLICT,
+			  _("Please, commit your changes before merging."));
 	die(_("Exiting because of unfinished merge."));
 }
 
diff --git a/branch.c b/branch.c
index 22f4f46b96..a87facd311 100644
--- a/branch.c
+++ b/branch.c
@@ -812,9 +812,9 @@ void create_branches_recursively(struct repository *r, const char *name,
 			int code = die_message(
 				_("submodule '%s': unable to find submodule"),
 				submodule_entry_list.entries[i].submodule->name);
-			if (advice_enabled(ADVICE_SUBMODULES_NOT_UPDATED))
-				advise(_("You may try updating the submodules using 'git checkout --no-recurse-submodules %s && git submodule update --init'"),
-				       start_committish);
+			advice_if_enabled(ADVICE_SUBMODULES_NOT_UPDATED,
+					  _("You may try updating the submodules using 'git checkout --no-recurse-submodules %s && git submodule update --init'"),
+					  start_committish);
 			exit(code);
 		}
 
diff --git a/builtin/am.c b/builtin/am.c
index e9623b8307..6039b69475 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1910,8 +1910,8 @@ static void am_run(struct am_state *state, int resume)
 			printf_ln(_("Patch failed at %s %.*s"), msgnum(state),
 				linelen(state->msg), state->msg);
 
-			if (advice_enabled(ADVICE_AM_WORK_DIR))
-				advise(_("Use 'git am --show-current-patch=diff' to see the failed patch"));
+			advice_if_enabled(ADVICE_AM_WORK_DIR,
+					  _("Use 'git am --show-current-patch=diff' to see the failed patch"));
 
 			die_user_resolve(state);
 		}
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2bc21aa49b..34f05d2381 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1612,8 +1612,8 @@ static void die_expecting_a_branch(const struct branch_info *branch_info)
 		 */
 		code = die_message(_("a branch is expected, got '%s'"), branch_info->name);
 
-	if (advice_enabled(ADVICE_SUGGEST_DETACHING_HEAD))
-		advise(_("If you want to detach HEAD at the commit, try again with the --detach option."));
+	advice_if_enabled(ADVICE_SUGGEST_DETACHING_HEAD,
+			  _("If you want to detach HEAD at the commit, try again with the --detach option."));
 
 	exit(code);
 }
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e7cd3225fa..5e4989a9aa 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1806,8 +1806,8 @@ static int add_possible_reference_from_superproject(
 		} else {
 			switch (sas->error_mode) {
 			case SUBMODULE_ALTERNATE_ERROR_DIE:
-				if (advice_enabled(ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE))
-					advise(_(alternate_error_advice));
+				advice_if_enabled(ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,
+						  _(alternate_error_advice));
 				die(_("submodule '%s' cannot add alternate: %s"),
 				    sas->submodule_name, err.buf);
 			case SUBMODULE_ALTERNATE_ERROR_INFO:
diff --git a/sequencer.c b/sequencer.c
index 65afd100d9..6d8be0c036 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -624,8 +624,8 @@ static int error_dirty_index(struct repository *repo, struct replay_opts *opts)
 	error(_("your local changes would be overwritten by %s."),
 		_(action_name(opts)));
 
-	if (advice_enabled(ADVICE_COMMIT_BEFORE_MERGE))
-		advise(_("commit your changes or stash them to proceed."));
+	advice_if_enabled(ADVICE_COMMIT_BEFORE_MERGE,
+			  _("commit your changes or stash them to proceed."));
 	return -1;
 }
 
@@ -3497,9 +3497,8 @@ static int create_seq_dir(struct repository *r)
 	}
 	if (in_progress_error) {
 		error("%s", in_progress_error);
-		if (advice_enabled(ADVICE_SEQUENCER_IN_USE))
-			advise(in_progress_advice,
-				advise_skip ? "--skip | " : "");
+		advice_if_enabled(ADVICE_SEQUENCER_IN_USE, in_progress_advice,
+				  advise_skip ? "--skip | " : "");
 		return -1;
 	}
 	if (mkdir(git_path_seq_dir(), 0777) < 0)
diff --git a/tools/coccinelle/advice.cocci b/tools/coccinelle/advice.cocci
new file mode 100644
index 0000000000..da4851c5d0
--- /dev/null
+++ b/tools/coccinelle/advice.cocci
@@ -0,0 +1,7 @@
+@@
+expression A;
+expression list args;
+@@
+-if (advice_enabled(A))
+-	advise(args);
++advice_if_enabled(A, args);
-- 
2.55.0-967-gab67bff200