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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 8, 2020, 19:01 UTC
Message-ID: <nycvar.QRO.7.76.6.2001081945490.46@tvgsbejvaqbjf.bet>
In-Reply-To: <CACg5j260h88bd=W_4EzAn7B0TiU02Y8BzKDQ7w3UJiHkhL60NQ@mail.gmail.com>

Hi,

On Thu, 9 Jan 2020, Heba Waly wrote:

Show 5 quoted lines

> On Wed, Jan 8, 2020 at 10:28 PM Eric Sunshine <sunshine@sunshineco.com> 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

FWIW I disagree that we need to reduce the number of config settings. Pretty much all of them have a good reason to exist.

I _could_ however see some sort of categorisation as a valuable goal, which would potentially make it easier to have chapters in the `git config` documentation where earlier chapters describe common settings and the later chapters describe subsequently more obscure settings.

> This raises a question though, do we really need a new configuration for
> every new advice?

I would keep it this way, if only for consistency (a department in which Git still has a lot of room for improvement).

Show 5 quoted lines

> 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?

This is the first time I hear about anybody wanting to disable any advice ;-)

If this is desired, it should be easy enough:

-- snip --

diff --git a/advice.c b/advice.c
index 3ee0ee2d8fb..28e48d5410b 100644
--- a/advice.c
+++ b/advice.c
@@ -138,6 +138,13 @@ int git_default_advice_config(const char *var, const char *value)
 	if (!skip_prefix(var, "advice.", &k))
 		return 0;

+	if (!strcmp(k, "suppressall")) {
+		if (git_config_bool(var, value))
+			for (i = 0; i < ARRAY_SIZE(advice_config); i++)
+				*advice_config[i].preference = 0;
+		return 0;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(advice_config); i++) {
 		if (strcasecmp(k, advice_config[i].name))
 			continue;
-- snap --

I don't really think that this is desired, though. Git has earned a
reputation for being hard to use, so I was personally delighted when we
started introducing the advise feature, and I have actually heard a couple
users say good things whenever Git learns to help them without having to
ask another human being (and feeling dumb as a consequence).

Ciao,
Dscho