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..config topic. In addition to $section.$variable, entries in the configuration file(s) that match $section..$variable are looked up and the one with 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 --- 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
Jeff King writes: > Ah, I missed that you could leave "key" empty. Yes, the general syntax is git config [--] --get-urlmatch [.] and giving without a specific would list all the variables in the section that apply to . 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 Mon, Jul 29, 2013 at 06:33:43PM -0700, Junio C Hamano wrote: > Jeff King 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 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
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..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
[PATCH 3/3] config: --get-urlmatch
"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..config topic. In addition to $section.$variable, entries in the configuration file(s) that match $section..$variable are looked up and the one with 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 --- 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 @@ -2,6 +2,7 @@ #include "cache.h" #include "color.h" #include "parse-options.h" +#include "url-match.h" static const char *const builtin_config_usage[] = { N_("git config [options]"), @@ -41,6 +42,7 @@ static int respect_includes = -1; #define ACTION_SET_ALL (1<<12) #define ACTION_GET_COLOR (1<<13) #define ACTION_GET_COLORBOOL (1<<14) +#define ACTION_GET_URLMATCH (1<<15) #define TYPE_BOOL (1<<0) #define TYPE_INT (1<<1) @@ -57,6 +59,7 @@ static struct option builtin_config_options[] = { OPT_BIT(0, "get", &actions, N_("get value: name [value-regex]"), ACTION_GET), OPT_BIT(0, "get-all", &actions, N_("get all values: key [value-regex]"), ACTION_GET_ALL), OPT_BIT(0, "get-regexp", &actions, N_("get values for regexp: name-regex [value-regex]"), ACTION_GET_REGEXP), + OPT_BIT(0, "get-urlmatch", &actions, N_("get value specific for the URL: section[.var] URL"), ACTION_GET_URLMATCH), OPT_BIT(0, "replace-all", &actions, N_("replace all matching variables: name value [value_regex]"), ACTION_REPLACE_ALL), OPT_BIT(0, "add", &actions, N_("add a new variable: name value"), ACTION_ADD), OPT_BIT(0, "unset", &actions, N_("remove a variable: name [value-regex]"), ACTION_UNSET), @@ -348,6 +351,140 @@ static int get_colorbool(int print) return get_colorbool_found ? 0 : 1; } +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; +}; + +static int urlmatch_collect(const char *var, const char *value, void *cb) +{ + struct string_list_item *item; + struct urlmatch_collect *collect = cb; + struct urlmatch_item *matched; + struct url_info *url = &collect->url; + const char *key, *dot; + size_t matchlen = 0; + int usermatch = 0; + + key = skip_prefix(var, collect->section); + if (!key || *(key++) != '.') + return 0; /* not interested */ + dot = strrchr(key, '.'); + if (dot) { + char *config_url, *norm_url; + struct url_info norm_info; + int matchlen; + + config_url = xmemdupz(key, dot - key); + norm_url = url_normalize(config_url, &norm_info); + free(config_url); + if (!norm_url) + return 0; + matchlen = match_urls(url, &norm_info, &usermatch); + free(norm_url); + if (!matchlen) + return 0; + key = dot + 1; + } + + if (collect->key && strcmp(key, collect->key)) + return 0; + + item = string_list_insert(&collect->vars, key); + if (!item->util) { + matched = xcalloc(1, sizeof(*matched)); + item->util = matched; + strbuf_init(&matched->value, 0); + } else { + matched = item->util; + /* +* Is our match shorter? Is our match the same +* length, and without user while the current +* candidate is with user? Then we cannot use it. +*/ + if (matchlen < matched->max_matched_len || + ((matchlen == matched->max_matched_len) && +(!usermatch && matched->user_matched))) + return 0; + /* +* Otherwise, release the old one and replace +* with ours. +*/ + strbuf_release(&matched->value); + } + + matched->max_matched_len = matchlen; + matched->user_matched = usermatch; + if (value) { + strbuf_addstr(&matched->value, value); + matched->value_is_null = 0; + } else { + matched->value_is_null = 1; + } + return 0; +} + +static int get_urlmatch(const char *var, const char *url) +{ + const char *section_tail; + struct string_list_item *item; + struct urlmatch_collect collect = { ST