advice: refactor advise API

Add a new advise_ng function that can check the visibility of advice
messages before printing.

Currently it's very easy for the callers to miss checking the
visibility step. Also, it makes more sense for this step to be handled
by the advice library.

Also change the advise call in tag library from advise() to advise_ng()
to construct an example of the usage of the new API.

Signed-off-by: Heba Waly <[email protected]>

Author: HebaWaly

Makefile

@@ -695,6 +695,7 @@ X =
 
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
+TEST_BUILTINS_OBJS += test-advise.o
 TEST_BUILTINS_OBJS += test-chmtime.o
 TEST_BUILTINS_OBJS += test-config.o
 TEST_BUILTINS_OBJS += test-ctype.o

advice.c

@@ -29,7 +29,6 @@ int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
 int advice_graft_file_deprecated = 1;
 int advice_checkout_ambiguous_remote_branch_name = 1;
-int advice_nested_tag = 1;
 int advice_submodule_alternate_error_strategy_die = 1;
 
 static int advice_use_color = -1;
@@ -89,7 +88,6 @@ static struct {
 	{ "waitingForEditor", &advice_waiting_for_editor },
 	{ "graftFileDeprecated", &advice_graft_file_deprecated },
 	{ "checkoutAmbiguousRemoteBranchName", &advice_checkout_ambiguous_remote_branch_name },
-	{ "nestedTag", &advice_nested_tag },
 	{ "submoduleAlternateErrorStrategyDie", &advice_submodule_alternate_error_strategy_die },
 
 	/* make this an alias for backward compatibility */
@@ -118,6 +116,41 @@ void advise(const char *advice, ...)
 	strbuf_release(&buf);
 }
 
+static const char turn_off_instructions[] =
+N_("\n"
+"Turn this message off by running\n"
+"\"git config %s false\"");
+
+void advise_ng(const char *key, const char *advice, ...)
+{
+	int value = 1;
+	struct strbuf buf = STRBUF_INIT;
+	va_list params;
+	const char *cp, *np;
+	
+	git_config_get_bool(key, &value);
+	
+	if(value)
+	{
+		va_start(params, advice);
+		strbuf_vaddf(&buf, advice, params);
+		va_end(params);
+
+		strbuf_addf(&buf, turn_off_instructions, key);
+		
+		for (cp = buf.buf; *cp; cp = np) {
+			np = strchrnul(cp, '\n');
+			fprintf(stderr,	_("%shint: %.*s%s\n"),
+				advise_get_color(ADVICE_COLOR_HINT),
+				(int)(np - cp), cp,
+				advise_get_color(ADVICE_COLOR_RESET));
+			if (*np)
+				np++;
+		}
+		strbuf_release(&buf);
+	}
+}
+
 int git_default_advice_config(const char *var, const char *value)
 {
 	const char *k, *slot_name;

advice.h

@@ -29,12 +29,16 @@ extern int advice_ignored_hook;
 extern int advice_waiting_for_editor;
 extern int advice_graft_file_deprecated;
 extern int advice_checkout_ambiguous_remote_branch_name;
-extern int advice_nested_tag;
 extern int advice_submodule_alternate_error_strategy_die;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))
 void advise(const char *advice, ...);
+
+/**
+ Checks the visibility of the advice before priniting.
+ */
+void advise_ng(const char *key, const char *advice, ...);
 int error_resolve_conflict(const char *me);
 void NORETURN die_resolve_conflict(const char *me);
 void NORETURN die_conclude_merge(void);

builtin/tag.c

@@ -231,8 +231,8 @@ static void create_tag(const struct object_id *object, const char *object_ref,
 	if (type <= OBJ_NONE)
 		die(_("bad object type."));
 
-	if (type == OBJ_TAG && advice_nested_tag)
-		advise(_(message_advice_nested_tag), tag, object_ref);
+	if (type == OBJ_TAG)
+		advise_ng("advice.nestedTag", _(message_advice_nested_tag), tag, object_ref);
 
 	strbuf_addf(&header,
 		    "object %s\n"

t/helper/test-advise.c

@@ -0,0 +1,15 @@
+#include "test-tool.h"
+#include "cache.h"
+#include "advice.h"
+
+int cmd__advise_ng(int argc, const char **argv)
+{
+	if (!argv[1] || !argv[2])
+	die("usage: %s <key> <advice>", argv[0]);
+
+	setup_git_directory();
+
+	advise_ng(argv[1], argv[2]);
+
+	return 0;
+}

t/helper/test-tool.c

@@ -14,6 +14,7 @@ struct test_cmd {
 };
 
 static struct test_cmd cmds[] = {
+	{ "advise", cmd__advise_ng },
 	{ "chmtime", cmd__chmtime },
 	{ "config", cmd__config },
 	{ "ctype", cmd__ctype },

t/helper/test-tool.h

@@ -4,6 +4,7 @@
 #define USE_THE_INDEX_COMPATIBILITY_MACROS
 #include "git-compat-util.h"
 
+int cmd__advise_ng(int argc, const char **argv);
 int cmd__chmtime(int argc, const char **argv);
 int cmd__config(int argc, const char **argv);
 int cmd__ctype(int argc, const char **argv);

t/t0018-advice.sh

@@ -0,0 +1,29 @@
+#!/bin/sh
+
+test_description='Test advise_ng functionality'
+
+. ./test-lib.sh
+
+cat > expected <<EOF
+hint: This is a piece of advice
+hint: Turn this message off by running
+hint: "git config advice.configVariable false"
+EOF
+test_expect_success 'advise should be printed when config variable is unset' '
+	test-tool advise "advice.configVariable" "This is a piece of advice" 2>actual &&
+	test_i18ncmp expected actual
+'
+
+test_expect_success 'advise should be printed when config variable is set to true' '
+	test_config advice.configVariable true &&
+	test-tool advise "advice.configVariable" "This is a piece of advice" 2>actual &&
+	test_i18ncmp expected actual
+'
+
+test_expect_success 'advise should not be printed when config variable is set to false' '
+	test_config advice.configVariable false &&
+	test-tool advise "advice.configVariable" "This is a piece of advice" 2>actual &&
+	test_must_be_empty actual
+'
+
+test_done

t/t7004-tag.sh

@@ -1726,6 +1726,8 @@ test_expect_success 'recursive tagging should give advice' '
 	hint: already a tag. If you meant to tag the object that it points to, use:
 	hint: |
 	hint: 	git tag -f nested annotated-v4.0^{}
+	hint: Turn this message off by running
+	hint: "git config advice.nestedTag false"
 	EOF
 	git tag -m nested nested annotated-v4.0 2>actual &&
 	test_i18ncmp expect actual