Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> A less well known edge case in the config format is that keys can be
> value-less, a shorthand syntax for "true" boolean keys. I.e. these two
> are equivalent as far as "--type=bool" is concerned:
>
> [a]key
> [a]key = true
>
> But as far as our parser is concerned the values for these two are
> NULL, and "true". I.e. for a sequence like:
>
> [a]key=x
> [a]key
> [a]key=y
>
> We get a "struct string_list" with "string" members with ".string"
> values of:
>
> { "x", NULL, "y" }
>
> This behavior goes back to the initial implementation of
> git_config_bool() in 17712991a59 (Add ".git/config" file parser,
> 2005-10-10).
I didn't know about this behavior before, actually. Thanks for the
explanation.
> When the "t/t1308-config-set.sh" tests were added in [1] only one of
> the three "(NULL)" lines in "t/helper/test-config.c" had any test
> coverage. This change adds tests that stress the remaining two.
I initially read this as testing that t/helper/test-config.c is doing
the right thing, which would be the antipattern of writing tests for our
tests.
During Review Club, you mentioned that the motivation was something
else, which IIRC is closer to exercising the internals of the configset
API, which makes sense to me, thought it would be helpful to clarify
that better in the commit message.
> +test_expect_success 'emit multi values from configset with NULL entry' '
> + test_when_finished "rm -f my.config" &&
> + cat >my.config <<-\EOF &&
> + [a]key=x
> + [a]key
> + [a]key=y
> + EOF
> + cat >expect <<-\EOF &&
> + x
> + (NULL)
> + y
> + EOF
> + test-tool config configset_get_value_multi a.key my.config >actual &&
> + test_cmp expect actual
> +'
So if this meant to exercise configset_get_value_multi(), maybe it would
be even clearer to just say so in the test name, e.g.
'configset_get_value_multi with NULL entry'.
Side comment: by the end of the series, *_get_value_multi() has no
legitimate callers outside of config.c and the test code, and if we
remove it from config.h, this scenario wouldn't ever bother us in actual
`git` usage, but we probably still want to test for it since we want to
exercise the internals.
