Re: [PATCH 3/3] advice: allow disabling the automatic hint in advise_if_enabled()

From: Rubén Justo <rjusto@gmail.com>
Date: Jan 20, 2024, 11:31 UTC
Message-ID: <6a27be56-9d52-46bf-8136-5f1c42379fd1@gmail.com>
In-Reply-To: <dd82bef87fd82f7c4bf7cbff5f873141@manjaro.org>

On 18-ene-2024 21:50:23, Dragan Simic wrote:

Show 22 quoted lines

> On 2024-01-18 21:19, Junio C Hamano wrote:
> > Dragan Simic <dsimic@manjaro.org> writes:
> > > On 2024-01-18 19:26, Junio C Hamano wrote:
> > > > Dragan Simic <dsimic@manjaro.org> writes:
> > > > 
> > > > > Perhaps having both options available could be a good thing.
> > > > > Though,
> > > > > adding quite a few knobs may end up confusing the users, so
> > > > > we should
> > > > > make sure to document it well.
> > > > 
> > > > I agree there is a need for documentation, but we are not increasing
> > > > the number of knobs, are we?
> > > 
> > > Perhaps there would be one more knob if we end up deciding to
> > > implement
> > > support for both approaches, as previously discussed.
> > 
> > But that would be just one knob (which I personally do not quite see
> > the need for), not "quite a few knobs" as you said, no?
> 
> I'm sorry for being imprecise.

Hi Dragan

My original motivation for starting this series has been fulfilled: Give the user an option to tell us not to include the disabling instructions alongside the advice.

The current consensus is: if the user explicitly enables visibility for an advice, we can stop including the instructions on how to disable its visibility.

As reference, in this thread two "global" options have been reviewed:

 - To take the disabling instructions as an advice in itself and as
   such, as with the rest, we can have a knob to disable it.  Affecting
   all advice messages.

 - A new knob to allow the user to set the default visibility for those
   advice messages without an explicit visibility set.  And so we can
   take on globally what we now take on locally;  if there is not an
   explicit visibility value but there is an explicit "default" value,
   we can stop showing the instruction on how to disable it.

> Regarding the need for that additional
> "global" knob, I think it can't hurt.  Some people might even find it
> quite useful

I don't quite understand what "global" knob you are referring to. But I share with you the feeling that a "global" knob might be useful.

The current iteration for this series looks like it will be merged to 'next' soon. In my opinion, it is a good stop for this series, and maybe we can explore that 'global' knob in a new one.

Thank you for your interest in making this series better.