Re: [PATCH] advice: use global config for default branch name

On Wed, Sep 09, 2026 at 11:51:03AM -0700, Junio C Hamano wrote:

quoted

will. So there are many missed opportunities for offering the turn-off
instructions. Nobody seems to have complained, which makes me wonder if
the turn-off instructions would be annoyingly chatty if we printed them
all the time. Most of those calls predate the addition if the turn-off
instructions and advise_if_enabled(), which was added in 2020. I wonder
how people would feel if we converted them all and started printing the
turn-off instructions everywhere.

Depends on how we do so, I guess.  Do you mean we should rewrite
advise() call above to advice_if_enabled(), even though the check
for ADVICE_FOO token appear redundant?

I mean we could mechanically rewrite:

  if (advice_enabled(ADVICE_FOO))
	advise(...);

to:

  advise_if_enabled(ADVICE_FOO, ...);

So the check wouldn't be redundant, but rather folded into the helper
function. The code becomes shorter, and the user-visible behavior
changes to produce the extra "turn-off" message.

quoted

Anyway, UI philosophizing aside, another obvious pattern for advise()
is:

  if (advice_is_enabled(ADVICE_FOO)) {
	/* do lots of work */
	advise("try %s", results_of_work);
  }

Yes, checking with is-enabled primarily for the purpose of skipping
"do lots of work" is a very typical use.  I do not know why you
assume ...

quoted

which _wouldn't_ want to convert to advise_if_enabled().

... this "try X" is something the users would not want to learn how
to disable, but assuming it is not, the existing code above as-is
should be what we want.

I meant only that they would not want the same mechanical conversion
above, because that would lose the ability to avoid the extra work.

quoted

I guess the caller could just do:

  advise_if_enabled(ADVICE_FOO, ...);

inside the block. We know that it's enabled, but it's not like the check
is expensive.

Yes, I think we already have some callers that do so, in a pattern
where they want to skip the "do lots of work" part.  Or at least I
think I suggested the pattern in the past for somebody who wanted to
do that.

I think we do the same thing with trace_want() in a few spots.

-Peff