Re: [PATCH] clarify error message when an abbreviated non-existent commit was specified

Tim Harper [off-list ref] writes:

quoted

diff --git a/parse-options.c b/parse-options.c
index 3b71fbb..95eb1c4 100644
--- a/parse-options.c
+++ b/parse-options.c

@@ -615,7 +615,7 @@ int parse_opt_with_commit(const struct option *opt, const char *arg, int unset)

       if (!arg)
               return -1;
       if (get_sha1(arg, sha1))
-               return error("malformed object name %s", arg);
+               return error("malformed object name or no such commit: %s", arg);
       commit = lookup_commit_reference(sha1);
       if (!commit)
               return error("no such commit %s", arg);
--
1.6.4

Does nobody think this is a good idea?

Probably people don't care enough.  I certainly didn't pay much attention
to the discussion on a rather trivial patch that was not yet signed off.

I'd probably write along this line instead, if I cared enough.  

	if (get_sha1(arg, sha1) ||
            !(commit = lookup_commit_reference(sha1)))
		return error("no such commit: %s", arg);

I think the important part of the message is that whatever the user gave
us when we expected to see a string that names a commit was not a commit;
it is immaterial if the failure was because an abbreviated hexadecimal
form was mistyped (get_sha1() would fail in this case) or because a tag
that points at a non commit, e.g. "v2.6.11-tree", was given (l-c-r will
fail in that case).

Giving two different messages depending on the nature of an error will
help debugging parse_opt_with_commit(), but that benefit is secondary.