From: "Kyle J. McKay" <mackyle@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH v6 6/6] config: "git config --get-urlmatch" parses section.<url>.key
Date: Wed, 31 Jul 2013 16:03:01 -0700 [thread overview]
Message-ID: <62E17EDB-B24D-4F37-95F8-E2E26118E5E9@gmail.com> (raw)
In-Reply-To: <20130731224511.GA25882@sigill.intra.peff.net>
On Jul 31, 2013, at 15:45, Jeff King wrote:
> On Wed, Jul 31, 2013 at 12:26:08PM -0700, Junio C Hamano wrote:
>
>> Using the same urlmatch_config_entry() infrastructure, add a new
>> mode "--get-urlmatch" to the "git config" command, to learn values
>> for the "virtual" two-level variables customized for the specific
>> URL.
>>
>> git config [--<type>] --get-urlmatch <section>[.<key>] <url>
>
> Do we want something like this on top, to convert the third form of
> test-url-normalize into git-config calls?
>
> It would be nicer squashed in, but we the tests are added earlier in
> the
> series than "--get-urlmatch", so we would have to rip the tests out of
> the earlier patches and have a "[PATCH 7/6] add tests for url
> normalizing".
>
> Two things to note about my test conversion:
>
> 1. Git-config expects pre-canonicalized variable names (so
> http.noepsv
> instead of "http.noEPSV"). I think the "git config --get" code
> path
> does this for the caller, so we should probably do the same for
> "--get-urlmatch". And it is even easier here, because we know that
> "http.noEPSV" does not contain a case-sensitive middle part. :)
The test was testing that too, which I think is a good thing. Your
replacement does not test that. With a fix for --get-urlmatch as you
mention above, the tests can check that again.
> 2. I turned the many 'test "$(git foo)" = "bar"' invocations into a
> wrapper function that uses test_cmp. This helped immensely with
> debugging (1).
>
> The wrapper is a little ugly. I do wonder if we actually need all
> of these tests (i.e., it is not clear to me what different things
> each is testing, and if it is not simply trying to exercise the
> different variable names, which now all follow the same code path,
> because git-config does not care about the particular names).
Each one tests a different item from the "$tc-n" config file to make
sure that everything that's in each config file actually behaves as
expected.
If we do this (and I don't really have any objection except for the
point noted above), then the tests really need to move out from t5200
as they're not tied to the http operations anymore. Also the Makefile
rule for test-url-normalize.c needs to be simplified since it won't
need the extra options to make it link since it's no longer including
http.c.
The README has this:
> First digit tells the family:
>
> 0 - the absolute basics and global stuff
> 1 - the basic commands concerning database
> 2 - the basic commands concerning the working tree
> 3 - the other basic commands (e.g. ls-files)
> 4 - the diff commands
> 5 - the pull and exporting commands
> 6 - the revision tree commands (even e.g. merge-base)
> 7 - the porcelainish commands concerning the working tree
> 8 - the porcelainish commands concerning forensics
> 9 - the git tools
But the best choice does not immediately jump out at me. However,
looking at the other tests that are there, I think perhaps 1307-config-
url might be a reasonable choice.
next prev parent reply other threads:[~2013-07-31 23:03 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-31 19:26 [PATCH v6 0/6] http.<url>.<key> and friends Junio C Hamano
2013-07-31 19:26 ` [PATCH v6 1/6] http.c: fix parsing of http.sslCertPasswordProtected variable Junio C Hamano
2013-07-31 19:26 ` [PATCH v6 2/6] config: add helper to normalize and match URLs Junio C Hamano
2013-07-31 20:50 ` Kyle J. McKay
2013-07-31 19:26 ` [PATCH v6 3/6] config: add generic callback wrapper to parse section.<url>.key Junio C Hamano
2013-07-31 19:26 ` [PATCH v6 4/6] config: parse http.<url>.<variable> using urlmatch Junio C Hamano
2013-07-31 20:51 ` Kyle J. McKay
2013-07-31 20:51 ` [PATCH ALTERNATIVE v6 0/2] http.<url>.<key> and friends Kyle J. McKay
2013-07-31 20:52 ` [PATCH ALTERNATIVE v6 2/4] config: add helper to normalize and match URLs Kyle J. McKay
2013-07-31 20:52 ` [PATCH ALTERNATIVE v6 4/4] config: parse http.<url>.<variable> using urlmatch Kyle J. McKay
2013-07-31 22:01 ` [PATCH ALTERNATIVE v6 0/2] http.<url>.<key> and friends Junio C Hamano
2013-07-31 22:41 ` [PATCH ALTERNATIVE v6.v2 4/6] config: parse http.<url>.<variable> using urlmatch Kyle J. McKay
2013-07-31 19:26 ` [PATCH v6 5/6] builtin/config: refactor collect_config() Junio C Hamano
2013-07-31 19:26 ` [PATCH v6 6/6] config: "git config --get-urlmatch" parses section.<url>.key Junio C Hamano
2013-07-31 22:45 ` Jeff King
2013-07-31 23:03 ` Kyle J. McKay [this message]
2013-07-31 23:44 ` Jeff King
2013-08-01 17:25 ` Junio C Hamano
2013-08-01 17:30 ` Jeff King
2013-08-05 20:20 ` [PATCH ALTERNATIVE v6.v3 4/6] config: parse http.<url>.<variable> using urlmatch Kyle J. McKay
2013-08-05 22:56 ` Junio C Hamano
2013-08-05 23:57 ` Kyle J. McKay
2013-07-31 23:47 ` [PATCH v6 6/6] config: "git config --get-urlmatch" parses section.<url>.key 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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=62E17EDB-B24D-4F37-95F8-E2E26118E5E9@gmail.com \
--to=mackyle@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).