[PATCH] gtk: Add show_tabs=on|off command line option.

[PATCH] gtk: Add show_tabs=on|off command line option.

[Felix xq Queißner]

The patch adds "show_tabs" command line option for GTK ui similar to "grab_on_hover". This option allows tabbed view mode to not have to be enabled by hand at each start of the VM.

Signed-off-by: Felix "xq" Queißner <xq@random-projects.net>
---
 qapi/ui.json    | 5 ++++-
 qemu-options.hx | 2 +-
 ui/gtk.c        | 4 ++++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 413371d5e8..049e7957a9 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1195,12 +1195,15 @@
 #               assuming the guest will resize the display to match
 #               the window size then.  Otherwise it defaults to "off".
 #               Since 3.1
+# @show-tabs:   Displays the tab items by default.
+#               Since 7.1
 #
 # Since: 2.12
 ##
 { 'struct'  : 'DisplayGTK',
   'data'    : { '*grab-on-hover' : 'bool',
-                '*zoom-to-fit'   : 'bool'  } }
+                '*zoom-to-fit'   : 'bool',
+                '*show-tabs'     : 'bool'  } }
 
 ##
 # @DisplayEGLHeadless:
diff --git a/qemu-options.hx b/qemu-options.hx
index 377d22fbd8..2b279afff7 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1937,7 +1937,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
     "            [,window-close=on|off]\n"
 #endif
 #if defined(CONFIG_GTK)
-    "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n"
+    "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off][,show-tabs=on|off]\n"
     "            [,show-cursor=on|off][,window-close=on|off]\n"
 #endif
 #if defined(CONFIG_VNC)
diff --git a/ui/gtk.c b/ui/gtk.c
index 2a791dd2aa..1467b8c7d7 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2390,6 +2390,10 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
         opts->u.gtk.grab_on_hover) {
         gtk_menu_item_activate(GTK_MENU_ITEM(s->grab_on_hover_item));
     }
+    if (opts->u.gtk.has_show_tabs &&
+        opts->u.gtk.show_tabs) {
+        gtk_menu_item_activate(GTK_MENU_ITEM(s->show_tabs_item));
+    }
     gd_clipboard_init(s);
 }
 
-- 
2.36.1

Re: [PATCH] gtk: Add show_tabs=on|off command line option.

[Hanna Reitz]

Hi,

(Thanks for the patch!)

On 27.06.22 18:44, Felix xq Queißner wrote:
> The patch adds "show_tabs" command line option for GTK ui similar to "grab_on_hover". This option allows tabbed view mode to not have to be enabled by hand at each start of the VM.

I’m not sure we have a hard rule on it, but I think generally commit 
messages should be wrapped at 72 characters.

> Signed-off-by: Felix "xq" Queißner <xq@random-projects.net>
> ---
>   qapi/ui.json    | 5 ++++-
>   qemu-options.hx | 2 +-
>   ui/gtk.c        | 4 ++++
>   3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 413371d5e8..049e7957a9 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1195,12 +1195,15 @@
>   #               assuming the guest will resize the display to match
>   #               the window size then.  Otherwise it defaults to "off".
>   #               Since 3.1
> +# @show-tabs:   Displays the tab items by default.
> +#               Since 7.1

I don’t really understand what “tab items” is supposed to mean. Perhaps 
“tabs item”?

But a bit more verbosity might be nice, too.  What about “Display the 
tab bar for switching between the various graphical interfaces (e.g. VGA 
and virtual console character devices) by default”?  (Note the 
imperative on “Display”, I think we generally use the imperative to 
document options.)

>   #
>   # Since: 2.12
>   ##
>   { 'struct'  : 'DisplayGTK',
>     'data'    : { '*grab-on-hover' : 'bool',
> -                '*zoom-to-fit'   : 'bool'  } }
> +                '*zoom-to-fit'   : 'bool',
> +                '*show-tabs'     : 'bool'  } }
>   
>   ##
>   # @DisplayEGLHeadless:
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 377d22fbd8..2b279afff7 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1937,7 +1937,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
>       "            [,window-close=on|off]\n"
>   #endif
>   #if defined(CONFIG_GTK)
> -    "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n"
> +    "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off][,show-tabs=on|off]\n"
>       "            [,show-cursor=on|off][,window-close=on|off]\n"

There is some documentation further below that explains the other 
options.  I think this new option should be explained there as well.  
(Probably reusing the documentation from qapi/ui.json.)

