On Thursday 09 May 2013 04:19 AM, Junio C Hamano wrote:
> [...] which in turn made me realize that some commands may not even know
> if the user mistyped a ref. It is not an objection to this patch
> per-se, but a useful future enhancement may be to allow the callers
> call guess_mistyped_ref() directly and let them decide what to do
> when they suspect the string they did not understand is not a
> mistyped ref but something else, i.e. not let help_unknown_ref() die
> unconditionally but allow it to return. Then the caller can do:
>
> commit = get_commit_from_string(argv[i]);
> if (!commit) {
> ... I do not understand argv[i], but ...
> ... it may be a mistyped ref ...
> help_unknown_ref(argv[i], "expected a revision");
> ... it is not likely to be a typo ...
> ... perhaps it was meant to be a filename? ...
> if (file_exists(argv[i])) {
> ... yes! ...
> ... do the "file" thing instead ...
> }
> }
>
I'm apprehensive about calling guess_mistyped_ref() (or it's equivalent,
which happens to be guess_refs()) directly, because it doesn't seem like
a clean enough separation. When the caller thinks it's got a bad
refname, it should just hand it over to help_unknown_ref, for further
processing.
If autocorrect is enabled, it can get back a single corrected refname
(that is what my next patch will include - is it okay to base it on pu?).
If the need ever did arise to get that kind of information from
help_unknown_ref, it could always be done using callback data?
commit = get_commit_from_string(argv[i]);
if (!commit) {
... maybe mistyped ref, maybe something else ...
struct unknown_ref_cb data;
help_unknown_ref(argv[i], "expected something else",
&data);
if (data.autocorrect)
commit = get_commit_from_string(
data.corrected_ref);
else if (data.is_file)
... do the file thing instead ...
}
I didn't see the need for this right away.
>> Example:
>> $ git merge foo
>> merge: foo - not something we can merge
>
> That leading "merge: " looks somewhat strange, especially when it
> immediately follows the command line to invoke "merge", making it
> appear to waste space by stating the obvious.
>
> Our messages are generally marked with "error:", "fatal:",
> "warning:", etc. at the beginning.
I agree, it looks strange. However the alternatives seem to be:
1) hard code 'fatal' into the error message
2) print the corrections before using die()
3) create and store the corrections string beforehand, and then call die()
1 and 3 are not elegant, and 2's output seems harder to read. I haven't
been able to figure out a way to do this well.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
