[0/2] advice: add diverging advice

Message

Felipe Contreras March 8, 2023, 2:48 a.m. UTC

Not all our users are experts in git who understand their configurations
perfectly, some might be stuck in a simple error:

  Not possible to fast-forward, aborting.

That's not very friendly.

Let's add a bit more advice in case the user doesn't know what's going
on.

Felipe Contreras (2):
  advice: add diverging advice for novices
  advice: make diverging advice configurable

 Documentation/config/advice.txt | 2 ++
 advice.c                        | 9 +++++++++
 advice.h                        | 1 +
 3 files changed, 12 insertions(+)

Comments

Taylor Blau March 8, 2023, 4:03 p.m. UTC | #1

Hi Felipe,

On Tue, Mar 07, 2023 at 08:48:32PM -0600, Felipe Contreras wrote:
> Not all our users are experts in git who understand their configurations
> perfectly, some might be stuck in a simple error:
>
>   Not possible to fast-forward, aborting.
>
> That's not very friendly.
>
> Let's add a bit more advice in case the user doesn't know what's going
> on.

Thanks for improving this case. I think the new advice() is reasonable
and will be helpful for users who might otherwise get stuck here.

I don't think that splitting it into two separate patches was strictly
necessary. If I were queuing, I'd probably squash the two together, but
I leave that one to Junio to figure out ;-).

Thanks,
Taylor

Junio C Hamano March 8, 2023, 5:17 p.m. UTC | #2

Taylor Blau <[e-mail]> writes:

> I don't think that splitting it into two separate patches was strictly
> necessary. If I were queuing, I'd probably squash the two together, ...

One advantage of sending a topic like this as two patches is that,
if the review discussion leads to a consensus that the new help
message should be given unconditionally to everybody, only [1/2] can
be queued while dropping [2/2].  But the point of advise() is to
serve as a training wheel that can be disabled by users who no
longer need it, so need for such a "flexibility" may not appear in
topics that add new advise() calls all that often, I would imagine.

I am willing to do the squashing on the receiving end.  The effect
of two patches combined together is a good improvement, and the
proposed log message for the first one covers why we want to do both
of these two.

Thanks, both, for writing and reviewing.

Junio C Hamano March 9, 2023, 11:44 p.m. UTC | #3

Taylor Blau <[e-mail]> writes:

> On Tue, Mar 07, 2023 at 08:48:32PM -0600, Felipe Contreras wrote:
>> Not all our users are experts in git who understand their configurations
>> perfectly, some might be stuck in a simple error:
>>
>>   Not possible to fast-forward, aborting.
>>
>> That's not very friendly.
>>
>> Let's add a bit more advice in case the user doesn't know what's going
>> on.
>
> Thanks for improving this case. I think the new advice() is reasonable
> and will be helpful for users who might otherwise get stuck here.

We may want to further tweak the advise() message depending on the
caller [*], but that can come on top after these two patches settle.

[Footnote]

* When a failed "git pull" gave the new advise() message, the user
  may not be advanced enough to adjust the advice to run "git merge
  --no-ff" to the given situation, for example.