From: Dragan Simic <dsimic@manjaro.org>
Date: Mon, 29 Apr 2024 17:54:55 GMT
Subject: Re: [PATCH v2 1/1] advice: add --no-advice global option
Message-ID: <d2d3e3b6b428e7384b701efba47c5dc1@manjaro.org>
In-Reply-To: <57b6c818-343a-450a-8183-2802abf1c62f@gmail.com>



On 2024-04-29 19:05, Rubén Justo wrote:
> On Mon, Apr 29, 2024 at 11:09:25AM +1000, James Liu wrote:
> 
>>  int advice_enabled(enum advice_type type)
>>  {
>> -	int enabled = advice_setting[type].level != ADVICE_LEVEL_DISABLED;
>> +	int enabled;
>> +
>> +	if (getenv(GIT_NO_ADVICE))
>> +		return 0;
>> +
>> +	enabled = advice_setting[type].level != ADVICE_LEVEL_DISABLED;
>> 
> 
> All hints are set to a default visibility value: "none", which means
> implicitly enabled.  Maybe we can get this "no-advice" knob by making
> this default value configurable:

Please note that the new environment variable isn't supposed
to be used externally, [1] i.e. it isn't meant to be set by
the users in their ~/.bash_profile files (or any other shell
configuration files), so I think that extending the scope of
this patch into such a direction isn't something we should
aim at.

[1] https://lore.kernel.org/git/xmqqh6fk1dmq.fsf@gitster.g/

> diff --git a/advice.c b/advice.c
> index 75111191ad..bc67b99ba7 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -39,6 +39,8 @@ enum advice_level {
>         ADVICE_LEVEL_ENABLED,
>  };
> 
> +static enum advice_level advice_default_level;
> +
>  static struct {
>         const char *key;
>         enum advice_level level;
> @@ -126,7 +128,19 @@ void advise(const char *advice, ...)
> 
>  int advice_enabled(enum advice_type type)
>  {
> -       int enabled = advice_setting[type].level != 
> ADVICE_LEVEL_DISABLED;
> +       static int once = 0;
> +       int enabled;
> +
> +       if (!once++) {
> +               const char* level = getenv("GIT_ADVICE_LEVEL");
> +
> +               if (level && !strcmp(level, "disable"))
> +                       advice_default_level = ADVICE_LEVEL_DISABLED;
> +       }
> +
> +       enabled = (advice_setting[type].level
> +                  ? advice_setting[type].level
> +                  : advice_default_level) != ADVICE_LEVEL_DISABLED;
> 
>         if (type == ADVICE_PUSH_UPDATE_REJECTED)
>                 return enabled &&