Hi,
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.
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. Stepping back for a
> second, what is the best way to handle that? The aforementioned
> hypothetical caller considering "try an alternate strategy" would make
> a wrong move, so we need to use a return value that would dissuade it.
>
> Iâm not sure what the most appropriate thing to do is. Maybe
>
> /*
> * Positive numbers are exit status from conflicts; negative
> * numbers are other errors.
> */
> enum pick_commit_error {
> PICK_COMMIT_USAGE_ERROR = -1
> };
>
> but itâs hard to think clearly about it since it seems too
> hypothetical to me. If all callers are going to exit, then
>
> error(...);
> return 129;
>
> will work, but in that case why not exit for them?
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?
Signed-off-by: Ramkumar Ramachandra <artagnon@xxxxxxxxx>
@@ -418,20 +424,20 @@ 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));
+ usage(_("%s: Commit %s is a merge but no -m option was given."),
+ me, sha1_to_hex(commit->object.sha1));
for (cnt = 1, p = commit->parents;
cnt != mainline && p;
cnt++)
p = p->next;
if (cnt != mainline || !p)
- die(_("Commit %s does not have parent %d"),
- sha1_to_hex(commit->object.sha1), mainline);
+ usage(_("%s: Commit %s does not have parent %d"),
+ me, sha1_to_hex(commit->object.sha1), mainline);
parent = p->item;
- } else if (0 < mainline)
- die(_("Mainline was specified but commit %s is not a merge."),
- sha1_to_hex(commit->object.sha1));
+ } else if (mainline > 0)
+ usage(_("%s: Mainline was specified but commit %s is not a merge."),
+ me, sha1_to_hex(commit->object.sha1));
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html