Re: [Qemu-devel] [PATCH v2] gtk: Rename File to Machine menu and add stop, reset and power-down items

[Anthony Liguori]

Jan Kiszka <address@hidden> writes:

> On 2013-02-22 18:16, Anthony Liguori wrote:
>> Jan Kiszka <address@hidden> writes:
>> 
>>> This adds basic guest control commands to the "Machine" menu - a nice
>>> added-value for the GTK UI. We pick up the stock stop accelerators (if
>>> any) but not its image as we add a check item.
>>>
>>> Signed-off-by: Jan Kiszka <address@hidden>
>>> ---
>>>
>>> Changes in v2:
>>>  - File -> Machine menu
>>>  - stock accels for stop
>>>
>>>  ui/gtk.c |   82 
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++--------
>>>  1 files changed, 71 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/ui/gtk.c b/ui/gtk.c
>>> index bc4ec67..5e8a158 100644
>>> --- a/ui/gtk.c
>>> +++ b/ui/gtk.c
>>> @@ -80,8 +80,11 @@ typedef struct GtkDisplayState
>>>  
>>>      GtkAccelGroup *accel_group;
>>>  
>>> -    GtkWidget *file_menu_item;
>>> -    GtkWidget *file_menu;
>>> +    GtkWidget *machine_menu_item;
>>> +    GtkWidget *machine_menu;
>>> +    GtkWidget *stop_item;
>>> +    GtkWidget *reset_item;
>>> +    GtkWidget *powerdown_item;
>>>      GtkWidget *quit_item;
>>>  
>>>      GtkWidget *view_menu_item;
>>> @@ -117,6 +120,8 @@ typedef struct GtkDisplayState
>>>      GdkCursor *null_cursor;
>>>      Notifier mouse_mode_notifier;
>>>      gboolean free_scale;
>>> +
>>> +    bool external_stop_update;
>>>  } GtkDisplayState;
>>>  
>>>  static GtkDisplayState *global_state;
>>> @@ -160,14 +165,19 @@ static void gd_update_caption(GtkDisplayState *s)
>>>      const char *status = "";
>>>      gchar *title;
>>>      const char *grab = "";
>>> +    bool is_stopped = !runstate_is_running();
>>>  
>>>      if (gd_is_grab_active(s)) {
>>>          grab = " - Press Ctrl+Alt+G to release grab";
>>>      }
>>>  
>>> -    if (!runstate_is_running()) {
>>> +    if (is_stopped) {
>>>          status = " [Stopped]";
>>>      }
>>> +    s->external_stop_update = true;
>>> +    gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->stop_item),
>>> +                                   is_stopped);
>>> +    s->external_stop_update = false;
>>>  
>>>      if (qemu_name) {
>>>          title = g_strdup_printf("QEMU (%s)%s%s", qemu_name, status, grab);
>>> @@ -584,6 +594,30 @@ static gboolean gd_key_event(GtkWidget *widget, 
>>> GdkEventKey *key, void *opaque)
>>>  
>>>  /** Window Menu Actions **/
>>>  
>>> +static void gd_menu_stop(GtkMenuItem *item, void *opaque)
>>> +{
>>> +    GtkDisplayState *s = opaque;
>>> +
>>> +    if (s->external_stop_update) {
>>> +        return;
>>> +    }
>>> +    if (runstate_is_running()) {
>>> +        qmp_stop(NULL);
>>> +    } else {
>>> +        qmp_cont(NULL);
>>> +    }
>>> +}
>>> +
>>> +static void gd_menu_reset(GtkMenuItem *item, void *opaque)
>>> +{
>>> +    qmp_system_reset(NULL);
>>> +}
>>> +
>>> +static void gd_menu_powerdown(GtkMenuItem *item, void *opaque)
>>> +{
>>> +    qmp_system_powerdown(NULL);
>>> +}
>>> +
>>>  static void gd_menu_quit(GtkMenuItem *item, void *opaque)
>>>  {
>>>      qmp_quit(NULL);
>>> @@ -952,6 +986,12 @@ static void gd_connect_signals(GtkDisplayState *s)
>>>      g_signal_connect(s->drawing_area, "key-release-event",
>>>                       G_CALLBACK(gd_key_event), s);
>>>  
>>> +    g_signal_connect(s->stop_item, "activate",
>>> +                     G_CALLBACK(gd_menu_stop), s);
>>> +    g_signal_connect(s->reset_item, "activate",
>>> +                     G_CALLBACK(gd_menu_reset), s);
>>> +    g_signal_connect(s->powerdown_item, "activate",
>>> +                     G_CALLBACK(gd_menu_powerdown), s);
>>>      g_signal_connect(s->quit_item, "activate",
>>>                       G_CALLBACK(gd_menu_quit), s);
>>>      g_signal_connect(s->full_screen_item, "activate",
>>> @@ -985,15 +1025,35 @@ static void gd_create_menus(GtkDisplayState *s)
>>>      int i;
>>>  
>>>      accel_group = gtk_accel_group_new();
>>> -    s->file_menu = gtk_menu_new();
>>> -    gtk_menu_set_accel_group(GTK_MENU(s->file_menu), accel_group);
>>> -    s->file_menu_item = gtk_menu_item_new_with_mnemonic(_("_File"));
>>> +    s->machine_menu = gtk_menu_new();
>>> +    gtk_menu_set_accel_group(GTK_MENU(s->machine_menu), accel_group);
>>> +    s->machine_menu_item = gtk_menu_item_new_with_mnemonic(_("_Machine"));
>>> +
>>> +    s->stop_item = gtk_check_menu_item_new_with_mnemonic(_("_Stop"));
>>> +    gtk_stock_lookup(GTK_STOCK_STOP, &item);
>>> +    gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->stop_item),
>>> +                                 "<QEMU>/Machine/Stop");
>>> +    gtk_accel_map_add_entry("<QEMU>/Machine/Stop", item.keyval, 
>>> item.modifier);
>>> +    gtk_menu_append(GTK_MENU(s->machine_menu), s->stop_item);
>>> +
>>> +    separator = gtk_separator_menu_item_new();
>>> +    gtk_menu_append(GTK_MENU(s->machine_menu), separator);
>>> +
>>> +    s->reset_item = gtk_image_menu_item_new_with_mnemonic(_("_Reset"));
>>> +    gtk_menu_append(GTK_MENU(s->machine_menu), s->reset_item);
>>> +
>>> +    s->powerdown_item =
>>> gtk_image_menu_item_new_with_mnemonic(_("_Power-down"));
>> 
>> 'Power Down' please.  I can make this change if you don't object.
>
> You also mentioned picking up the label string for Stop. Can do both and
> send v3.

How do you feel about changing Stop to Pause?  I think Pause makes more
sense to me.  I think it will translate better too.

Stop is a synonym with terminate.  I don't think it's a given that you
can always restart once you stop.

However, pause and unpause are much more clearly related.

Please do send a v3.

Regards,

Anthony Liguori

>
> Jan
>
> -- 
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux