On Fri, May 21 2021, Jeff King wrote:
> On Fri, May 21, 2021 at 08:23:15AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> The "performant" really doesn't matter here, we're comparing shelling
>> out to getting a small number of config keys back. So I wasn't trying to
>> optimize this.
>>
>> Returning a flattened list is idiomatic in Perl, it means that a caller
>> can do any of:
>>
>> # I only care about the last value for a key, or only about
>> # existence checks
>> my %hash = func();
>>
>> Or:
>>
>> # I want all key-values to iterate over
>> my @kv = func();
>>
>> Returning touples like this makes that less convenient for both, who'll
>> need to do more work to unpack them.
>>
>> For what it's worth In Perl "return a list" means "flattened list", the
>> term "flattened list" I think comes from other languages. You'd call
>> what you're suggesting a list of arrays, or (if a top-level reference)
>> an array of arrays, AoA for short, AoH for array (ref) of hashes etc.
>
> Yeah, I think that is reasonable. But it made me wonder how we handle
> value-less booleans, and I think there's indeed a bug.
>
> Try a config like this:
>
> $ cat >foo <<\EOF
> [foo]
> key-with-value = string
> key-with-empty =
> just-bool
> another-key-with-value = another
> EOF
>
> A regular config --list looks like:
>
> $ git config --file=foo --list
> foo.key-with-value=string
> foo.key-with-empty=
> foo.just-bool
> foo.another-key-with-value=another
>
> Note how "just-bool" drops the "=" to distinguish it from the empty
> string. With "-z", it looks like this:
>
> $ git config --file=foo --list -z
> foo.key-with-value
> string^@foo.key-with-empty
> ^@foo.just-bool^@foo.another-key-with-value
> another^@
>
> The NULs separate keys, but keys are separated from their values by a
> newline. And again, just-bool omits the newline.
>
> Your parser splits on newline, so for that entry we'll get only one
> string returned in the list (the key), rather than two (the key and
> value). In a flattened list, that becomes ambiguous. E.g., adapting your
> parser into a stand-alone script:
>
> $ git config --file=foo --list -z |
> perl -e '
> local $/;
> my $data = <STDIN>;
> my (@kv) = map { split /\n/, $_, 2 } split /\0/, $data;
>
> while (@kv) {
> my $k = shift @kv;
> print "key: $k\n";
> my $v = shift @kv;
> print " value: ", (defined $v ? $v : "undef"), "\n";
> }
> '
> key: foo.key-with-value
> value: string
> key: foo.key-with-empty
> value:
> key: foo.just-bool
> value: foo.another-key-with-value
> key: another
> value: undef
>
> We end up misinterpreting a key as a value, and vice versa.
>
> Using a non-flattened structure would have prevented this (we'd sensibly
> get undef when trying to access the missing second element of the
> array). But I do agree the flattened structure is more perl-ish.
> Probably you'd want to insert an explicit "undef" into the list. The
> most perl-ish I could come up with is:
>
> my (@kv) = map { my ($k, $v) = split /\n/, $_, 2;
> ($k, $v)
> } split /\0/, $data;
>
> I notice that $known_keys then becomes a non-flat representation. You'd
> either want to turn that back into a zero-length array there, or store
> the "undef" and handle it appropriately (it can be a synonym for "true",
> though that is just an optimization at this point).
>
> -Peff
Ah yes, that's indeed a bug. I'd forgetten about the empty value case.
For what it's worth you can slightly golf that as (split /\n/, $_,
2)[0,1], but I think in this case your version is better than that, it's
more obvious what we're trying to do in always returning the $v.
