Vikrant Varma <vikrant.varma94@xxxxxxxxx> writes:
> When a ref is not known, currently functions call die() with an
> error message.
The first part read somewhat awkward, so I started rewriting the
above like so:
When the user gives an unknown string to a command that expects
to see a ref, we could be more helpful than just saying "that's
not a ref" and die.
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 ...
}
}
> Add helper function help_unknown_ref to take care of displaying an
> error message along with a list of suggested refs the user might
> have meant.
OK.
> 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.
>
> Did you mean one of these?
> origin/foo
> upstream/foo
>
> Signed-off-by: Vikrant Varma <vikrant.varma94@xxxxxxxxx>
> ...
> +struct string_list guess_refs(const char *ref)
> +{
> + struct similar_ref_cb ref_cb;
> + struct string_list similar_refs = STRING_LIST_INIT_NODUP;
> +
> + ref_cb.base_ref = ref;
> + ref_cb.similar_refs = &similar_refs;
> + for_each_ref(append_similar_ref, &ref_cb);
> + return similar_refs;
> +}
> +
> +void help_unknown_ref(const char *ref, const char *cmd, const char *error)
> +{
> + int i;
> + struct string_list suggested_refs = guess_refs(ref);
> +
> + fprintf_ln(stderr, _("%s: %s - %s"), cmd, ref, error);
> +
> + if (suggested_refs.nr > 0) {
> + fprintf_ln(stderr,
> + Q_("\nDid you mean this?",
> + "\nDid you mean one of these?",
> + suggested_refs.nr));
> + for (i = 0; i < suggested_refs.nr; i++)
> + fprintf(stderr, "\t%s\n", suggested_refs.items[i].string);
> + }
> +
> + string_list_clear(&suggested_refs, 0);
> + exit(1);
> +}
Looks sensible.
> diff --git a/help.h b/help.h
> index 0ae5a12..5003423 100644
> --- a/help.h
> +++ b/help.h
> @@ -27,4 +27,9 @@ extern void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes);
> extern int is_in_cmdlist(struct cmdnames *cmds, const char *name);
> extern void list_commands(unsigned int colopts, struct cmdnames *main_cmds, struct cmdnames *other_cmds);
>
> +/*
> + * This function has been called because ref is not known.
> + * Print a list of refs that the user might have meant, and exit.
> + */
The wording is a bit funny; I'll amend it somehow before queuing.
> +extern void help_unknown_ref(const char *ref, const char *cmd, const char *error);
> #endif /* HELP_H */
--
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