Vikrant Varma wrote:
> Give better advice when trying to merge a branch that doesn't exist. If
> the branch exists in any remotes, display a list of suggestions.
Interesting. Thanks for working on this.
You say advice, but you're not invoking advise() or guarding the
advice with an advice.* -- the advice is undoubtedly helpful, but not
everyone wants to see it.
> diff --git a/help.c b/help.c
> index 02ba043..faf18b9 100644
> --- a/help.c
> +++ b/help.c
> @@ -7,6 +7,7 @@
> #include "string-list.h"
> #include "column.h"
> #include "version.h"
> +#include "refs.h"
>
> void add_cmdname(struct cmdnames *cmds, const char *name, int len)
> {
> @@ -404,3 +405,46 @@ int cmd_version(int argc, const char **argv, const char *prefix)
> printf("git version %s\n", git_version_string);
> return 0;
> }
> +
> +struct similar_ref_cb {
I see that there are other structs in our codebase suffixing _cb, to
indicate "callback data". I normally reserve _cb for callback
functions.
> + const char *base_ref;
> + struct string_list similar_refs;
Okay, so you might have more than one matching candidate.
> +static int append_similar_ref(const char* refname, const unsigned char *sha1, int flags, void *cb_data)
> +{
> + int i;
> + struct similar_ref_cb *cb = (struct similar_ref_cb *)(cb_data);
> + for (i = strlen(refname); refname[i] != '/'; i--)
> + ;
Er, what is this? A re-implementation of strrchr()?
> + /* A remote branch of the same name is deemed similar */
> + if (!prefixcmp(refname, "refs/remotes/") && !strcmp(refname + i + 1, cb->base_ref))
> + string_list_append(&(cb->similar_refs), refname + 13);
What is 13? Please use strlen("refs/remotes/") for readability.
I don't like the + i + 1 thing, but you should be able to get rid of
it with strrchr().
> +void help_unknown_ref(const char* ref) {
> + int i;
> + struct similar_ref_cb ref_cb;
> + ref_cb.similar_refs = (struct string_list)STRING_LIST_INIT_NODUP;
Why are you casting STRING_LIST_INIT_NODUP?
> + ref_cb.base_ref = ref;
> +
> + for_each_ref(append_similar_ref, &ref_cb);
> +
> + fprintf_ln(stderr, _("fatal: %s - not something we can merge"), ref);
Hm, "fatal: " was emitted by die() earlier. I wonder if something
like error() will be nicer than hard-coding "fatal: ".
> + if (ref_cb.similar_refs.nr > 0) {
> + fprintf_ln(stderr,
> + Q_("\nDid you mean this?",
> + "\nDid you mean one of these?",
> + ref_cb.similar_refs.nr));
Hm, why did you use Q_?
> + for (i = 0; i < ref_cb.similar_refs.nr; i++)
> + fprintf(stderr, "\t%s\n", ref_cb.similar_refs.items[i].string);
> + }
> + exit(1);
die() exits with 128, no? Why are you exiting with 1 now?
--
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