Re: [PATCH 1/2] keyval: accept escaped commas in implied option

[Paolo Bonzini <pbonzini@redhat.com>]

On 27/11/20 15:39, Markus Armbruster wrote:
>> Nah, just lazy cut-and-paste of the existing error message.  I should
>> rename that error to something "No implicit parameter name for '.key'"
>> (again, different grammar -> different parser -> different error).  That
>> error message actually makes sense: "--object .key" would create an
>> object of type ".key" both without or with these changes.
> However, --object a=b,.key would not, because the sugar is available
> for the leftmost value only.
> 
> "No implicit parameter name" assumes the user intended .key as a value,
> and forgot to write the key.  We could instead assume the user intended
> .key as key, and messed it up (forgot a fragment, fat-fingered '.',
> whatever).  The absence of '=' makes the value assumption more
> plausible, but that's already lookahead.

To be fair, lookahead is a common trick to get better error messages.

The typical example is C's "id1 id2".  After "id1 id2" you already know 
it's a syntax error, but you do some lookahead because "id1 id2;" can be 
recovered as "id1 was supposed to be a type, so treat this as declaring 
a variable id2".  "id1 id2)" is not handled the same way.

Of course that's done for a completely different reason (cascading error 
messages---QEMU only reports the first), but it goes to show that 
parsing ahead is not necessarily a bad idea

> Error messages based on guesses what the user has in mind can be quite
> confusing when we guess wrong.  A strictly factual syntax error style
> like "I expected FOO instead of BAR here" may not be great, but has a
> relatively low risk of being confusing.

This is true.  That's a point in favor of "Expected '=' after parameter".

>>>
>>>
>>>      master:       Invalid parameter 'key.1a.b'
>>>      your patch:   same
>>>
>>>      Slightly better, I think:
>>>
>>>                    'key.1a' is not a valid parameter name
>>
>> Or just "Invalid parameter '1a'".  I'm not going to do that in v2 
>> though, since parameter parsing is not being
> 
> Sentence not being finished?

not being modified.

>> This invocation is useful (for some value of useful) to see which 
>> properties you can pass with -global.  So there *is* a valid (for some 
>> value of valid) use of escaped commas in implied options.  It can be 
>> fixed with deprecation etc. but that would be a more complicated 
>> endeavor than just adjusting keyval.
> 
> The question becomes whether CLI help syntax is subject to the
> compatibility promise.

Indeed.  But I still don't see it as a good reason _not_ to do the 
change, as I find the modified definition (grammar, code, etc.) to be 
easier on the brain too.

Paolo