Re: [PATCH 1/8] revert: Improve error handling by cascading errors upwards

[Jonathan Nieder <jrnieder@gmail.com>]

Ramkumar Ramachandra wrote:

> I'm preparing a large series dedicated to solving error-handling
> issues before getting to the sequencer series.  I plan to post some
> quick-and-dirty diffs of various things and ask for feedback.

Sounds good.  I wonder if there's a good way to test this (maybe I'll
try making an XS module so Git.pm can take advantage of similar
changes).

> Jonathan Nieder writes:
>> Ramkumar Ramachandra wrote:

>>> @@ -418,19 +434,19 @@ static int do_pick_commit(void)
>>>  		struct commit_list *p;
>>>  
>>>  		if (!mainline)
>>> -			die(_("Commit %s is a merge but no -m option was given."),
>>> -			    sha1_to_hex(commit->object.sha1));
>>> +			return error(_("Commit %s is a merge but no -m option was given."),
>>> +				sha1_to_hex(commit->object.sha1));
>>
>> This is not a conflict but an error in usage.
[...]
> For this part, I think the correct way to handle the usage error is to
> print a message like this:
>
>     usage: cherry-pick: Commit b8bf32 is a merge but no -m option was given.
>
> And exit with status 129. Is this acceptable?

On second thought, status 128 seems appropriate --- the caller made a
mistake, but it's more analagous to merging with a dirty index than
(i.e., the command was not able to be fulfilled as desired) than to
misspelling a command-line option (i.e., broken script).  I suppose
treating it as an error (as in your "return error") would work, and
the caller can transform -1 to exit(128).

Sorry for the thinko.