[PATCH 2/3] ui: Switch "-display sdl" to use the QAPI parser

[Markus Armbruster]

Thomas Huth <thuth at redhat.com> writes:

> The "-display sdl" option still uses a hand-crafted parser for its
> parameters since some of them used underscores which is forbidden
> in QAPI.

Kind of.  QAPI indeed requires lower-case-with-hyphens for such names,
but there is an exception mechanism for names with upper case and
underscore: pragma member-name-exceptions.  Used for old names that
predate naming rule enforcement, and for newer names we've elected to
make consistent with their old siblings.

The series that QAPIfied -display partially explained why it left
-display sdl,... and -display vnc,... unQAPIfied in commit 7a61f43859
"ui: document non-qapi parser cases.":

    diff --git a/vl.c b/vl.c
    index 5f1734d851..3b39bbd7a8 100644
    --- a/vl.c
    +++ b/vl.c
    @@ -2114,6 +2114,16 @@ static void parse_display(const char *p)
         const char *opts;

         if (strstart(p, "sdl", &opts)) {
    +        /*
    +         * sdl DisplayType needs hand-crafted parser instead of
    +         * parse_display_qapi() due to some options not in
    +         * DisplayOptions, specifically:
    +         *   - frame
    +         *     Already deprecated.
    +         *   - ctrl_grab + alt_grab
    +         *     Not clear yet what happens to them long-term.  Should
    +         *     replaced by something better or deprecated and dropped.

This sounds like it was mostly reluctance to drag undesirables into the
QAPI schema.

Commit f6b560bbc1 "softmmu/vl: Remove obsolete comment about the "frame"
parameter" dropped item "frame".

Commit 8e8e844be4 "softmmu/vl: Add a "grab-mod" parameter to the
-display sdl option" decided the future of ctrl_grab + alt_grab.  It
replaced the second item's text:

    +         *     They can't be moved into the QAPI since they use underscores,
    +         *     thus they will get replaced by "grab-mod" in the long term

I figure they could've been moved to the QAPI schema back then.
Instead, we're getting rid of them without a detour through the schema.
Good!

    +         */
             dpy.type = DISPLAY_TYPE_SDL;
             while (*opts) {
                 const char *nextopt;
    @@ -2179,6 +2189,10 @@ static void parse_display(const char *p)
                 opts = nextopt;
             }
         } else if (strstart(p, "vnc", &opts)) {
    +        /*
    +         * vnc isn't a (local) DisplayType but a protocol for remote
    +         * display access.
    +         */
             if (*opts == '=') {
                 vnc_parse(opts + 1, &error_fatal);
             } else {

This remains, and that's fine.  One step at time.

>          Now that the problematic parameters have been removed, we can
> switch to use the QAPI parser instead.

Here's my attempt at a more accurate commit message.

  The "-display sdl" option still uses a hand-crafted parser for its
  parameters since we didn't want to drag an interface we considered
  somewhat flawed into the QAPI schema.  Since the flaws are gone now,
  it's time to QAPIfy.

> This introduces the new "DisplaySDL" QAPI struct that is used to hold
> the parameters that are unique to the SDL display. The only specific
> parameter is currently "grab-mod" which is modeled as a string, so that
> it could be extended for other arbitrary modifiers later more easily.

Are the values of @grab-mod parsed in any way, or do we recognize a set
of fixed strings?

The former would be problematic.  We try hard to represent complex data
as JSON instead of inventing little ad hoc languages.

If it's the latter, use an enum.  Makes introspection more useful, and
adding enumeration values is no harder than adding string literals.

> Signed-off-by: Thomas Huth <thuth at redhat.com>