Re: [PATCH 3/3] config: --get-urlmatch
On Mon, Jul 29, 2013 at 06:33:43PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: +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? git config does not know the semantics of each key, nor available set of keys, no? The string-list is only to support git config --get-urlmatch http http://www.google.com/ i.e. list everything under http.* hierarchy. Ah, I missed that you could leave key empty. I had expected collect-key to be filled in, at which point you only ever have one such key (and you do not need to know the semantics, only which one is the winner). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] config: --get-urlmatch
Jeff King p...@peff.net writes: Ah, I missed that you could leave key empty. Yes, the general syntax is git config [--type] --get-urlmatch section[.key] url and giving section without a specific key would list all the variables in the section that apply to url. This is why we should do documentation at some point before publishing a patch series; sorry about that. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] config: --get-urlmatch
On Jul 29, 2013, at 15:49, 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. In addition to $section.$variable, entries in the configuration file(s) that match $section.urlpattern.$variable are looked up and the one with urlpattern that matches the given $url the best is used to answer the query. This can still be further refactored to remove code from http_options() in http.c, I think. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/config.c | 141 ++ + 1 file changed, 141 insertions(+) diff --git a/builtin/config.c b/builtin/config.c index 12c5073..c1d32ae 100644 --- a/builtin/config.c +++ b/builtin/config.c [...] +static int get_urlmatch(const char *var, const char *url) +{ + const char *section_tail; + struct string_list_item *item; + struct urlmatch_collect collect = { STRING_LIST_INIT_DUP }; + + if (!url_normalize(url, collect.url)) + die(collect.url.err); The value now stored in collect.url.url is never freed. + + section_tail = strchr(var, '.'); + if (section_tail) { + collect.section = xmemdupz(var, section_tail - var); + collect.key = strrchr(var, '.') + 1; + show_keys = 0; + } else { + collect.section = var; + collect.key = NULL; + show_keys = 1; + } + + git_config_with_options(urlmatch_collect, collect, + given_config_file, respect_includes); + + for_each_string_list_item(item, collect.vars) { + struct urlmatch_item *matched = item-util; + struct strbuf key = STRBUF_INIT; + struct strbuf buf = STRBUF_INIT; + + strbuf_addstr(key, collect.section); + strbuf_addch(key, '.'); + strbuf_addstr(key, item-string); + format_config(buf, key.buf, + matched-value_is_null ? NULL : matched-value.buf); + fwrite(buf.buf, 1, buf.len, stdout); + strbuf_release(key); + strbuf_release(buf); + + strbuf_release(matched-value); + } + string_list_clear(collect.vars, 1); Needs something like this here: + free(collect.url.url); + + /* +* section name may have been copied to replace the dot, in which +* case it needs to be freed. key name is either NULL (e.g. 'http' +* alone) or points into var (e.g. 'http.savecookies'), and we do +* not own the storage. +*/ + if (collect.section != var) + free((void *)collect.section); + return 0; +} Still needed after 4/3 except it's now config.url.url instead. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] config: --get-urlmatch
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? +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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] config: --get-urlmatch
Jeff King p...@peff.net writes: +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? git config does not know the semantics of each key, nor available set of keys, no? The string-list is only to support git config --get-urlmatch http http://www.google.com/ i.e. list everything under http.* hierarchy. And unlike http_option() which can incrementally always call back the setter (and let it override older value), the command has to read everything through and then give us the final value, so I do not think we can get away without one. 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 That is in the works, of course ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html