Re: [PATCH v4] Add default merge options for all branches

From: Junio C Hamano <gitster@pobox.com>
Date: May 3, 2011, 22:03 UTC
Message-ID: <7vwri7h26z.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4DC0608F.9040208@dailyvoid.com>

Michael Grubb <devel@dailyvoid.com> writes:

/*
 * Our Multi-line comments begin with a line with
 * slash asterisk then newline.
 */

Show 9 quoted lines

> +/* This is for branch.<foo>. blocks
> + * the vote member holds a value between
> + * 0.0 and 1.0 which measures how closely
> + * a branch name matches the key member.
> + * where branch.*.mergeoptions would be 0.1 and
> + * branch.<name>.mergeoptions would be 1.0
> + * Also it is called vote because I couldn't come
> + * up with a better name.
> + */

How about simply dropping that "vote" thing? I do not want to see unnecessary float creeping into our codebase.

The k and v parameters are volatile from the point of view of this function. You need to xstrdup() them to keep a copy.

There is no need to store "branch." part in cb->key, as it is common across the variables.

The logic would probably look like this:

	if (prefixcmp(k, "branch."))
		return;
	k += 7; /* past "branch." part */
	eon = strrchr(k, '.'); /* end-of-name 8/
        if (!eon || strcmp(eon, ".mergeoptions"))
        	return;

	/* k thru eon is the name or wildcard */
	spec = xmemdupz(k, eon - k);
        /*
         * NEEDSWORK: for now we say "*" matches; we would need
         * to turn the following into something like:
         *	if (has_wildcard(spec) 
	 *		? !glob_matches(spec, branch)
	 *		: strcmp(spec, branch)) {
         *		free(spec);
         *		return;
         *	}
         */
	if (strcmp(spec, "*") && strcmp(spec, branch)) {
        	free(spec);
                return;
	}

        if (!merge_options->option ||
             cmp_specificity(merge_options->spec, spec) < 0) {
		/* use this one */
                free(merge_options->spec);
                free(merge_options->option);
                merge_options->option = xstrdup(v);
                merge_options->spec = spec;
		return;
	}
        free(spec);

And then cmp_specificity() would say something like:

	static int cmp_specificity(const char *a, const char *b)
        {
        	switch ((!strcmp(a, "*") ? 2 : 0) |
                	(!strcmp(b, "*") ? 1 : 0)) {
		case 3:
                        /*
                         * NEEDSWORK: when we start truly globbing,
                         * we need to decide "foo/*" is more specific than
                         * "*" and the like. But for now we do not have to
                         * worry about that case.
                         */
		case 0:
                        return -1; /* later one wins if they are the same */
		case 1:
			return 1;
		case 2:
			return -1;
		}
	}

meaning, the ones with wildcard are weaker than the ones without.