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",
                if (data.autocorrect)
                        commit = get_commit_from_string(
                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

Reply via email to