>   #endif
>   #if defined(CONFIG_VNC)
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 2a791dd2aa..1467b8c7d7 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -2390,6 +2390,10 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
>           opts->u.gtk.grab_on_hover) {
>           gtk_menu_item_activate(GTK_MENU_ITEM(s->grab_on_hover_item));
>       }
> +    if (opts->u.gtk.has_show_tabs &&
> +        opts->u.gtk.show_tabs) {
> +        gtk_menu_item_activate(GTK_MENU_ITEM(s->show_tabs_item));
> +    }
>       gd_clipboard_init(s);
>   }

Not that I’d know any of this code, but FWIW, makes sense and looks good 
to me.

Thanks!

Hanna

Re: [PATCH] gtk: Add show_tabs=on|off command line option.

[Hanna Reitz]

On 30.06.22 16:09, Hanna Reitz wrote:
> Hi,
>
> (Thanks for the patch!)
>
> On 27.06.22 18:44, Felix xq Queißner wrote:
>> The patch adds "show_tabs" command line option for GTK ui similar to 
>> "grab_on_hover". This option allows tabbed view mode to not have to 
>> be enabled by hand at each start of the VM.
>
> I’m not sure we have a hard rule on it, but I think generally commit 
> messages should be wrapped at 72 characters.
>
>> Signed-off-by: Felix "xq" Queißner <xq@random-projects.net>
>> ---
>>   qapi/ui.json    | 5 ++++-
>>   qemu-options.hx | 2 +-
>>   ui/gtk.c        | 4 ++++
>>   3 files changed, 9 insertions(+), 2 deletions(-)

[...]

>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 377d22fbd8..2b279afff7 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -1937,7 +1937,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
>>       "            [,window-close=on|off]\n"
>>   #endif
>>   #if defined(CONFIG_GTK)
>> -    "-display 
>> gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n"
>> +    "-display 
>> gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off][,show-tabs=on|off]\n"
>>       " [,show-cursor=on|off][,window-close=on|off]\n"

Oops, noticed another thing (a bit late): Considering the options are 
already spit over two lines, it looks to me like this line’s length is 
supposed to be limited.  (My guess is we’re trying to not exceed 80 
characters here in this source file.)  Therefore, this new option should 
probably go on a separate new line.

Hanna

Re: [PATCH] gtk: Add show_tabs=on|off command line option.

[Markus Armbruster]

Hanna Reitz <hreitz@redhat.com> writes:

> Hi,
>
> (Thanks for the patch!)
>
> On 27.06.22 18:44, Felix xq Queißner wrote:
>> The patch adds "show_tabs" command line option for GTK ui similar to "grab_on_hover". This option allows tabbed view mode to not have to be enabled by hand at each start of the VM.
>
> I’m not sure we have a hard rule on it, but I think generally commit messages should be wrapped at 72 characters.

Yes, please.

[...]

RE: [PATCH] gtk: Add show_tabs=on|off command line option.

[Zhang, Chen]

> -----Original Message-----
> From: Qemu-devel <qemu-devel-
> bounces+chen.zhang=intel.com@nongnu.org> On Behalf Of Felix xq
> Queißner
> Sent: Tuesday, June 28, 2022 12:44 AM
> To: qemu-devel@nongnu.org
> Cc: kraxel@redhat.com; thuth@redhat.com; Felix "xq" Queißner
> <xq@random-projects.net>
> Subject: [PATCH] gtk: Add show_tabs=on|off command line option.
> 
> The patch adds "show_tabs" command line option for GTK ui similar to
> "grab_on_hover". This option allows tabbed view mode to not have to be
> enabled by hand at each start of the VM.
> 
> Signed-off-by: Felix "xq" Queißner <xq@random-projects.net>

Thanks your patch, but please use your real name to sign a patch.
For the details:
docs/devel/submitting-a-patch.rst

Thanks
Chen


> ---
>  qapi/ui.json    | 5 ++++-
>  qemu-options.hx | 2 +-
>  ui/gtk.c        | 4 ++++
>  3 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi/ui.json b/qapi/ui.json index 413371d5e8..049e7957a9 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1195,12 +1195,15 @@
>  #               assuming the guest will resize the display to match
>  #               the window size then.  Otherwise it defaults to "off".
>  #               Since 3.1
> +# @show-tabs:   Displays the tab items by default.
> +#               Since 7.1
>  #
>  # Since: 2.12
>  ##
>  { 'struct'  : 'DisplayGTK',
>    'data'    : { '*grab-on-hover' : 'bool',
> -                '*zoom-to-fit'   : 'bool'  } }
> +                '*zoom-to-fit'   : 'bool',
> +                '*show-tabs'     : 'bool'  } }
> 
>  ##
>  # @DisplayEGLHeadless:
> diff --git a/qemu-options.hx b/qemu-options.hx index
> 377d22fbd8..2b279afff7 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1937,7 +1937,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
>      "            [,window-close=on|off]\n"
>  #endif
>  #if defined(CONFIG_GTK)
> -    "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-hover=on|off]\n"
> +    "-display gtk[,full-screen=on|off][,gl=on|off][,grab-on-
> hover=on|off][,show-tabs=on|off]\n"
>      "            [,show-cursor=on|off][,window-close=on|off]\n"
>  #endif
>  #if defined(CONFIG_VNC)
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 2a791dd2aa..1467b8c7d7 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -2390,6 +2390,10 @@ static void gtk_display_init(DisplayState *ds,
> DisplayOptions *opts)
>          opts->u.gtk.grab_on_hover) {
>          gtk_menu_item_activate(GTK_MENU_ITEM(s->grab_on_hover_item));
>      }
> +    if (opts->u.gtk.has_show_tabs &&
> +        opts->u.gtk.show_tabs) {
> +        gtk_menu_item_activate(GTK_MENU_ITEM(s->show_tabs_item));
> +    }
>      gd_clipboard_init(s);
>  }
> 
> --
> 2.36.1
> 

