From: Junio C Hamano <gitster@pobox.com>
To: "Kyle J. McKay" <mackyle@gmail.com>
Cc: GIT Mailing-list <git@vger.kernel.org>, Jeff King <peff@peff.net>
Subject: Re: [PATCH 5/3] revert most of the http_options() change
Date: Tue, 30 Jul 2013 14:09:37 -0700 [thread overview]
Message-ID: <7vy58nvjou.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <3C93D9D6-8FC3-4D94-AE6E-37150202314A@gmail.com> (Kyle J. McKay's message of "Tue, 30 Jul 2013 12:41:37 -0700")
"Kyle J. McKay" <mackyle@gmail.com> writes:
> And now all the t5200-url-normalize tests pass again.
>
> FYI, I couldn't get the patches to apply against next or pu without
> some minor tweaks that were just conflict resolutions having to do
> with git_config_with_options changing its signature.
Thanks.
I built these five patches directly on top of your series, i.e. on
top of cea9928d (docs: update http.<url>.* options documentation,
2013-07-25).
I'll rebuild the series with your fixes and I may even queue it to
'pu', but with some random thoughts:
* "url-match.[ch]" are ugly names. I'd be happier with
"urlmatch.[ch]".
* In the end result, http_options() looks mostly identical to that
of the 'master', but it got an extra "void *matched", only to
support "git config --get-urlmatch". I however do not have to,
if I do a few things:
- Instead of implementing urlmatch_config_item that extends
urlmatch_item, have a separate string_list, keyed by the
variable names, and point the string_list with the generic cb
pointer I already have in urlmatch_config. The fn() on the
"git config" side has to index this second string_list with the
variable and keep track of the value from the best candidate we
have seen so far.
- The above allows us to lose item_alloc and item_clear callback
functions from urlmatch_config, as we will not be doing the
structure inheritance to extend urlmatch_item.
- We still need cascade_fn callback, though.
* With the above, http_options() does not have to change in the
series. We could restructure the series perhaps this way:
- http.sslCertPasswordProtected parsing fix.
- Add urlmatch.[ch], which would be your "config: improve support
for http.<url>.* settings" and yesterday's "url-match: split
out URL matching logic out of http.c", and a half of
"url-match: generalize configuration collection logic".
- Update http.c to use urlmatch.[ch] to make http.c match the
result of the endgame patch [5/3], and add necessary end user
documentation to Documentation/config.txt.
- Add test-url-normalize and t/t5200
- Update builtin/config.c for "--get-urlmatch", to make
builtin/config.c match the result of the endgame patch [5/3],
add end user documentation to Documentation/git-config.txt.
- Add some tests for "git config --get-urlmatch".
next prev parent reply other threads:[~2013-07-30 21:09 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-24 19:07 [PATCH] test-url-normalize.c: Fix gcc errors and sparse warnings Ramsay Jones
2013-07-24 19:35 ` Kyle J. McKay
2013-07-24 20:39 ` Junio C Hamano
2013-07-29 22:49 ` [PATCH 0/3] "git config --get-urlmatch $section.$key $url" Junio C Hamano
2013-07-29 22:49 ` [PATCH 1/3] url-match: split out URL matching logic out of http.c Junio C Hamano
2013-07-29 22:49 ` [PATCH 2/3] builtin/config: refactor collect_config() Junio C Hamano
2013-07-29 22:49 ` [PATCH 3/3] config: --get-urlmatch Junio C Hamano
2013-07-30 0:37 ` Jeff King
2013-07-30 1:33 ` Junio C Hamano
2013-07-30 8:14 ` Jeff King
2013-07-30 13:52 ` Junio C Hamano
2013-07-30 19:14 ` Kyle J. McKay
2013-07-30 2:03 ` [PATCH 4/3] url-match: generalize configuration collection logic Junio C Hamano
2013-07-30 2:13 ` [PATCH 5/3] revert most of the http_options() change Junio C Hamano
2013-07-30 19:14 ` Kyle J. McKay
2013-07-30 19:41 ` Kyle J. McKay
2013-07-30 21:09 ` Junio C Hamano [this message]
2013-07-31 17:59 ` [PATCH 0/3] "git config --get-urlmatch $section.$key $url" Ramsay Jones
2013-07-31 19:31 ` 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vy58nvjou.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=mackyle@gmail.com \
--cc=peff@peff.net \
/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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.