From: Jonathan Nieder Date: Wed, 27 Jul 2011 14:33:46 GMT Subject: Re: [PATCH 17/18] revert: Don't implictly stomp pending sequencer operation Message-ID: <20110727143346.GD24785@elie> In-Reply-To: Ramkumar Ramachandra wrote: > Jonathan Nieder writes: >> Ramkumar Ramachandra wrote: >>> +             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.