[1/3] advice: enhance `detach_advice()` to `detach_advice_if_enabled()`

Commit Message

Rubén Justo Dec. 8, 2024, 8:12 a.m. UTC

We have the `detachedHead` advice since 13be3e31f1 ("Reword 'detached
HEAD' notification", 2010-01-29).

This advice is shown to the user in the `detach_advice()` function, and
its only two clients verify beforehand if the advice is desired, in
order to call the function accordingly.

The `advise_if_enabled()` API encapsulates some functionality that we
can take advantage of:

 - Checks if the advice is desired, using `advice_enabled()`.

 - Automatically adds help, when needed, on how to disable the
   advice: "Turn off this advice by ...".

 - Displays the message consistently with other advise messages,
   prefixing each line with 'hint:'.

Let's simplify the logic for the clients of `detach_advice()` by
eliminating their need to decide whether to show the advice, bringing
that decision into `detach_advice()`.  Also, let's make the it use
`advice_if_enabled()` to ensure consistency with other advice messages.

To better reflect the changes in the function let's rename it to
`detach_advice_if_enabled()`.

Finally, note that we have two tests in t7201 related to this advice:
"checkout to detach HEAD (with advice declined)" and "checkout a detach
HEAD".  They are unaffected by the change we're doing here, so it is
not necessary to adjust them in this step.

Signed-off-by: Rubén Justo <[e-mail]>
---
 advice.c           | 8 +++-----
 advice.h           | 2 +-
 builtin/checkout.c | 5 ++---
 builtin/clone.c    | 3 +--
 4 files changed, 7 insertions(+), 11 deletions(-)

Patch

diff --git a/advice.c b/advice.c
index 6b879d805c..399ae58437 100644
--- a/advice.c
+++ b/advice.c
@@ -272,7 +272,7 @@  void advise_on_updating_sparse_paths(struct string_list *pathspec_list)
 			    "* Disable or modify the sparsity rules."));
 }
 
-void detach_advice(const char *new_name)
+void detach_advice_if_enabled(const char *new_name)
 {
 	const char *fmt =
 	_("Note: switching to '%s'.\n"
@@ -288,11 +288,9 @@  void detach_advice(const char *new_name)
 	"\n"
 	"Or undo this operation with:\n"
 	"\n"
-	"  git switch -\n"
-	"\n"
-	"Turn off this advice by setting config variable advice.detachedHead to false\n\n");
+	"  git switch -\n");
 
-	fprintf(stderr, fmt, new_name);
+	advise_if_enabled(ADVICE_DETACHED_HEAD, fmt, new_name);
 }
 
 void advise_on_moving_dirty_path(struct string_list *pathspec_list)
diff --git a/advice.h b/advice.h
index d7466bc0ef..739c5e4987 100644
--- a/advice.h
+++ b/advice.h
@@ -79,7 +79,7 @@  void NORETURN die_resolve_conflict(const char *me);
 void NORETURN die_conclude_merge(void);
 void NORETURN die_ff_impossible(void);
 void advise_on_updating_sparse_paths(struct string_list *pathspec_list);
-void detach_advice(const char *new_name);
+void detach_advice_if_enabled(const char *new_name);
 void advise_on_moving_dirty_path(struct string_list *pathspec_list);
 
 #endif /* ADVICE_H */
diff --git a/builtin/checkout.c b/builtin/checkout.c
index c449558e66..e1366556d7 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1009,9 +1009,8 @@  static void update_refs_for_switch(const struct checkout_opts *opts,
 				NULL,
 				REF_NO_DEREF, UPDATE_REFS_DIE_ON_ERR);
 		if (!opts->quiet) {
-			if (old_branch_info->path &&
-			    advice_enabled(ADVICE_DETACHED_HEAD) && !opts->force_detach)
-				detach_advice(new_branch_info->name);
+			if (old_branch_info->path && !opts->force_detach)
+				detach_advice_if_enabled(new_branch_info->name);
 			describe_detached_head(_("HEAD is now at"), new_branch_info->commit);
 		}
 	} else if (new_branch_info->path) {	/* Switch branches. */
diff --git a/builtin/clone.c b/builtin/clone.c
index 21721db28a..d0c5e89a2a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -752,8 +752,7 @@  static int checkout(int submodule_progress, int filter_submodules,
 		return 0;
 	}
 	if (!strcmp(head, "HEAD")) {
-		if (advice_enabled(ADVICE_DETACHED_HEAD))
-			detach_advice(oid_to_hex(&oid));
+		detach_advice_if_enabled(oid_to_hex(&oid));
 		FREE_AND_NULL(head);
 	} else {
 		if (!starts_with(head, "refs/heads/"))