On Tue, Jan 09, 2024 at 04:30:16PM +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 can 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.
If I'm reading your patch correctly, this is a single option that
controls the extra line for _all_ advice messages. But I'd have expected
this to be something you'd want to set on a per-message basis. Here's my
thinking.
The original idea for advice messages was that they might be verbose and
annoying, but if you had one that showed up a lot you'd choose to shut
it up individually. But you wouldn't do so for _all_ messages, because
you might benefit from seeing others (including new ones that get
added). The "Disable this..." part was added later to help you easily
know which config option to tweak.
The expectation was that you'd fall into one of two categories:
1. You don't see the message often enough to care, so you do nothing.
2. You do find it annoying, so you disable this instance.
Your series proposes a third state:
3. You find the actual hint useful, but the verbosity of "how to shut
it up" is too much for you.
That make sense to me, along with being able to partially shut-up a
message. But wouldn't you still need the "how to shut up" hint for
_other_ messages, since it's customized for each situation?
E.g., suppose that after getting annoyed at advice.skippedCherryPicks,
you run "git config advice.adviseOff false".
But now you run into another hint, like:
$ git foo
hint: you can use --empty-commits to deal with isn't as good as --xyzzy
and you want to disable it entirely. Which advice.* config option does
so? You're stuck trying to find it in the manpage (which is tedious but
also kind of tricky since you're now guessing which name goes with which
message). You probably do want:
hint: Disable this message with "git config advice.xyzzy false"
in that case (at which point you may decide to silence it completely or
partially).
Which implies to me that the value of advice.* should be a tri-state to
match the cases above: true, false, or a "minimal" / "quiet" mode (there
might be a better name). And then you'd do:
git config advice.skippedCherryPicks minimal
(or whatever it is called) to get the mode you want, without affecting
advice.xyzzy.
> advice.c | 3 ++-
> advice.h | 3 ++-
> t/t0018-advice.sh | 8 ++++++++
> 3 files changed, 12 insertions(+), 2 deletions(-)
Speaking of manpages, we'd presumably need an update to
Documentation/config/advice.txt. :)
-Peff
