Re: [PATCH] vl: Print display options for -display help

[BALATON Zoltan <balaton@eik.bme.hu>]

[-- Attachment #1: Type: text/plain, Size: 3135 bytes --]

On Thu, 14 Dec 2023, Philippe Mathieu-Daudé wrote:
> Hi Akihiko,
>
> On 14/12/23 07:47, Akihiko Odaki wrote:
>> -display lists display backends, but does not tell their options.
>> Use the help messages from qemu-options.def, which include the list of
>> options.
>> 
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   include/ui/console.h |  1 -
>>   system/vl.c          | 11 ++++++-----
>>   ui/console.c         | 20 --------------------
>>   3 files changed, 6 insertions(+), 26 deletions(-)
>
>
>> diff --git a/ui/console.c b/ui/console.c
>> index 7db921e3b7d6..6aee5e9a7ffb 100644
>> --- a/ui/console.c
>> +++ b/ui/console.c
>> @@ -1691,23 +1691,3 @@ const char *qemu_display_get_vc(DisplayOptions 
>> *opts)
>>       }
>>       return vc;
>>   }
>> -
>> -void qemu_display_help(void)
>> -{
>> -    int idx;
>> -
>> -    printf("Available display backend types:\n");
>> -    printf("none\n");
>> -    for (idx = DISPLAY_TYPE_NONE; idx < DISPLAY_TYPE__MAX; idx++) {
>> -        if (!dpys[idx]) {
>> -            Error *local_err = NULL;
>> -            int rv = ui_module_load(DisplayType_str(idx), &local_err);
>> -            if (rv < 0) {
>> -                error_report_err(local_err);
>> -            }
>> -        }
>> -        if (dpys[idx]) {
>> -            printf("%s\n",  DisplayType_str(dpys[idx]->type));
>
> Is the "qapi/qapi-commands-ui.h" header still necessary?
>
>> -        }
>> -    }
>> -}
>
> So we go from:
>
> $ ./qemu-system-aarch64 -display help
> Available display backend types:
> none
> gtk
> sdl
> curses
> cocoa
> dbus
>
> to:
>
> $ ./qemu-system-aarch64 -display help
> -display sdl[,gl=on|core|es|off][,grab-mod=<mod>][,show-cursor=on|off]
>            [,window-close=on|off]
> -display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]
>            [,show-tabs=on|off][,show-cursor=on|off][,window-close=on|off]
>            [,show-menubar=on|off]
> -display vnc=<display>[,<optargs>]
> -display curses[,charset=<encoding>]
> -display cocoa[,full-grab=on|off][,swap-opt-cmd=on|off]
> -display dbus[,addr=<dbusaddr>]
>             [,gl=on|core|es|off][,rendernode=<file>]
> -display cocoa[,show-cursor=on|off][,left-command-key=on|off]
> -display none
>                select display backend type
>                The default display is equivalent to
>                "-display gtk"
>
> The latter is indeed more helpful.

It is more helpful but maybe a bit overwhelming. Would it be possible to 
only print the options with -display cocoa,help similar to how -device 
help lists devices and -device sm501,help lists options for one device? 
Adding info about default to -display help is really helpful though (that 
could also be marked with (default) like in -machine help.

I'm not complaining, thanks for taking care of this so quickly but if it's 
not too difficult to add separate -display cocoa,help and not list options 
in -display help maybe that would be better and more consistent with other 
help options.

Regards,
BALATON Zoltan

> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>