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

[Paolo Bonzini]

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