Re: [PATCH v2 1/1] branch: advise the user to checkout a different branch before deleting

Heba Waly [off-list ref] writes:

On Wed, Jan 8, 2020 at 10:28 PM Eric Sunshine [off-list ref] wrote:

quoted

advice seems simple on the surface, but every new piece of advice
means having to add yet another configuration variable, writing more
code, more tests, and more documentation

This raises a question though, do we really need a new configuration
for every new advice?
So a user who's not interested in receiving advice will have to
disable every single advice config? It doesn't seem scalable to me.
I imagine a user will either want to enable or disable the advice
feature all together. Why don't we have only one `enable_advice`
configuration that controls all the advice messages?

The advice mechanism was a way to help new people learn the system
by giving a bit of extra help messages that would become annoying
once they learned that part of the system, so by default they are
on, and can be turned off once they learn enough about the specific
situation that gives one kind of advise.  Hence, "[advice] !all" to
decline any and all advice message, including anything that would be
introduced in the future, is somewhat a foreign concept in that
picture.

Having said that, I am not opposed to add support for such an
overall "turn all off" (or on for that matter).  Totally untested,
but something along this line, perhaps?  The idea is that

 - the config keys may come in any order;

 - once advice.all is set to either true or false, we set all the
   advice.* variables to the given value,

 - for any other advice.* config, we interpret it only if we haven't
   seen advice.all



 advice.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/advice.c b/advice.c
index 098ac0abea..b9a8fe1360 100644
--- a/advice.c
+++ b/advice.c

@@ -3,6 +3,8 @@
 #include "color.h"
 #include "help.h"
 
+static int advice_all_seen = -1; /* not seen yet */
+
 int advice_fetch_show_forced_updates = 1;
 int advice_push_update_rejected = 1;
 int advice_push_non_ff_current = 1;

@@ -142,13 +144,22 @@ int git_default_advice_config(const char *var, const char *value)
 	if (!skip_prefix(var, "advice.", &k))
 		return 0;
 
-	for (i = 0; i < ARRAY_SIZE(advice_config); i++) {
-		if (strcasecmp(k, advice_config[i].name))
-			continue;
-		*advice_config[i].preference = git_config_bool(var, value);
+	if (!strcmp(var, "advise.all")) {
+		advice_all_seen = git_config_bool(var, value);
+		for (i = 0; i < ARRAY_SIZE(advice_config); i++)
+			*advice_config[i].preference = advice_all_seen;
 		return 0;
 	}
 
+	if (advice_all_seen < 0) {
+		for (i = 0; i < ARRAY_SIZE(advice_config); i++) {
+			if (strcasecmp(k, advice_config[i].name))
+				continue;
+			*advice_config[i].preference = git_config_bool(var, value);
+			return 0;
+		}
+	}
+
 	return 0;
 }