On Thu, Jan 19, 2017 at 12:44:15PM -0200, Eduardo Lima (Etrunko) wrote: > On 19/01/17 10:48, Christophe Fergeau wrote: > > The issues pointed out in > > https://www.redhat.com/archives/virt-tools-list/2017-January/msg00039.html > > do not seem to have been addressed? > > > > True, I did overlook that message, thanks for the reminder. I changed > the code so the spinner stops and set the reload button sensitive. This > way the user can try refreshing the list again in case of error. Also it > was not necessary to get the iso_list twice, as you pointed. > > > Did you try merging the patches from "Introduce ISO List dialog" to "Run > > iso-dialog when 'Change CD' menu is activated" (without the headerbar > > change)? I think they would make more sense together, did not check if > > that would make the diff far too big or not. > > > > I split those two because I thought the xml would be big enough for > itself, but I can merge the patches without problems. Personally, the XML file is just a big opaque thing, so it could be 10.000 lines, or 5 lines, it's the same to me during review, I skip it :) So maybe having all the changes together would work (once again, I haven't looked at a diff with all the C changes together). > >> +void > >> +remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer G_GNUC_UNUSED, > >> + gchar *path, > >> + gpointer user_data) > >> +{ > >> + RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(user_data); > >> + RemoteViewerISOListDialogPrivate *priv = self->priv; > >> + GtkTreeModel *model = GTK_TREE_MODEL(priv->list_store); > >> + GtkTreePath *tree_path = gtk_tree_path_new_from_string(path); > >> + GtkTreeIter iter; > >> + gboolean active; > >> + gchar *name; > >> + > >> + gtk_tree_view_set_cursor(GTK_TREE_VIEW(priv->tree_view), tree_path, NULL, FALSE); > >> + gtk_tree_model_get_iter(model, &iter, tree_path); > >> + gtk_tree_model_get(model, &iter, > >> + ISO_IS_ACTIVE, &active, > >> + ISO_NAME, &name, -1); > >> + > >> + gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); > >> + gtk_widget_set_sensitive(priv->tree_view, FALSE); > >> + > >> + ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active ? NULL : name, NULL, > >> + (GAsyncReadyCallback)ovirt_foreign_menu_iso_name_changed, > >> + self); > >> + gtk_tree_path_free(tree_path); > >> + g_free(name); > > > > > > That is quite similar to remote_viewer_iso_list_dialog_foreach(), > > dunno if part of the code could be shared? > > Hmm, not quite, here we get the ISO name the user clicked to be > set/unset, while the code there is used to populate the list, and if > there is one particular ISO set, that one is highlighted. Oops, I wanted to add that comment to ovirt_foreign_menu_iso_name_changed(), not to that function :( So ovirt_foreign_menu_iso_name_changed() and remote_viewer_iso_list_dialog_foreach() are vaguely similar. Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
