Hi,
Some commit log suggestions but feel free to ignore it.
On Tue, Jul 31, 2018 at 03:24:43PM +0200, marcandre.lureau@xxxxxxxxxx wrote:
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
>
> There is a hack to maintain the the toggle state to a desired
Removing the double 'the'
> state within the "toggled" handler. (since there is no way to
I would move this parenthesis to the end of commit log as a 'Note
that the hack was needed since there is no way to ...'
> hook a signal handler on "clicked" before "toggled" is emitted
> and handled by Gtk, to avoid the recursion)
>
> However it is only necessary for the ask-quit case. In this
> case, we want to maintain the item active, which is simpler to
> handle than the general case. Simplify the code by folding
> virt_viewer_app_window_set_visible() and removing the static
> "reentering" hack, only maintaining "active" on the last item.
Out of curiosity, why we quit when disabling the last display
instead of not allowing the last display to be toggled?
I would say that might happen by mistake (disabling last display)
instead of intentional desire to quit the app.
> Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
Patch looks fine,
Acked-by: Victor Toso <victortoso@xxxxxxxxxx>
> ---
> src/virt-viewer-app.c | 43 +++++++++++--------------------------------
> src/virt-viewer-app.h | 2 +-
> 2 files changed, 12 insertions(+), 33 deletions(-)
>
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 2a88882..62dbf19 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -502,29 +502,6 @@ virt_viewer_app_get_n_windows_visible(VirtViewerApp *self)
> return n;
> }
>
> -gboolean
> -virt_viewer_app_window_set_visible(VirtViewerApp *self,
> - VirtViewerWindow *window,
> - gboolean visible)
> -{
> - g_return_val_if_fail(VIRT_VIEWER_IS_APP(self), FALSE);
> - g_return_val_if_fail(VIRT_VIEWER_IS_WINDOW(window), FALSE);
> -
> - if (visible) {
> - virt_viewer_window_show(window);
> - return TRUE;
> - } else {
> - if (virt_viewer_app_get_n_windows_visible(self) > 1) {
> - virt_viewer_window_hide(window);
> - return FALSE;
> - }
> -
> - virt_viewer_app_maybe_quit(self, window);
> - }
> -
> - return FALSE;
> -}
> -
> static void hide_one_window(gpointer value,
> gpointer user_data G_GNUC_UNUSED)
> {
> @@ -2232,19 +2209,21 @@ menu_display_visible_toggled_cb(GtkCheckMenuItem *checkmenuitem,
> {
> VirtViewerApp *self = virt_viewer_session_get_app(virt_viewer_display_get_session(display));
> gboolean visible = gtk_check_menu_item_get_active(checkmenuitem);
> - static gboolean reentering = FALSE;
> VirtViewerWindow *vwin;
>
> - if (reentering) /* do not reenter if I switch you back */
> - return;
> -
> - reentering = TRUE;
> -
> vwin = ensure_window_for_display(self, display);
> - visible = virt_viewer_app_window_set_visible(self, vwin, visible);
>
> - gtk_check_menu_item_set_active(checkmenuitem, /* will be toggled again */ !visible);
> - reentering = FALSE;
> + if (visible) {
> + virt_viewer_window_show(vwin);
> + } else {
> + if (virt_viewer_app_get_n_windows_visible(self) > 1) {
> + virt_viewer_window_hide(vwin);
> + } else {
> + virt_viewer_app_maybe_quit(self, vwin);
> + /* the last item remains active, doesn't matter if we quit */
> + gtk_check_menu_item_set_active(checkmenuitem, TRUE);
> + }
> + }
>
> virt_viewer_session_update_displays_geometry(virt_viewer_display_get_session(display));
> }
> diff --git a/src/virt-viewer-app.h b/src/virt-viewer-app.h
> index 16b1c8c..0321229 100644
> --- a/src/virt-viewer-app.h
> +++ b/src/virt-viewer-app.h
> @@ -84,7 +84,7 @@ void virt_viewer_app_set_connect_info(VirtViewerApp *self,
> const gchar *user,
> gint port,
> const gchar *guri);
> -gboolean virt_viewer_app_window_set_visible(VirtViewerApp *self, VirtViewerWindow *window, gboolean visible);
> +
> void virt_viewer_app_show_status(VirtViewerApp *self, const gchar *fmt, ...);
> void virt_viewer_app_show_display(VirtViewerApp *self);
> GList* virt_viewer_app_get_windows(VirtViewerApp *self);
> --
> 2.18.0.321.gffc6fa0e39
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
