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

[Ramkumar Ramachandra <artagnon@gmail.com>]

Hi Jonathan,

I'm a little confused, so I'm also including a commit message
justifying the change.
Have I understood the issue correctly? Does this diff look alright?

Note: I've removed the die from get_message in another unrelated
patch; essentially, do_pick_commit never calls die.

    Since do_pick_commit is only delegated the job of picking a single
    commit in an entire cherry-pick or revert operation, don't die in this
    function.  Instead, return an error to be handled by the caller
    appropriately.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>

diff --git a/builtin/revert.c b/builtin/revert.c
index 0cc3b6b..138485f 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -415,20 +415,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));
+			return error(_("%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);
+			return error(_("%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));
+		return error(_("%s: Mainline was specified but commit %s is not a merge."),
+			me, sha1_to_hex(commit->object.sha1));
 	else
 		parent = commit->parents->item;

@@ -438,8 +438,8 @@ static int do_pick_commit(void)
 	if (parent && parse_commit(parent) < 0)
 		/* TRANSLATORS: The first %s will be "revert" or
 		   "cherry-pick", the second %s a SHA1 */
-		die(_("%s: cannot parse parent commit %s"),
-		    me, sha1_to_hex(parent->object.sha1));
+		return error(_("%s: cannot parse parent commit %s"),
+			me, sha1_to_hex(parent->object.sha1));

 	/*
 	 * "commit" is an existing commit.  We would want to apply
@@ -578,8 +578,8 @@ static int revert_or_cherry_pick(int argc, const
char **argv)

 	while ((commit = get_revision(&revs))) {
 		int res = do_pick_commit();
-		if (res)
-			return res;
+		if (res < 0)
+			exit(128);
 	}

 	return 0;