Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Robert Hu]

On Mon, 2016-06-06 at 09:28 +0200, Markus Armbruster wrote:
> Robert Hu <address@hidden> writes:
> 
> > On Tue, 2016-05-31 at 13:17 +0200, Markus Armbruster wrote:
> >> Robert Hu <address@hidden> writes:
> >> 
> >> > On Tue, 2016-05-31 at 09:51 +0200, Markus Armbruster wrote:
[trim...]
> > I don't see a './configure' option related to this '-vnc to' param. Is
> > there any?
> > '--help', you mean 'qemu-system_x86-64 --help'? or './configure --help'?
[seems repeated contents, trim...]
> > I don't see a './configure' option related to this '-vnc to' param. Is
> > there any?
> > '--help', you mean 'qemu-system_x86-64 --help'? or './configure --help'?
> 
> The former.
> 
> The modern way to select a display is -display.  The older -nographic,
> -curses, -sdl are retained for backward compatibility.
> 
> Relevant parts of -help:
> 
>     Display options:
>     -display sdl[,frame=on|off][,alt_grab=on|off][,ctrl_grab=on|off]
>                 [,window_close=on|off]|curses|none|
>                 gtk[,grab_on_hover=on|off]|
>                 vnc=<display>[,<optargs>]
>                     select display type
>     -nographic      disable graphical output and redirect serial I/Os to 
> console
>     -curses         use a curses/ncurses interface instead of SDL
>     [...]
>     -sdl            enable SDL
>     [...]
>     -vnc display    start a VNC server on display
> 
> Issues:
> 
> * Help for -display is broken: the mutually exclusive option arguments
>   are concatenated.  -display curses and -display none are undocumented.
>   It should look more like this:
> 
>     -display sdl[,frame=on|off][,alt_grab=on|off][,ctrl_grab=on|off]
>                 [,window_close=on|off]|curses|none|
>     -display gtk[,grab_on_hover=on|off]|
>     -display vnc=<display>[,<optargs>]
>     -display curses
>     -display none
>                     select display type
> 
> * -display sdl,gl=on|off and -display gtk,gl=on|off are undocumented
>    (missed in commit 0b71a5d5c and 97edf3b).
> 
> * There is no help on the <display> in -display vnc=<display>.
> 
> * There is no help on the default.  main() picks the default depending
>   on configure options:
> 
>     #if defined(CONFIG_GTK)
>             display_type = DT_GTK;
>     #elif defined(CONFIG_SDL)
>             display_type = DT_SDL;
>     #elif defined(CONFIG_COCOA)
>             display_type = DT_COCOA;
>     #elif defined(CONFIG_VNC)
>             vnc_parse("localhost:0,to=99,id=default", &error_abort);
>             show_vnc_port = 1;
>     #else
>             display_type = DT_NONE;
>     #endif
> 
>   Help should show the default this binary will pick.  This is what I
>   meant by "Ideally, --help output would show the defaults for this
>   build's configuration."
> 
> * Help should explain syntacic sugar:
>   -curses is sugar for -display curses
>   -sdl is sugar for -display sdl
>   -vnc display is sugar for -display vnc=display
> 
>   -nographic is also sugar, but too complicated to explain; I'd leave it
>   as is.
> 
> Non-issue
> 
> * Help shows options even when they're not compiled in.  That's okay,
>   because trying to use them fails with an "FOO support is disabled"
>   error message.
> 
> >> If we decide users need more information than the current "VNC server
> >> running on" line, perhaps it should be included right in that line.
> 
> This would complement, but not replace better -help ouput.
> 
> If you would like to work on these issues, let us know.

OK, if not in a hurry and assuming this is not a huge amount of work.
I also need to look into the build arch so that completely understand
your 'the default this binary will pick', till now I don't.

Another concern is that I'm not a native English speaker, so those
description words may not be that apt and concise.

Meanwhile, this is another work extended from the original patch. How
about accept the patch 1 first? as you and Paolo both think it is OK.
Ought I rework a version 2 of single patch 1? or not necessary?