git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Michael Grubb <devel@dailyvoid.com>
Cc: git@vger.kernel.org, Jonathan Nieder <jrnieder@gmail.com>,
	vmiklos@frugalware.org
Subject: Re: [PATCH v4] Add default merge options for all branches
Date: Tue, 03 May 2011 15:03:16 -0700	[thread overview]
Message-ID: <7vwri7h26z.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <4DC0608F.9040208@dailyvoid.com> (Michael Grubb's message of "Tue, 03 May 2011 15:07:43 -0500")

Michael Grubb <devel@dailyvoid.com> writes:

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

> +/* 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.

  parent reply	other threads:[~2011-05-03 22:03 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-02 19:23 [PATCH 2] Add default merge options for all branches Michael Grubb
2011-05-02 22:47 ` Miklos Vajna
2011-05-02 23:36 ` Junio C Hamano
2011-05-03  5:35   ` Michael Grubb
2011-05-03  5:38 ` [PATCH v3] " Michael Grubb
2011-05-03  9:03   ` Jonathan Nieder
2011-05-03  9:49     ` Jonathan Nieder
2011-05-03 16:46     ` Michael Grubb
2011-05-03 18:16     ` Junio C Hamano
2011-05-03 20:22       ` Michael Grubb
2011-05-03 20:50         ` Jonathan Nieder
2011-05-03 20:37       ` Jens Lehmann
2011-05-03 20:07     ` [PATCH v4] " Michael Grubb
2011-05-03 20:36       ` Michael Grubb
2011-05-03 20:44       ` Jonathan Nieder
2011-05-03 22:51         ` Junio C Hamano
2011-05-04  4:25           ` Junio C Hamano
2011-05-04  4:28             ` Michael Grubb
2011-05-04  4:58               ` Jonathan Nieder
2011-05-04 18:58                 ` Michael Grubb
2011-05-04 21:35                   ` Junio C Hamano
2011-05-04 10:58             ` John Szakmeister
2011-05-03 22:03       ` Junio C Hamano [this message]
2011-05-03 20:37     ` [PATCH v4.1] " Michael Grubb
2011-05-04 22:07     ` [PATCH v5] " Michael Grubb
2011-05-05  0:42       ` Junio C Hamano
2011-05-06 20:36         ` Junio C Hamano
2011-05-06 21:59           ` Jonathan Nieder
2011-05-06 20:54         ` [PATCH 0/2] tests: make verify_merge check that the number of parents is right Jonathan Nieder
2011-05-06 20:58           ` [PATCH 1/2] tests: eliminate unnecessary setup test assertions Jonathan Nieder
2011-05-06 21:48             ` Jeff King
2011-05-06 22:13               ` Jeff King
2011-05-06 22:27                 ` Junio C Hamano
2011-05-06 22:29                   ` Jeff King
2011-05-07 22:05                     ` Junio C Hamano
2011-05-09 13:31                     ` [PATCH 0/3] blame --line-porcelain Jeff King
2011-05-09 13:33                       ` [PATCH 1/3] add tests for various blame formats Jeff King
2011-05-09 13:34                       ` [PATCH 2/3] blame: refactor porcelain output Jeff King
2011-05-09 15:39                         ` Thiago Farina
2011-05-09 13:34                       ` [PATCH 3/3] blame: add --line-porcelain output format Jeff King
2011-05-06 22:26               ` [PATCH 1/2] tests: eliminate unnecessary setup test assertions Jonathan Nieder
2011-05-06 21:00           ` [PATCH 2/2] tests: teach verify_parents to check for extra parents Jonathan Nieder
2011-05-06 21:34             ` Junio C Hamano
2011-05-06 21:42               ` Jonathan Nieder
2011-05-06 21:32         ` [PATCH v5] Add default merge options for all branches Jonathan Nieder
2011-05-06 22:01           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vwri7h26z.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=devel@dailyvoid.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=vmiklos@frugalware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).