Re: [PATCH] gtk: Add show_tabs=on|off command line option.

[Gerd Hoffmann]

  Hi,

> But a bit more verbosity might be nice, too.  What about “Display the tab
> bar for switching between the various graphical interfaces (e.g. VGA and
> virtual console character devices) by default”?  (Note the imperative on
> “Display”, I think we generally use the imperative to document options.)

And 'tab bar' is more clear too I think.

take care,
  Gerd

Re: [PATCH] gtk: Add show_tabs=on|off command line option.

[kraxel]

On Fri, Jul 01, 2022 at 09:14:02AM +0000, Zhang, Chen wrote:
> 
> 
> > Signed-off-by: Felix "xq" Queißner <xq@random-projects.net>
> 
> Thanks your patch, but please use your real name to sign a patch.
> For the details:
> docs/devel/submitting-a-patch.rst

Hmm?  Felix Queißner looks like a real name to me ...

take care,
  Gerd

Re: [PATCH] gtk: Add show_tabs=on|off command line option.

[Felix Queißner]

Heya!

On 27.06.22 18:44, Felix xq Queißner wrote:
> The patch adds "show_tabs" command line option for GTK ui similar to "grab_on_hover". This option allows tabbed view mode to not have to be enabled by hand at each start of the VM.

On 30.06.22 16:09, Hanna Reitz wrote:
 > [snip]

On 30.06.22 16:53, Markus Armbruster wrote:
 > [snip]

On 01.07.22 12:00, Gerd Hoffmann wrote:
 > [snip]

I have addressed the things mentioned:
- limiting line length to 80 in ui.json, qemu-options.hx
- limiting line length in commit to 72
- improved description of the option as Hanna suggested

On 01.07.22 11:14, Zhang, Chen wrote:
 >> Signed-off-by: Felix "xq" Queißner <xq@random-projects.net>
 >
 > Thanks your patch, but please use your real name to sign a patch.

Not sure what to do about this one. Felix Queißer *is* my real name, so 
the only thing i could do is to remove the "xq"?

Should i submit my changes as a response to this or create a new mail 
thread with a new patch?

Regards
- Felix

Re: [PATCH] gtk: Add show_tabs=on|off command line option.

[Hanna Reitz]

On 12.07.22 09:35, Felix Queißner wrote:
> Heya!
>
> On 27.06.22 18:44, Felix xq Queißner wrote:
>> The patch adds "show_tabs" command line option for GTK ui similar to 
>> "grab_on_hover". This option allows tabbed view mode to not have to 
>> be enabled by hand at each start of the VM.
>
> On 30.06.22 16:09, Hanna Reitz wrote:
> > [snip]
>
> On 30.06.22 16:53, Markus Armbruster wrote:
> > [snip]
>
> On 01.07.22 12:00, Gerd Hoffmann wrote:
> > [snip]
>
> I have addressed the things mentioned:
> - limiting line length to 80 in ui.json, qemu-options.hx
> - limiting line length in commit to 72
> - improved description of the option as Hanna suggested
>
> On 01.07.22 11:14, Zhang, Chen wrote:
> >> Signed-off-by: Felix "xq" Queißner <xq@random-projects.net>
> >
> > Thanks your patch, but please use your real name to sign a patch.
>
> Not sure what to do about this one. Felix Queißer *is* my real name, 
> so the only thing i could do is to remove the "xq"?

I feel like you can leave it that way because given the quotation marks 
it seems obvious what you real name is.

(FWIW I can see precedent in commit 
f5507e0448bd34473af72509297617a783049024.)

> Should i submit my changes as a response to this or create a new mail 
> thread with a new patch?

Please submit them as a new thread with a “[PATCH v2]” tag (`git 
format-patch -v2` will create that tag).

Hanna