Christian Couder <christian.couder@xxxxxxxxx> writes:
>> + const char *query[][2] = {
>> + { ignore_case ? "Key1" : "key1", "value1" },
>> + { ignore_case ? "keY2" : "key2", "value2" },
>> -+ { ignore_case ? "foobarfrotz" : "fooBarFrotz", "value3" }
>> ++ { ignore_case ? "FOObarFrotz" : "fooBarFrotz", "value3" },
>> ++ { ignore_case ? "TENOR" : "tenor",
>> ++ ignore_case ? "value4" : "value5" }
>> + };
>
> I suggested adding the following test case:
>
> { ignore_case ? "FOOBarFrotZ" : "foobarfrotz",
> ignore_case ? : "value3" : "value4" }
>
> to better check that things work well especially when not ignoring the case.
>
> This is because, when not ignoring the case, there used to be a choice
> between { "fooBarFrotz", "value3" } and { "foobarfrotz", "value4" }
> that can be decided only by the case of the key in 'query'. But
> instead you removed that choice from 'key_val'.
>
> And it seems to me that the new test case you added doesn't bring much value.
>
> Maybe there was something wrong in what I suggested, but then please explain it.
>
> The other changes look good to me and seem to address the suggestions
> made by others and me.
>
> Thanks!
Thanks for a careful review, and of course thanks for working on the
patch, A U Thor ;-)
