It is quite convenient to simply die() in builtins, in the absence of
proper exception handling, because it allows us to just go belly up
without having to implement error handling chains.

Of course, for reusable library functions, this is a big no-no, so we
(try to) restrict the usage of die() to one-shot commands, i.e. places
where we know that the caller does not want to, say, give the user a
useful high-level error message, i.e. a message that the function calling
die() could not possibly know.

The problem with this reasoning is that sooner or later, pretty much all
useful functions will *need* to be libified: the more useful a function,
the more likely it is to be called from a call chain where the outer
function implements a high-level operation that needs to provide
additional advice to the user in case of failure.

This is the case here: the create_graft() function is useful enough to be
called in a loop, say, in the upcoming patch to convert a graft file in
one fell swoop. Therefore, this function must not be allowed to die(), nor
any of its callees.

Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
---
 builtin/replace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 935647be6bd..43264f0998e 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -395,7 +395,9 @@ static int create_graft(int argc, const char **argv, int 
force)
 
        if (get_oid(old_ref, &old_oid) < 0)
                die(_("Not a valid object name: '%s'"), old_ref);
-       commit = lookup_commit_or_die(&old_oid, old_ref);
+       commit = lookup_commit_reference(&old_oid);
+       if (!commit)
+               return error(_("could not parse %s"), old_ref);
 
        buffer = get_commit_buffer(commit, &size);
        strbuf_add(&buf, buffer, size);
-- 
2.17.0.windows.1.4.g7e4058d72e3


Reply via email to