Re: [PATCH 17/18] revert: Don't implictly stomp pending sequencer operation

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 27, 2011, 14:33 UTC
Message-ID: <20110727143346.GD24785@elie>
In-Reply-To: <CALkWK0mhAc4TtSzN5DW8OX91QBvosbbKsYfB5QPFy563qg7KRQ@mail.gmail.com>

Ramkumar Ramachandra wrote:

> Jonathan Nieder writes:
>> Ramkumar Ramachandra wrote:

Show 16 quoted lines

>>> +             if (create_seq_dir() < 0) {
>>> +                     advise(_("A cherry-pick or revert is in progress."));
>>> +                     advise(_("Use --reset to forget about it"));
>>> +                     return -1;
>>> +             }
>>
>> The usual formula is:
>>
>>        error(... description of error ...)
>>        if (advice_foo_bar [i.e., if the user is not tired of the advice already]) {
>>                advise(... how to recover from error ...);
>>                advise(... more lines ...);
>>        }
>
> I think you're trying to say two things here:
> 1. Put the error() call in the caller.

No, I didn't mean anything about the caller. Here the error is "a cherry-pick or revert is in progress". Before the advice mechanism existed, such a line would have been written with error(), not note(), right?

[...]

> 2. Guard the advice using a variable.  I have to invent a new
> configuration variable; can't that wait*?

Sure, if the advice is quiet (as it is in this case), I suppose there is no need to guard it until someone using it complains. :)

I mentioned advice configuration to emphasize that output from running git with all "hint:" lines stripped out is supposed to be just as sensible as with them, if I understand the purpose of the facility correctly.

Thanks for thinking it over.