All of lore.kernel.org
 help / color / mirror / Atom feed
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".

  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.