8:24 a.m.
On 09/08/22 09:57, Richard W.M. Jones wrote:
On Mon, Sep 05, 2022 at 01:25:28PM +0200, Laszlo Ersek wrote:
> Each call to table_attach() in fact open-codes (row, row + 1), for setting
> the top and bottom grid lines of the widget being added to the table.
> What's more, within a given table, we add widgets in a left-to-right
> first, top-down second fashion, so "row" only ever increases within a
> particular table.
>
> Modify the table_attach() macro to calculate "bottom" automatically, plus
> replace the open-coded "row" constants with actual (incremented)
"row"
> variables.
>
> (This patch is easiest to review with "git show --word-diff=color".)
>
> Don't do the same simplification for colums, as we're going to introduce a
columns
Thanks, will fix!
Laszlo
> multi-column widget next.
>
> Note that one definition of table_attach() now evaluates "top" twice.
> Preventing that would be a mess: we could be tempted to introduce a do {
> ... } while (0) block with a block-scope auto-storage variable. However,
> it could trigger gcc warnings (about shadowing other variables), in which
> case we'd end up copying NBDKIT_UNIQUE_NAME() from nbdkit, which is
> totally overkill. So just evaluate "top" twice; it's not a complex or
very
> widely used macro. (BTW, the previous replacement text of the *other*
> definition of table_attach() used to evaluate "top" twice as well.)
>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1590721
> Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
> ---
> gui-gtk3-compat.h | 8 +--
> gui.c | 69 ++++++++++++--------
> 2 files changed, 46 insertions(+), 31 deletions(-)
>
> diff --git a/gui-gtk3-compat.h b/gui-gtk3-compat.h
> index 5388c3a3530c..e26506469eea 100644
> --- a/gui-gtk3-compat.h
> +++ b/gui-gtk3-compat.h
> @@ -63,7 +63,7 @@ typedef enum
> */
> #define table_new(grid, rows, columns) \
> (grid) = gtk_grid_new ()
> -#define table_attach(grid, child, left, right, top, bottom, xoptions, yoptions,
xpadding, ypadding) \
> +#define table_attach(grid, child, left, right, top, xoptions, yoptions, xpadding,
ypadding) \
> do { \
> if (((xoptions) & GTK_EXPAND) != 0) \
> gtk_widget_set_hexpand ((child), TRUE); \
> @@ -75,14 +75,14 @@ typedef enum
> gtk_widget_set_valign ((child), GTK_ALIGN_FILL); \
> set_padding ((child), (xpadding), (ypadding)); \
> gtk_grid_attach (GTK_GRID (grid), (child), \
> - (left), (top), (right)-(left), (bottom)-(top)); \
> + (left), (top), (right)-(left), 1); \
> } while (0)
> #else
> #define table_new(table, rows, columns) \
> (table) = gtk_table_new ((rows), (columns), FALSE)
> -#define table_attach(table, child, left, right,top, bottom, xoptions, yoptions,
xpadding, ypadding) \
> +#define table_attach(table, child, left, right,top, xoptions, yoptions, xpadding,
ypadding) \
> gtk_table_attach (GTK_TABLE (table), (child), \
> - (left), (right), (top), (bottom), \
> + (left), (right), (top), (top + 1), \
> (xoptions), (yoptions), (xpadding), (ypadding))
> #endif
>
> diff --git a/gui.c b/gui.c
> index b4a9fed18410..eeb15a096351 100644
> --- a/gui.c
> +++ b/gui.c
> @@ -210,6 +210,7 @@ create_connection_dialog (struct config *config)
> GtkWidget *configure_network;
> GtkWidget *xterm;
> char port_str[64];
> + int row;
>
> conn_dlg = gtk_dialog_new ();
> gtk_window_set_title (GTK_WINDOW (conn_dlg), g_get_prgname ());
> @@ -221,9 +222,10 @@ create_connection_dialog (struct config *config)
> set_padding (intro, 10, 10);
>
> table_new (table, 5, 2);
> + row = 0;
> server_label = gtk_label_new_with_mnemonic (_("Conversion _server:"));
> table_attach (table, server_label,
> - 0, 1, 0, 1, GTK_FILL, GTK_FILL, 4, 4);
> + 0, 1, row, GTK_FILL, GTK_FILL, 4, 4);
> set_alignment (server_label, 1., 0.5);
>
> hbox_new (server_hbox, FALSE, 4);
> @@ -240,11 +242,12 @@ create_connection_dialog (struct config *config)
> gtk_box_pack_start (GTK_BOX (server_hbox), port_colon_label, FALSE, FALSE, 0);
> gtk_box_pack_start (GTK_BOX (server_hbox), port_entry, FALSE, FALSE, 0);
> table_attach (table, server_hbox,
> - 1, 2, 0, 1, GTK_EXPAND|GTK_FILL, GTK_FILL, 4, 4);
> + 1, 2, row, GTK_EXPAND|GTK_FILL, GTK_FILL, 4, 4);
>
> + row++;
> username_label = gtk_label_new_with_mnemonic (_("_User name:"));
> table_attach (table, username_label,
> - 0, 1, 1, 2, GTK_FILL, GTK_FILL, 4, 4);
> + 0, 1, row, GTK_FILL, GTK_FILL, 4, 4);
> set_alignment (username_label, 1., 0.5);
> username_entry = gtk_entry_new ();
> gtk_label_set_mnemonic_widget (GTK_LABEL (username_label), username_entry);
> @@ -253,11 +256,12 @@ create_connection_dialog (struct config *config)
> else
> gtk_entry_set_text (GTK_ENTRY (username_entry), "root");
> table_attach (table, username_entry,
> - 1, 2, 1, 2, GTK_EXPAND|GTK_FILL, GTK_FILL, 4, 4);
> + 1, 2, row, GTK_EXPAND|GTK_FILL, GTK_FILL, 4, 4);
>
> + row++;
> password_label = gtk_label_new_with_mnemonic (_("_Password:"));
> table_attach (table, password_label,
> - 0, 1, 2, 3, GTK_FILL, GTK_FILL, 4, 4);
> + 0, 1, row, GTK_FILL, GTK_FILL, 4, 4);
> set_alignment (password_label, 1., 0.5);
> password_entry = gtk_entry_new ();
> gtk_label_set_mnemonic_widget (GTK_LABEL (password_label), password_entry);
> @@ -269,25 +273,27 @@ create_connection_dialog (struct config *config)
> if (config->auth.password != NULL)
> gtk_entry_set_text (GTK_ENTRY (password_entry), config->auth.password);
> table_attach (table, password_entry,
> - 1, 2, 2, 3, GTK_EXPAND|GTK_FILL, GTK_FILL, 4, 4);
> + 1, 2, row, GTK_EXPAND|GTK_FILL, GTK_FILL, 4, 4);
>
> + row++;
> identity_label = gtk_label_new_with_mnemonic (_("SSH _Identity URL:"));
> table_attach (table, identity_label,
> - 0, 1, 3, 4, GTK_FILL, GTK_FILL, 4, 4);
> + 0, 1, row, GTK_FILL, GTK_FILL, 4, 4);
> set_alignment (identity_label, 1., 0.5);
> identity_entry = gtk_entry_new ();
> gtk_label_set_mnemonic_widget (GTK_LABEL (identity_label), identity_entry);
> if (config->auth.identity.url != NULL)
> gtk_entry_set_text (GTK_ENTRY (identity_entry), config->auth.identity.url);
> table_attach (table, identity_entry,
> - 1, 2, 3, 4, GTK_EXPAND|GTK_FILL, GTK_FILL, 4, 4);
> + 1, 2, row, GTK_EXPAND|GTK_FILL, GTK_FILL, 4, 4);
>
> + row++;
> sudo_button =
> gtk_check_button_new_with_mnemonic (_("Use su_do when running
virt-v2v"));
> gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (sudo_button),
> config->auth.sudo);
> table_attach (table, sudo_button,
> - 1, 2, 4, 5, GTK_FILL, GTK_FILL, 4, 4);
> + 1, 2, row, GTK_FILL, GTK_FILL, 4, 4);
>
> hbox_new (test_hbox, FALSE, 0);
> test = gtk_button_new_with_mnemonic (_("_Test connection"));
> @@ -729,6 +735,7 @@ create_conversion_dialog (struct config *config)
> GtkWidget *interfaces_frame, *interfaces_sw;
> char vcpus_str[64];
> char memory_str[64];
> + int row;
>
> conv_dlg = gtk_dialog_new ();
> gtk_window_set_title (GTK_WINDOW (conv_dlg), g_get_prgname ());
> @@ -750,35 +757,38 @@ create_conversion_dialog (struct config *config)
> vbox_new (target_vbox, FALSE, 1);
>
> table_new (target_tbl, 3, 3);
> + row = 0;
> guestname_label = gtk_label_new_with_mnemonic (_("_Name:"));
> table_attach (target_tbl, guestname_label,
> - 0, 1, 0, 1, GTK_FILL, GTK_FILL, 1, 1);
> + 0, 1, row, GTK_FILL, GTK_FILL, 1, 1);
> set_alignment (guestname_label, 1., 0.5);
> guestname_entry = gtk_entry_new ();
> gtk_label_set_mnemonic_widget (GTK_LABEL (guestname_label), guestname_entry);
> if (config->guestname != NULL)
> gtk_entry_set_text (GTK_ENTRY (guestname_entry), config->guestname);
> table_attach (target_tbl, guestname_entry,
> - 1, 2, 0, 1, GTK_FILL, GTK_FILL, 1, 1);
> + 1, 2, row, GTK_FILL, GTK_FILL, 1, 1);
>
> + row++;
> vcpus_label = gtk_label_new_with_mnemonic (_("# _vCPUs:"));
> table_attach (target_tbl, vcpus_label,
> - 0, 1, 1, 2, GTK_FILL, GTK_FILL, 1, 1);
> + 0, 1, row, GTK_FILL, GTK_FILL, 1, 1);
> set_alignment (vcpus_label, 1., 0.5);
> vcpus_entry = gtk_entry_new ();
> gtk_label_set_mnemonic_widget (GTK_LABEL (vcpus_label), vcpus_entry);
> snprintf (vcpus_str, sizeof vcpus_str, "%d", config->vcpu.cores);
> gtk_entry_set_text (GTK_ENTRY (vcpus_entry), vcpus_str);
> table_attach (target_tbl, vcpus_entry,
> - 1, 2, 1, 2, GTK_FILL, GTK_FILL, 1, 1);
> + 1, 2, row, GTK_FILL, GTK_FILL, 1, 1);
> vcpus_warning = gtk_image_new_from_stock (GTK_STOCK_DIALOG_WARNING,
> GTK_ICON_SIZE_BUTTON);
> table_attach (target_tbl, vcpus_warning,
> - 2, 3, 1, 2, 0, 0, 1, 1);
> + 2, 3, row, 0, 0, 1, 1);
>
> + row++;
> memory_label = gtk_label_new_with_mnemonic (_("_Memory (MB):"));
> table_attach (target_tbl, memory_label,
> - 0, 1, 2, 3, GTK_FILL, GTK_FILL, 1, 1);
> + 0, 1, row, GTK_FILL, GTK_FILL, 1, 1);
> set_alignment (memory_label, 1., 0.5);
> memory_entry = gtk_entry_new ();
> gtk_label_set_mnemonic_widget (GTK_LABEL (memory_label), memory_entry);
> @@ -786,11 +796,11 @@ create_conversion_dialog (struct config *config)
> config->memory / 1024 / 1024);
> gtk_entry_set_text (GTK_ENTRY (memory_entry), memory_str);
> table_attach (target_tbl, memory_entry,
> - 1, 2, 2, 3, GTK_FILL, GTK_FILL, 1, 1);
> + 1, 2, row, GTK_FILL, GTK_FILL, 1, 1);
> memory_warning = gtk_image_new_from_stock (GTK_STOCK_DIALOG_WARNING,
> GTK_ICON_SIZE_BUTTON);
> table_attach (target_tbl, memory_warning,
> - 2, 3, 2, 3, 0, 0, 1, 1);
> + 2, 3, row, 0, 0, 1, 1);
>
> gtk_box_pack_start (GTK_BOX (target_vbox), target_tbl, TRUE, TRUE, 0);
>
> @@ -809,20 +819,22 @@ create_conversion_dialog (struct config *config)
> vbox_new (output_vbox, FALSE, 1);
>
> table_new (output_tbl, 5, 2);
> + row = 0;
> o_label = gtk_label_new_with_mnemonic (_("Output _to (-o):"));
> table_attach (output_tbl, o_label,
> - 0, 1, 0, 1, GTK_FILL, GTK_FILL, 1, 1);
> + 0, 1, row, GTK_FILL, GTK_FILL, 1, 1);
> set_alignment (o_label, 1., 0.5);
> o_combo = gtk_combo_box_text_new ();
> gtk_label_set_mnemonic_widget (GTK_LABEL (o_label), o_combo);
> gtk_widget_set_tooltip_markup (o_combo, _("<b>libvirt</b> means
send the converted guest to libvirt-managed KVM on the conversion server.
<b>local</b> means put it in a directory on the conversion server.
<b>rhv</b> means write it to RHV-M/oVirt. <b>glance</b> means
write it to OpenStack Glance. See the virt-v2v(1) manual page for more information about
output options."));
> repopulate_output_combo (config);
> table_attach (output_tbl, o_combo,
> - 1, 2, 0, 1, GTK_FILL, GTK_FILL, 1, 1);
> + 1, 2, row, GTK_FILL, GTK_FILL, 1, 1);
>
> + row++;
> oc_label = gtk_label_new_with_mnemonic (_("_Output conn. (-oc):"));
> table_attach (output_tbl, oc_label,
> - 0, 1, 1, 2, GTK_FILL, GTK_FILL, 1, 1);
> + 0, 1, row, GTK_FILL, GTK_FILL, 1, 1);
> set_alignment (oc_label, 1., 0.5);
> oc_entry = gtk_entry_new ();
> gtk_label_set_mnemonic_widget (GTK_LABEL (oc_label), oc_entry);
> @@ -830,11 +842,12 @@ create_conversion_dialog (struct config *config)
> if (config->output.connection != NULL)
> gtk_entry_set_text (GTK_ENTRY (oc_entry), config->output.connection);
> table_attach (output_tbl, oc_entry,
> - 1, 2, 1, 2, GTK_FILL, GTK_FILL, 1, 1);
> + 1, 2, row, GTK_FILL, GTK_FILL, 1, 1);
>
> + row++;
> os_label = gtk_label_new_with_mnemonic (_("Output _storage (-os):"));
> table_attach (output_tbl, os_label,
> - 0, 1, 2, 3, GTK_FILL, GTK_FILL, 1, 1);
> + 0, 1, row, GTK_FILL, GTK_FILL, 1, 1);
> set_alignment (os_label, 1., 0.5);
> os_entry = gtk_entry_new ();
> gtk_label_set_mnemonic_widget (GTK_LABEL (os_label), os_entry);
> @@ -842,11 +855,12 @@ create_conversion_dialog (struct config *config)
> if (config->output.storage != NULL)
> gtk_entry_set_text (GTK_ENTRY (os_entry), config->output.storage);
> table_attach (output_tbl, os_entry,
> - 1, 2, 2, 3, GTK_FILL, GTK_FILL, 1, 1);
> + 1, 2, row, GTK_FILL, GTK_FILL, 1, 1);
>
> + row++;
> of_label = gtk_label_new_with_mnemonic (_("Output _format (-of):"));
> table_attach (output_tbl, of_label,
> - 0, 1, 3, 4, GTK_FILL, GTK_FILL, 1, 1);
> + 0, 1, row, GTK_FILL, GTK_FILL, 1, 1);
> set_alignment (of_label, 1., 0.5);
> of_entry = gtk_entry_new ();
> gtk_label_set_mnemonic_widget (GTK_LABEL (of_label), of_entry);
> @@ -854,11 +868,12 @@ create_conversion_dialog (struct config *config)
> if (config->output.format != NULL)
> gtk_entry_set_text (GTK_ENTRY (of_entry), config->output.format);
> table_attach (output_tbl, of_entry,
> - 1, 2, 3, 4, GTK_FILL, GTK_FILL, 1, 1);
> + 1, 2, row, GTK_FILL, GTK_FILL, 1, 1);
>
> + row++;
> oa_label = gtk_label_new_with_mnemonic (_("Output _allocation
(-oa):"));
> table_attach (output_tbl, oa_label,
> - 0, 1, 4, 5, GTK_FILL, GTK_FILL, 1, 1);
> + 0, 1, row, GTK_FILL, GTK_FILL, 1, 1);
> set_alignment (oa_label, 1., 0.5);
> oa_combo = gtk_combo_box_text_new ();
> gtk_label_set_mnemonic_widget (GTK_LABEL (oa_label), oa_combo);
> @@ -875,7 +890,7 @@ create_conversion_dialog (struct config *config)
> break;
> }
> table_attach (output_tbl, oa_combo,
> - 1, 2, 4, 5, GTK_FILL, GTK_FILL, 1, 1);
> + 1, 2, row, GTK_FILL, GTK_FILL, 1, 1);
>
> gtk_box_pack_start (GTK_BOX (output_vbox), output_tbl, TRUE, TRUE, 0);
> gtk_container_add (GTK_CONTAINER (output_frame), output_vbox);
Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com>
Rich.