GNOME Bugzilla – Bug 643725
[PATCH] Glade previewer window must be properly set.
Last modified: 2018-03-26 15:18:52 UTC
Created attachment 182303 [details] [review] Properly set glade previewer window title. The attached patch properly set the title in glade previewer window. Please review it.
Sure, some comments: a.) When launched for a file, it should also show what toplevel it's going to preview in the title b.) We should send the project name to the previewer process when launching the preview *and* when updating it That way the title can be always consistent: "Previewing %s (%s)" = "Previewing ~/myfile.ui (window1)" Furthermore, when running the previewer program standalone, we should make the filename look nicer (so it never says ../../../home/tristan/opt/share/...") To do so, use first: glade_util_canonical_path() to get a canonical path and then: glade_utils_replace_home_dir_with_tilde () to strip the leading "/home/username"
Created attachment 182438 [details] [review] Set window preview title. The attached patch implements the requirements that were asked. Please review it.
Created attachment 182439 [details] [review] Properly set glade previewer window title. Same as before, but with Changelog entry. Please review.
Ok, looks pretty good, one main thing could be improved and some minor details. Main thing that should be improved: Currently it looks like the preview window title is not updated when the preview is updated (i.e. the window title only gets updated once when the preview is initially launched). This should be fixed, actually ideally it should be fixed by adding a new token to the protocol, when the project name changes, it should send the new project name to all of it's running previews so those previews can update the window title. (This is to ensure running previews have the correct title when the project gets saved under a different name, or initially saved when priorly the project name was "Untitled 1" or such). Other than that, just a few easier to fix problems: + gwidget = glade_widget_get_toplevel (gwidget); + object = glade_widget_get_object (gwidget); + if (!GTK_IS_WIDGET (object)) + return; + + /* Temporarily turn visibility off */ + visible = gtk_widget_get_visible (GTK_WIDGET (object)); + gtk_widget_set_visible (GTK_WIDGET (object), FALSE); + This code, as I said on irc, has to use glade_widget_property_get() and glade_widget_property_set()... the actual visible state of the GtkWidget that the GladeWidget happens to be wrapping has no effect on the GladeProperty that is serialized (actually in this case, the GladeProperty does not even have any effect on the GtkWidget's runtime state, all widgets appear visible in the workspace, however Glade's data model is what we're after here). So we just need to change the gtk_widget_get/set_visible() for glade_widget_property_get/set() and this will work as intended. if (!preview) { - preview = glade_preview_launch (gwidget, text); + //preview = glade_preview_launch (gwidget, text); + project_name = glade_project_get_name (project); + preview = glade_preview_launch (gwidget, text, project_name); Here we are adding some garbage comments, please clean that out. Otherwise the patch looks fine. I would prefer if launching the preview initially had some stronger syntax in the protocol (i.e. using explicit keywords to depict data instead of a simple g_strsplit() call), but the protocol is a completely private detail and we don't need to ensure any api stability there so it's not really an issue immediately.
Looking at the code, I see that the project name comes from project->priv->path. So, I need to update previews whenever it changes. Is there a simple way to detect it?
According to what was agreed on with Tristan (on IRC), there must be a new function on glade-project.c called glade_project_set_path(). This function must call glade_preview_update_title() for all the previews of such project. The glade_preview_update_title() function will then send a token to the preview to update its title. We'll have to changes all accesses that modify priv->path to call glade_project_set_path(). Also, glade_preview_update_title() should be called if the toplevel widget it is previewing is changed.
(In reply to comment #6) > According to what was agreed on with Tristan (on IRC), there must be a new > function on glade-project.c called glade_project_set_path(). This function must > call glade_preview_update_title() for all the previews of such project. The > glade_preview_update_title() function will then send a token to the preview to > update its title. Right, and sending the token to update the title should be implied when initially creating the preview. > > We'll have to changes all accesses that modify priv->path to call > glade_project_set_path(). Also, glade_preview_update_title() should be called > if the toplevel widget it is previewing is changed. Right, if we want to put the toplevel widget name in the title then we must also watch the toplevel widget name.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glade/issues/83.