Taylor Blau <me@xxxxxxxxxxxx> writes:
> On Tue, Jan 09, 2024 at 04:25:32PM +0100, Rubén Justo wrote:
>> Using advise_if_enabled() to display an advice will automatically
>> include instructions on how to disable the advice, along with the main
>> advice:
>>
>> hint: use --reapply-cherry-picks to include skipped commits
>> hint: Disable this message with "git config advice.skippedCherryPicks false"
>>
>> This may become distracting or "noisy" over time, while the user may
>> still want to receive the main advice.
>>
>> Let's have a switch to allow disabling this automatic advice.
>
> I reviewed this and had a couple of notes, mostly focused on what to
> call the new configuration option, and if we should be modifying the
> test-tool helpers to accept arbitrary configuration via command-line
> options.
>
> I think that we could reasonably drop the first two patches by
> imitating the existing style of t0018 more closely, but I don't feel all
> that strongly about it.
Even though I do not have a fundamental objection against test-tool
learning "-c key=val", I do not see a strong need for the first two
steps, either.
I actually have more problems with the primary change of this series
(in addition to that configuration knob is probably misnamed, as you
pointed out). How to disable the advice is an integral part of the
end-user experience about the conditional advice system. The idea
is to show it repeatedly, perhaps loud enough to be called "noisy",
so that the user learns to follow the suggestion given by the hint.
Until then, the user is expected to gradually learn to follow the
suggestion more and more, seeing the advice message less and less
often. If we rob the "how to disable THIS PARTICULAR message" and
gave only this line:
>> hint: use --reapply-cherry-picks to include skipped commits
it would become impossible to find how to disable it, once the user
get comfortable enough to pass --reapply-cherry-picks when it is
appropriate to do so. I do not think there is no quick way other
than grepping in the source to find that the user needs to disable
the skippedCherryPicks message (no, you can look at the output from
"git config --help" and find "skippedCherryPicks" if you know that
is the symbol to be found, but there is nothing to link the above
hint message to that particular help page entry).
I would understand if the proposed change were to change the
"advice.<key>" configuration variable from a Boolean to a tristate
(yes = default, no = disabled, always = give advice without
instruction), though. IOW, the message might look like so:
hint: use --reapply-cherry-picks to include skipped commits
hint: setting advice.skippedCherryPicks to 'false' disables this message
hint: and setting it to 'always' removes this additional instruction.
Then, those who find the hint always useful (because the particular
mistake the hint is given against is something the user commits very
rarely and the convenience of being reminded when it happens, which
is rare, outweigh the burden of learning what is suggested) may want
to set it to 'always' to accept that new choice.
But not the way the new feature is proposed.
Thanks.
