[pulseaudio-discuss] RFC: Change the modarg parser to keep the raw form

[Colin Guthrie]

'Twas brillig, and Maarten Bosmans at 07/09/11 08:41 did gyre and gimble:
> 2011/9/6 Colin Guthrie <gmane at colin.guthr.ie>:
>> 'Twas brillig, and Colin Guthrie at 06/09/11 11:53 did gyre and gimble:
>>> Late in the cycle so wanted to get more eyes on this before pushing.
>>>
>>> These patches simply modify things to ensure that the 'raw' form of
>>> modargs will still be accessable and thus can be used by the proplist
>>> parser.
>>>
>>> Now, this does break the proplist-test as it no longer parses the triple
>>> escaped input. IMO, this is acceptable, as there should be no need to triple
>>> escape things due to a quirk of how the modargs are parsed in the first place.
> 
> The gain in consistency outweighs the loss of compitibility here, IMHO.
> With gain in consistency, I mean that regardless of the inner level of
> quoting, you only need one escape for a quote in the string.
> before:
>    sink_properties="device.description='escape \' me'"
>    sink_properties='device.description="escape \\' me"'
> with patch:
>    sink_properties="device.description='escape \' me'"
>    sink_properties='device.description="escape \' me"'


I agree this is nicer.

>>> That said, compatibility may be more important here? I suspect the impact of
>>> a change would be minimal however.
>>
>> Just for the sake of completeness, this does have some drawbacks.
>>
>> Under the old scheme, you could vary which type of quotes you used for
>> your property assignments in the proplist itself. (i.e. mix and match
>> double quotes, vs single quotes: foo="bar" foo2='wibble'
>>
>> With this new "improved" scheme, we have to use the same style quotes
>> all the way through - i.e. the opposite type to what we delimit the
>> argument itself with:
>>
>> e.g.
>>  sink_properties="foo='bar\"quote\"' foo2='wibble'"
>> or
>>  sink_properties='foo="bar\"quote\"" foo2="wibble"'
> 
> Indeed, this works, but I was initially thrown off by the double-quote
> escaping of pactl/pulseaudio output. But that's only done on output
> (by pa_proplist_to_string_sep), so no problem.

Yeah I got tripped up by that too!

>> both work fine, but you can't mix and match.
>>
>> IMO, even with this limitation, it is still simpler than passing:
>>  sink_properties="foo='bar\\\"quote\\\"' foo2='wibble'"
>> or
>>  sink_properties='foo="bar\\\"quote\\\"" foo2="wibble"'
> 
> As you mentioned on IRC, the triple quoting is only when using it in c
> source code. For example in default.pa it's just a single vs. double
> quote issue.

Yup.... I went a bit "Inception" on ya there :p


>> For reference, this is the change needed to "fix" the test:
>>
>> diff --git a/src/tests/proplist-test.c b/src/tests/proplist-test.c
>> index 27a0d3f..8b5a235 100644
>> --- a/src/tests/proplist-test.c
>> +++ b/src/tests/proplist-test.c
>> @@ -81,7 +81,7 @@ int main(int argc, char*argv[]) {
>>     printf("%s\n", v);
>>     pa_xfree(v);
>>
>> -    pa_assert_se(ma = pa_modargs_new("foo='foobar=waldo
>> foo2=\"lj\\\\\"dhflh\" foo3=\\'kjlskj\\\\\\'\\''", x));
>> +    pa_assert_se(ma = pa_modargs_new("foo='foobar=waldo
>> foo2=\"lj\\\"dhflh\" foo3=\"kjlskj\\'\"'", x));
>>     pa_assert_se(a = pa_proplist_new());
>>
>>     pa_assert_se(pa_modargs_get_proplist(ma, "foo", a,
>> PA_UPDATE_REPLACE) >= 0);
> 
> Yup, that would be an improvement.

Glad you agree :)


Any other opinions on this one before I push it?

Lennart, you in particular!

Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]