Heba Waly <heba.waly@xxxxxxxxx> writes:
> On Wed, Jan 8, 2020 at 10:28 PM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>>
>> 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;
}
