Re: [PATCH 3/3] config: --get-urlmatch

From: Jeff King <peff@peff.net>
Date: Jul 30, 2013, 00:37 UTC
Message-ID: <20130730003716.GA13114@sigill.intra.peff.net>
In-Reply-To: <1375138150-19520-4-git-send-email-gitster@pobox.com>

On Mon, Jul 29, 2013 at 03:49:10PM -0700, Junio C Hamano wrote:

> "git config --get-urlmatch $section[.$variable] $url" is a way to
> learn what the configured value for $section.$variable is for the
> given URL, using the logic introduced by the http.<url>.config
> topic.

That interface looks good to me. It matches the --get-all/--get-regexp options of git-config. It does not quite match --get-color, which is really a type (like --bool or --int), but I think --get-color is the odd one out there.

> This can still be further refactored to remove code from http_options()
> in http.c, I think.

Yeah, that would be the ultimate goal. I think you would just need to keep the same string_list there, with one urlmatch_item per key that we care about.

> ---
>  builtin/config.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++++++

It seems like some of the logic here should be reusable for http.c. Should it be in config.c instead?

Show 13 quoted lines

> +struct urlmatch_collect {
> +	struct string_list vars;
> +	struct url_info url;
> +	const char *section;
> +	const char *key;
> +};
> +
> +struct urlmatch_item {
> +	size_t max_matched_len;
> +	char user_matched;
> +	char value_is_null;
> +	struct strbuf value;
> +};

I think you ultimately want such a string_list for matching arbitrary numbers of keys, but do you need it for the git-config case?

You will always be matching collect->key, so you will only ever insert a single item into the collect->vars list. IOW, this could be:

  struct urlmatch_collect {
    struct url_info url;
    const char *section;
    const char *key;
    struct urlmatch_item match;
  };

I don't mind if it is more complicated than this single-case needs to be if the code is also being used to http.c, but I haven't seen that yet (and this code would not used as-is, as you would want to call the function with many potential collect->key values, but without re-normalizing the URL over and over).

And of course, it needs docs and tests. :) The latter can probably come by converting some of the test-url-normalize tests to just use git-config instead.

-Peff