Re: [PATCH v6 6/6] config: "git config --get-urlmatch" parses section.<url>.key

On Jul 31, 2013, at 15:45, Jeff King wrote:

On Wed, Jul 31, 2013 at 12:26:08PM -0700, Junio C Hamano wrote:

quoted

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.