Paolo Bonzini <pbonz...@redhat.com> writes: > On 22/01/21 14:48, Markus Armbruster wrote: >> --nbd .key= >> master: Invalid parameter '..key' >> your patch: same >> Likweise. >> If I omit the '=', your patch's message changes to >> No implicit parameter name for value 'key..' >> I consider that worse than before, because it's talking about >> something outside the user's control (lack of an implict parameter >> name) where it should instead tell the user what needs fixing in the >> input. > > I think whether it's better or worse depends on the specific erroneous > command line (think "--nbd /path/to/file.qcow2"), but I can certainly > change it.
Quoting myself: 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. >> Your patch also adds an "Expected parameter at end of string" error. >> Can you tell me how to trigger it? > > It is meant for "--nbd ''" but it is effectively dead code due to the > "while (*s)" in the caller. Possibilities: > > 1) leave it in as dead code I'd prefer not to. > 2) replace it with an assert Works for me. > 3) change the caller to use a do...while in such a way that it > triggers it (and be careful not to change the grammar). Only if it's not too hard, and results in better error messages. >> I believe your grammar is ambiguous. Your code seems to pick the sane >> alternative. If I'm wrong, you need to enlighten me. If I'm right, you >> need to fix your grammar. > > Will do. Can I change the EBNF to use "+" and "*" for simplicity and > clarity? I tried to stick to ISO 14977 as described in <https://en.wikipedia.org/wiki/Extended_Backus%E2%80%93Naur_form>, less its foolish ',' for concatenation. I'm not at all enamored with it. All I want is something that is widely understood, and preferably not invented here. Switch to <https://www.w3.org/TR/xml/#sec-notation> perhaps? [...]
