Tim Harper <timcharper@xxxxxxxxx> writes:
>> 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.
--
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