GNOME Bugzilla – Bug 567966
title management refactoring
Last modified: 2010-01-08 19:45:02 UTC
The title management code is obscure.
Created attachment 126574 [details] [review] first attempt The title is divided into a static and a dynamic part. The profile determines the static part, which can be overriden with the --title option. And the GUI (trough the Set Title dialog) allows the user to set the dynamic part, which then becomes static, except if it is empty.
I know, the icon-title stuff is missing at this time. If I've been doing things the right way so far, I'll add it.
Marking patch as needs-work. The icon title should be restored, I think.
Created attachment 127315 [details] [review] second attempt, with icon titles
Comment on attachment 127315 [details] [review] second attempt, with icon titles Index: src/terminal-screen.c =================================================================== --- src/terminal-screen.c (revision 3286) +++ src/terminal-screen.c (working copy) @@ -61,17 +61,15 @@ TerminalProfile *profile; /* may be NULL at times */ guint profile_changed_id; guint profile_forgotten_id; - char *raw_title, *raw_icon_title; - char *cooked_title, *cooked_icon_title; - char *override_title; - gboolean icon_title_set; + char *title, *icon_title; + char *static_title, *dynamic_title, *dynamic_icon_title; + gboolean dynamic_titles_are_static; char **initial_env; char **override_command; char *working_dir; int child_pid; double font_scale; guint recheck_working_dir_idle; - gboolean user_title; /* title was manually set */ GSList *match_tags; }; @@ -88,9 +86,12 @@ PROP_0, PROP_PROFILE, PROP_ICON_TITLE, - PROP_ICON_TITLE_SET, PROP_OVERRIDE_COMMAND, PROP_TITLE, + PROP_STATIC_TITLE, + PROP_DYNAMIC_TITLE, + PROP_DYNAMIC_ICON_TITLE, + PROP_DYNAMIC_TITLES_ARE_STATIC, PROP_INITIAL_ENVIRONMENT }; @@ -131,11 +132,6 @@ static void update_color_scheme (TerminalScreen *screen); -static gboolean terminal_screen_format_title (TerminalScreen *screen, const char *raw_title, char **old_cooked_title); - -static void terminal_screen_cook_title (TerminalScreen *screen); -static void terminal_screen_cook_icon_title (TerminalScreen *screen); - static void queue_recheck_working_dir (TerminalScreen *screen); static char* terminal_screen_check_match (TerminalScreen *screen, @@ -384,8 +380,11 @@ gtk_target_table_free (targets, n_targets); gtk_target_list_unref (target_list); - priv->override_title = NULL; - priv->user_title = FALSE; + priv->title = NULL; + priv->static_title = NULL; + priv->dynamic_title = NULL; + priv->dynamic_icon_title = NULL; + priv->dynamic_titles_are_static = FALSE; g_signal_connect (screen, "window-title-changed", G_CALLBACK (terminal_screen_window_title_changed), @@ -421,12 +420,6 @@ case PROP_PROFILE: g_value_set_object (value, terminal_screen_get_profile (screen)); break; - case PROP_ICON_TITLE: - g_value_set_string (value, terminal_screen_get_icon_title (screen)); - break; - case PROP_ICON_TITLE_SET: - g_value_set_boolean (value, terminal_screen_get_icon_title_set (screen)); - break; case PROP_OVERRIDE_COMMAND: g_value_set_boxed (value, terminal_screen_get_override_command (screen)); break; @@ -436,6 +429,21 @@ case PROP_TITLE: g_value_set_string (value, terminal_screen_get_title (screen)); break; + case PROP_ICON_TITLE: + g_value_set_string (value, terminal_screen_get_icon_title (screen)); + break; + case PROP_STATIC_TITLE: + g_value_set_string (value, terminal_screen_get_static_title (screen)); + break; + case PROP_DYNAMIC_TITLE: + g_value_set_string (value, terminal_screen_get_dynamic_title (screen)); + break; + case PROP_DYNAMIC_ICON_TITLE: + g_value_set_string (value, terminal_screen_get_dynamic_icon_title (screen)); + break; + case PROP_DYNAMIC_TITLES_ARE_STATIC: + g_value_set_boolean (value, terminal_screen_get_dynamic_titles_are_static (screen)); + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -466,8 +474,19 @@ case PROP_INITIAL_ENVIRONMENT: terminal_screen_set_initial_environment (screen, g_value_get_boxed (value)); break; + case PROP_STATIC_TITLE: + terminal_screen_set_static_title (screen, g_value_get_string (value)); + break; + case PROP_DYNAMIC_TITLE: + terminal_screen_set_dynamic_title (screen, g_value_get_string (value)); + break; + case PROP_DYNAMIC_ICON_TITLE: + terminal_screen_set_dynamic_icon_title (screen, g_value_get_string (value)); + break; + case PROP_DYNAMIC_TITLES_ARE_STATIC: + terminal_screen_set_dynamic_titles_are_static (screen, g_value_get_boolean (value)); + break; case PROP_ICON_TITLE: - case PROP_ICON_TITLE_SET: case PROP_TITLE: /* not writable */ default: @@ -548,6 +567,20 @@ g_object_class_install_property (object_class, + PROP_OVERRIDE_COMMAND, + g_param_spec_boxed ("override-command", NULL, NULL, + G_TYPE_STRV, + G_PARAM_READWRITE | G_PARAM_STATIC_NAME | G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)); + + g_object_class_install_property + (object_class, + PROP_TITLE, + g_param_spec_string ("title", NULL, NULL, + NULL, + G_PARAM_READABLE | G_PARAM_STATIC_NAME | G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)); + + g_object_class_install_property + (object_class, PROP_ICON_TITLE, g_param_spec_string ("icon-title", NULL, NULL, NULL, @@ -555,27 +588,34 @@ g_object_class_install_property (object_class, - PROP_ICON_TITLE_SET, - g_param_spec_boolean ("icon-title-set", NULL, NULL, - FALSE, - G_PARAM_READABLE | G_PARAM_STATIC_NAME | G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)); - + PROP_STATIC_TITLE, + g_param_spec_string ("static-title", NULL, NULL, + NULL, + G_PARAM_READWRITE | G_PARAM_STATIC_NAME | G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)); + g_object_class_install_property (object_class, - PROP_ICON_TITLE, - g_param_spec_boxed ("override-command", NULL, NULL, - G_TYPE_STRV, - G_PARAM_READWRITE | G_PARAM_STATIC_NAME | G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)); - + PROP_DYNAMIC_TITLE, + g_param_spec_string ("dynamic-title", NULL, NULL, + NULL, + G_PARAM_READWRITE | G_PARAM_STATIC_NAME | G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)); + g_object_class_install_property (object_class, - PROP_TITLE, - g_param_spec_string ("title", NULL, NULL, + PROP_DYNAMIC_ICON_TITLE, + g_param_spec_string ("dynamic-icon-title", NULL, NULL, NULL, - G_PARAM_READABLE | G_PARAM_STATIC_NAME | G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)); - + G_PARAM_READWRITE | G_PARAM_STATIC_NAME | G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)); + g_object_class_install_property (object_class, + PROP_DYNAMIC_TITLES_ARE_STATIC, + g_param_spec_string ("dynamic-titles-are-static", NULL, NULL, + NULL, + G_PARAM_READWRITE | G_PARAM_STATIC_NAME | G_PARAM_STATIC_NICK | G_PARAM_STATIC_BLURB)); + + g_object_class_install_property + (object_class, PROP_INITIAL_ENVIRONMENT, g_param_spec_boxed ("initial-environment", NULL, NULL, G_TYPE_STRV, @@ -654,11 +694,11 @@ if (priv->recheck_working_dir_idle) g_source_remove (priv->recheck_working_dir_idle); - g_free (priv->raw_title); - g_free (priv->cooked_title); - g_free (priv->override_title); - g_free (priv->raw_icon_title); - g_free (priv->cooked_icon_title); + g_free (priv->title); + g_free (priv->icon_title); + g_free (priv->static_title); + g_free (priv->dynamic_title); + g_free (priv->dynamic_icon_title); g_strfreev (priv->override_command); g_free (priv->working_dir); g_strfreev (priv->initial_env); @@ -686,7 +726,7 @@ terminal_screen_set_profile (screen, profile); if (title) - terminal_screen_set_override_title (screen, title); + terminal_screen_set_static_title (screen, title); if (working_dir) terminal_screen_set_working_dir (screen, working_dir); @@ -703,169 +743,185 @@ return screen; } -const char* -terminal_screen_get_raw_title (TerminalScreen *screen) +static char* +terminal_screen_format_title (TerminalScreen *screen, + const char *static_title, + const char *dynamic_title) { - TerminalScreenPrivate *priv = screen->priv; - - if (priv->raw_title) - return priv->raw_title; + TerminalTitleMode title_mode; + char *title; - return ""; + title_mode = terminal_profile_get_property_enum (screen->priv->profile, TERMINAL_PROFILE_TITLE_MODE); + title = NULL; + + switch (title_mode) + { + case TERMINAL_TITLE_REPLACE: + if (dynamic_title != NULL) + title = g_strdup (dynamic_title); + break; + case TERMINAL_TITLE_BEFORE: + if (static_title != NULL && dynamic_title != NULL) + title = g_strdup_printf ("%s - %s", dynamic_title, static_title); + break; + case TERMINAL_TITLE_AFTER: + if (static_title != NULL && dynamic_title != NULL) + title = g_strdup_printf ("%s - %s", static_title, dynamic_title); + break; + case TERMINAL_TITLE_IGNORE: + /* Ignore ;-) */ + break; + default: + g_assert_not_reached (); + } + + if (title == NULL && static_title != NULL) + title = g_strdup (static_title); + + return title; } -const char* -terminal_screen_get_title (TerminalScreen *screen) +static void +terminal_screen_update_title (TerminalScreen *screen) { TerminalScreenPrivate *priv = screen->priv; - - if (priv->cooked_title == NULL) - terminal_screen_cook_title (screen); + char *title; - /* cooked_title may still be NULL */ - if (priv->cooked_title != NULL) - return priv->cooked_title; + title = terminal_screen_format_title (screen, priv->static_title, priv->dynamic_title); + + if (g_strcmp0 (title, priv->title) != 0) + { + g_free (priv->title); + priv->title = title; + g_object_notify (G_OBJECT (screen), "title"); + } else - return ""; + { + g_free (title); + } } -const char* -terminal_screen_get_icon_title (TerminalScreen *screen) +static void +terminal_screen_update_icon_title (TerminalScreen *screen) { TerminalScreenPrivate *priv = screen->priv; - - if (priv->cooked_icon_title == NULL) - terminal_screen_cook_icon_title (screen); + char *icon_title; - /* cooked_icon_title may still be NULL */ - if (priv->cooked_icon_title != NULL) - return priv->cooked_icon_title; + icon_title = terminal_screen_format_title (screen, + priv->static_title, + priv->dynamic_icon_title != NULL ? priv->dynamic_icon_title : priv->dynamic_title); + + if (g_strcmp0 (icon_title, priv->icon_title) != 0) + { + g_free (priv->icon_title); + priv->icon_title = icon_title; + g_object_notify (G_OBJECT (screen), "icon-title"); + } else - return ""; + { + g_free (icon_title); + } } -gboolean -terminal_screen_get_icon_title_set (TerminalScreen *screen) +const char* +terminal_screen_get_static_title (TerminalScreen *screen) { - return screen->priv->icon_title_set; + g_return_val_if_fail (TERMINAL_IS_SCREEN (screen), NULL); + return screen->priv->static_title; } -/* Supported format specifiers: - * %S = static title - * %D = dynamic title - * %A = dynamic title, falling back to static title if empty - * %- = separator, if not at start or end of string (excluding whitespace) - */ -static const char * -terminal_screen_get_title_format (TerminalScreen *screen) +const char* +terminal_screen_get_dynamic_title (TerminalScreen *screen) { - TerminalScreenPrivate *priv = screen->priv; - static const char *formats[] = { - "%A" /* TERMINAL_TITLE_REPLACE */, - "%D%-%S" /* TERMINAL_TITLE_BEFORE */, - "%S%-%D" /* TERMINAL_TITLE_AFTER */, - "%S" /* TERMINAL_TITLE_IGNORE */ - }; + g_return_val_if_fail (TERMINAL_IS_SCREEN (screen), NULL); + return screen->priv->dynamic_title; +} - return formats[terminal_profile_get_property_enum (priv->profile, TERMINAL_PROFILE_TITLE_MODE)]; +const char* +terminal_screen_get_dynamic_icon_title (TerminalScreen *screen) +{ + g_return_val_if_fail (TERMINAL_IS_SCREEN (screen), NULL); + return screen->priv->dynamic_icon_title; } -/** - * terminal_screen_format_title:: - * @screen: - * @raw_title: main ingredient - * @titleptr <inout>: pointer of the current title string - * - * Format title according @format, and stores it in <literal>*titleptr</literal>. - * Always ensures that *titleptr will be non-NULL. - * - * Returns: %TRUE iff the title changed - */ -static gboolean -terminal_screen_format_title (TerminalScreen *screen, - const char *raw_title, - char **titleptr) +const char* +terminal_screen_get_title (TerminalScreen *screen) { - TerminalScreenPrivate *priv = screen->priv; - const char *format, *arg; - const char *static_title = NULL; - GString *title; - gboolean add_sep = FALSE; + g_return_val_if_fail (TERMINAL_IS_SCREEN (screen), NULL); + return screen->priv->title; +} - g_assert (titleptr); +const char* +terminal_screen_get_icon_title (TerminalScreen *screen) +{ + g_return_val_if_fail (TERMINAL_IS_SCREEN (screen), NULL); + return screen->priv->icon_title; +} - /* use --title argument if one was supplied, otherwise ask the profile */ - if (priv->override_title) - static_title = priv->override_title; - else - static_title = terminal_profile_get_property_string (priv->profile, TERMINAL_PROFILE_TITLE); +void +terminal_screen_set_static_title (TerminalScreen *screen, + const char *title) +{ + TerminalScreenPrivate *priv = screen->priv; - //title = g_string_sized_new (strlen (static_title) + strlen (raw_title) + 3 + 1); - title = g_string_sized_new (128); + g_assert (TERMINAL_IS_SCREEN (screen)); - format = terminal_screen_get_title_format (screen); - for (arg = format; *arg; arg += 2) - { - const char *text_to_append = NULL; + if (g_strcmp0 (title, priv->static_title) == 0) + return; + + g_free (priv->static_title); + priv->static_title = g_strdup (title); - g_assert (arg[0] == '%'); + terminal_screen_update_title (screen); +} - switch (arg[1]) - { - case 'A': - text_to_append = raw_title ? raw_title : static_title; - break; - case 'D': - text_to_append = raw_title; - break; - case 'S': - text_to_append = static_title; - break; - case '-': - text_to_append = NULL; - add_sep = TRUE; - break; - default: - g_assert_not_reached (); - } +void +terminal_screen_set_dynamic_title (TerminalScreen *screen, + const char *title) +{ + TerminalScreenPrivate *priv = screen->priv; - if (!text_to_append || !text_to_append[0]) - continue; + g_assert (TERMINAL_IS_SCREEN (screen)); - if (add_sep && title->len > 0) - g_string_append (title, " - "); + if (g_strcmp0 (title, priv->dynamic_title) == 0) + return; + + g_free (priv->dynamic_title); + priv->dynamic_title = g_strdup (title); - g_string_append (title, text_to_append); - add_sep = FALSE; - } - - if (*titleptr == NULL || strcmp (title->str, *titleptr) != 0) - { - g_free (*titleptr); - *titleptr = g_string_free (title, FALSE); - return TRUE; - } - - g_string_free (title, TRUE); - return FALSE; + terminal_screen_update_title (screen); } -static void -terminal_screen_cook_title (TerminalScreen *screen) +void +terminal_screen_set_dynamic_icon_title (TerminalScreen *screen, + const char *icon_title) { TerminalScreenPrivate *priv = screen->priv; + + g_assert (TERMINAL_IS_SCREEN (screen)); + + if (g_strcmp0 (icon_title, priv->dynamic_icon_title) == 0) + return; - if (terminal_screen_format_title (screen, priv->raw_title, &priv->cooked_title)) - g_object_notify (G_OBJECT (screen), "title"); + g_free (priv->dynamic_icon_title); + priv->dynamic_icon_title = g_strdup (icon_title); + + terminal_screen_update_icon_title (screen); } -static void -terminal_screen_cook_icon_title (TerminalScreen *screen) +gboolean +terminal_screen_get_dynamic_titles_are_static (TerminalScreen *screen) { - TerminalScreenPrivate *priv = screen->priv; + g_assert (TERMINAL_IS_SCREEN (screen)); + return screen->priv->dynamic_titles_are_static; +} - if (terminal_screen_format_title (screen, priv->raw_icon_title, &priv->cooked_icon_title)) - g_object_notify (G_OBJECT (screen), "icon-title"); +void +terminal_screen_set_dynamic_titles_are_static (TerminalScreen *screen, + gboolean is_static) +{ + g_assert (TERMINAL_IS_SCREEN (screen)); + screen->priv->dynamic_titles_are_static = is_static; } static void @@ -896,14 +952,19 @@ if (!prop_name || prop_name == I_(TERMINAL_PROFILE_SCROLLBAR_POSITION)) _terminal_screen_update_scrollbar (screen); - + + if (!prop_name || prop_name == I_(TERMINAL_PROFILE_TITLE)) + terminal_screen_set_static_title (screen, + terminal_profile_get_property_string (profile, + TERMINAL_PROFILE_TITLE)); + if (!prop_name || - prop_name == I_(TERMINAL_PROFILE_TITLE_MODE) || - prop_name == I_(TERMINAL_PROFILE_TITLE)) + prop_name == I_(TERMINAL_PROFILE_TITLE) || + prop_name == I_(TERMINAL_PROFILE_TITLE_MODE)) { - terminal_screen_cook_title (screen); - terminal_screen_cook_icon_title (screen); - } + terminal_screen_update_title (screen); + terminal_screen_update_icon_title (screen); + } if (GTK_WIDGET_REALIZED (screen) && (!prop_name || @@ -1647,86 +1708,7 @@ return FALSE; } -static void -terminal_screen_set_dynamic_title (TerminalScreen *screen, - const char *title, - gboolean userset) -{ - TerminalScreenPrivate *priv = screen->priv; - - g_assert (TERMINAL_IS_SCREEN (screen)); - - if ((priv->user_title && !userset) || - (priv->raw_title && title && - strcmp (priv->raw_title, title) == 0)) - return; - - g_free (priv->raw_title); - priv->raw_title = g_strdup (title); - terminal_screen_cook_title (screen); -} - -static void -terminal_screen_set_dynamic_icon_title (TerminalScreen *screen, - const char *icon_title, - gboolean userset) -{ - TerminalScreenPrivate *priv = screen->priv; - GObject *object = G_OBJECT (screen); - - g_assert (TERMINAL_IS_SCREEN (screen)); - - if ((priv->user_title && !userset) || - (priv->icon_title_set && - priv->raw_icon_title && - icon_title && - strcmp (priv->raw_icon_title, icon_title) == 0)) - return; - - g_object_freeze_notify (object); - - g_free (priv->raw_icon_title); - priv->raw_icon_title = g_strdup (icon_title); - priv->icon_title_set = TRUE; - - g_object_notify (object, "icon-title-set"); - terminal_screen_cook_icon_title (screen); - - g_object_thaw_notify (object); -} - void -terminal_screen_set_override_title (TerminalScreen *screen, - const char *title) -{ - TerminalScreenPrivate *priv = screen->priv; - char *old_title; - - old_title = priv->override_title; - priv->override_title = g_strdup (title); - g_free (old_title); - - terminal_screen_set_dynamic_title (screen, title, FALSE); - terminal_screen_set_dynamic_icon_title (screen, title, FALSE); -} - -const char* -terminal_screen_get_dynamic_title (TerminalScreen *screen) -{ - g_return_val_if_fail (TERMINAL_IS_SCREEN (screen), NULL); - - return screen->priv->raw_title; -} - -const char* -terminal_screen_get_dynamic_icon_title (TerminalScreen *screen) -{ - g_return_val_if_fail (TERMINAL_IS_SCREEN (screen), NULL); - - return screen->priv->raw_icon_title; -} - -void terminal_screen_set_working_dir (TerminalScreen *screen, const char *dirname) { @@ -1868,9 +1850,11 @@ terminal_screen_window_title_changed (VteTerminal *vte_terminal, TerminalScreen *screen) { + if (screen->priv->dynamic_titles_are_static) + return; + terminal_screen_set_dynamic_title (screen, - vte_terminal_get_window_title (vte_terminal), - FALSE); + vte_terminal_get_window_title (vte_terminal)); queue_recheck_working_dir (screen); } @@ -1879,9 +1863,11 @@ terminal_screen_icon_title_changed (VteTerminal *vte_terminal, TerminalScreen *screen) { + if (screen->priv->dynamic_titles_are_static) + return; + terminal_screen_set_dynamic_icon_title (screen, - vte_terminal_get_icon_title (vte_terminal), - FALSE); + vte_terminal_get_icon_title (vte_terminal)); queue_recheck_working_dir (screen); } @@ -1912,24 +1898,6 @@ } } -void -terminal_screen_set_user_title (TerminalScreen *screen, - const char *text) -{ - TerminalScreenPrivate *priv = screen->priv; - - /* The user set the title to nothing, let's understand that as a - request to revert to dynamically setting the title again. */ - if (!text || !text[0]) - priv->user_title = FALSE; - else - { - priv->user_title = TRUE; - terminal_screen_set_dynamic_title (screen, text, TRUE); - terminal_screen_set_dynamic_icon_title (screen, text, TRUE); - } -} - static void terminal_screen_drag_data_received (GtkWidget *widget, GdkDragContext *context, @@ -2275,8 +2243,8 @@ terminal_util_key_file_set_argv (key_file, group, TERMINAL_CONFIG_TERMINAL_PROP_COMMAND, -1, priv->override_command); - if (priv->override_title) - g_key_file_set_string (key_file, group, TERMINAL_CONFIG_TERMINAL_PROP_TITLE, priv->override_title); + if (priv->dynamic_titles_are_static) + g_key_file_set_string (key_file, group, TERMINAL_CONFIG_TERMINAL_PROP_TITLE, priv->dynamic_title); dir = terminal_screen_get_working_dir (screen); if (dir != NULL && *dir != '\0') /* should always be TRUE anyhow */ Index: src/terminal-tab-label.c =================================================================== --- src/terminal-tab-label.c (revision 3286) +++ src/terminal-tab-label.c (working copy) @@ -72,9 +72,9 @@ title = terminal_screen_get_title (screen); hbox = gtk_widget_get_parent (label); - gtk_label_set_text (GTK_LABEL (label), title); + gtk_label_set_text (GTK_LABEL (label), title != NULL ? title : _("Terminal")); - gtk_widget_set_tooltip_text (hbox, title); + gtk_widget_set_tooltip_text (hbox, title != NULL ? title : _("Terminal")); } /* public functions */ Index: src/terminal-screen.h =================================================================== --- src/terminal-screen.h (revision 3286) +++ src/terminal-screen.h (working copy) @@ -95,13 +95,21 @@ void terminal_screen_launch_child (TerminalScreen *screen); -const char* terminal_screen_get_raw_title (TerminalScreen *screen); -const char* terminal_screen_get_title (TerminalScreen *screen); -const char* terminal_screen_get_icon_title (TerminalScreen *screen); -gboolean terminal_screen_get_icon_title_set (TerminalScreen *screen); +const char* terminal_screen_get_title (TerminalScreen *screen); +const char* terminal_screen_get_icon_title (TerminalScreen *screen); +const char* terminal_screen_get_static_title (TerminalScreen *screen); +const char* terminal_screen_get_dynamic_title (TerminalScreen *screen); +const char* terminal_screen_get_dynamic_icon_title (TerminalScreen *screen); +void terminal_screen_set_static_title (TerminalScreen *screen, + const char *title); +void terminal_screen_set_dynamic_title (TerminalScreen *screen, + const char *title); +void terminal_screen_set_dynamic_icon_title (TerminalScreen *screen, + const char *title); -void terminal_screen_set_user_title (TerminalScreen *screen, - const char *text); +gboolean terminal_screen_get_dynamic_titles_are_static (TerminalScreen *screen); +void terminal_screen_set_dynamic_titles_are_static (TerminalScreen *screen, + gboolean is_static); void terminal_screen_set_override_title (TerminalScreen *screen, const char *title); Index: src/terminal-tabs-menu.c =================================================================== --- src/terminal-tabs-menu.c (revision 3286) +++ src/terminal-tabs-menu.c (working copy) @@ -176,7 +176,7 @@ title = terminal_screen_get_title (screen); - g_object_set (action, "label", title, NULL); + g_object_set (action, "label", title != NULL ? title : _("Terminal"), NULL); } static void Index: src/terminal-window.c =================================================================== --- src/terminal-window.c (revision 3286) +++ src/terminal-window.c (working copy) @@ -2059,11 +2059,14 @@ TerminalWindow *window) { TerminalWindowPrivate *priv = window->priv; + const char *title; if (screen != priv->active_screen) return; - gtk_window_set_title (GTK_WINDOW (window), terminal_screen_get_title (screen)); + title = terminal_screen_get_title (screen); + + gtk_window_set_title (GTK_WINDOW (window), title != NULL ? title : _("Terminal")); } static void @@ -2072,35 +2075,20 @@ TerminalWindow *window) { TerminalWindowPrivate *priv = window->priv; - + const char *icon_title; + if (screen != priv->active_screen) return; - if (!terminal_screen_get_icon_title_set (screen)) - return; + icon_title = terminal_screen_get_icon_title (screen); - gdk_window_set_icon_name (GTK_WIDGET (window)->window, terminal_screen_get_icon_title (screen)); + gdk_window_set_icon_name (GTK_WIDGET (window)->window, + icon_title != NULL ? icon_title : gtk_window_get_title(GTK_WINDOW (window))); + + /* FIXME: when GTK+ #535557 is fixed, write that instead: */ + // gdk_window_set_icon_name (GTK_WIDGET (window)->window, icon_title); } -static void -sync_screen_icon_title_set (TerminalScreen *screen, - GParamSpec *psepc, - TerminalWindow *window) -{ - TerminalWindowPrivate *priv = window->priv; - - if (screen != priv->active_screen) - return; - - if (terminal_screen_get_icon_title_set (screen)) - return; - - /* Need to reset the icon name */ - /* FIXME: Once gtk+ bug 535557 is fixed, use that to unset the icon title. */ - - gdk_window_set_icon_name (GTK_WIDGET (window)->window, terminal_screen_get_title (screen)); -} - /* Notebook callbacks */ static void @@ -2395,7 +2383,6 @@ } sync_screen_title (screen, NULL, window); - sync_screen_icon_title_set (screen, NULL, window); sync_screen_icon_title (screen, NULL, window); /* set size of window to current grid size */ @@ -2547,8 +2534,6 @@ G_CALLBACK (sync_screen_title), window); g_signal_connect (screen, "notify::icon-title", G_CALLBACK (sync_screen_icon_title), window); - g_signal_connect (screen, "notify::icon-title-set", - G_CALLBACK (sync_screen_icon_title_set), window); g_signal_connect (screen, "selection-changed", G_CALLBACK (terminal_window_update_copy_sensitivity), window); @@ -2623,16 +2608,11 @@ g_signal_handlers_disconnect_by_func (G_OBJECT (screen), G_CALLBACK (sync_screen_title), window); - g_signal_handlers_disconnect_by_func (G_OBJECT (screen), G_CALLBACK (sync_screen_icon_title), window); g_signal_handlers_disconnect_by_func (G_OBJECT (screen), - G_CALLBACK (sync_screen_icon_title_set), - window); - - g_signal_handlers_disconnect_by_func (G_OBJECT (screen), G_CALLBACK (terminal_window_update_copy_sensitivity), window); @@ -3217,10 +3197,15 @@ { GtkEntry *entry; const char *text; + gboolean text_is_empty; entry = GTK_ENTRY (g_object_get_data (G_OBJECT (dialog), "title-entry")); text = gtk_entry_get_text (entry); - terminal_screen_set_user_title (screen, text); + text_is_empty = strcmp (text, "") == 0; + + terminal_screen_set_dynamic_titles_are_static (screen, !text_is_empty); + terminal_screen_set_dynamic_title (screen, !text_is_empty ? text : NULL); + terminal_screen_set_dynamic_icon_title (screen, !text_is_empty ? text : NULL); } gtk_widget_destroy (dialog); @@ -3232,6 +3217,7 @@ { TerminalWindowPrivate *priv = window->priv; GtkWidget *dialog, *hbox, *label, *entry; + const char *title; if (priv->active_screen == NULL) return; @@ -3272,7 +3258,8 @@ gtk_widget_show_all (hbox); gtk_widget_grab_focus (entry); - gtk_entry_set_text (GTK_ENTRY (entry), terminal_screen_get_raw_title (priv->active_screen)); + if ((title = terminal_screen_get_dynamic_title (priv->active_screen)) != NULL) + gtk_entry_set_text (GTK_ENTRY (entry), title); gtk_editable_select_region (GTK_EDITABLE (entry), 0, -1); g_object_set_data (G_OBJECT (dialog), "title-entry", entry);
Created attachment 127316 [details] [review] third attempt wooops, don't know what Bugzilla did; I wanted to edit (add two lines to) my last patch. Sorry for the mess.
I would prefer if no changes outside terminal-screen were made; in particular I don't think the changes wrt icon-title-set are correct in terminal-window; we really only should set a separate icon-title with the gdk function if there's an different icon title to be set. I don't see the need to rename any properties in terminal-screen, or to add the new ones, which are unused outside terminal-screen anyway in the patch. - TerminalScreenPrivate *priv = screen->priv; [...] + title_mode = terminal_profile_get_property_enum (screen->priv->profile, TERMINAL_PROFILE_TITLE_MODE); Just leave the Private *priv define, and use it instead of going screen->priv->.... . + if (dynamic_title != NULL) + title = g_strdup (dynamic_title); g_strdup is NULL-safe. terminal-window.c: + terminal_screen_set_dynamic_titles_are_static (screen, !text_is_empty); + terminal_screen_set_dynamic_title (screen, !text_is_empty ? text : NULL); + terminal_screen_set_dynamic_icon_title (screen, !text_is_empty ? text : NULL); Shouldn't the set-title dialogue set the 'static title' part, not the dynamic title ? I know it currently doesn't, but I think it should. That would also let you get rid of the dynamic-is-static thingy. - prop_name == I_(TERMINAL_PROFILE_TITLE_MODE) || - prop_name == I_(TERMINAL_PROFILE_TITLE)) + prop_name == I_(TERMINAL_PROFILE_TITLE) || + prop_name == I_(TERMINAL_PROFILE_TITLE_MODE)) Indent accident?
Created attachment 128119 [details] [review] fourth attempt I corrected the code according to your recommendations. Notably, I dropped the properties (and the get-setters which where not used outside of terminal-screen) and reverted the changes in terminal-window. However, I do not agree about renaming. I think the actual names are confusing (especially the mapping field-getter-setter, e.g. the getter of raw_title is get_dynamic_title, but there is still a get_raw_title function, and set_user_title is not exactly the setter of the user_title boolean). There is one last thing: the icon-title-set event seems useless since when it is set, both icon-title-set and icon-title signals are emitted. Shall I remove it?
BTW, this patch fixes #510315 too.
Created attachment 128122 [details] [review] fifth attempt If the title is set via --title or the dialog, the replace mode is disabled (and the static user-defined title is kept). Otherwise, I'd find the feature useless. This fixes #567158.
(In reply to comment #8) > There is one last thing: the icon-title-set event seems useless since when it > is set, both icon-title-set and icon-title signals are emitted. Shall I remove > it? You want to replace icont-title-set with an "icontitle != NULL" check in the terminal-window code? Seems ok to me.
Created attachment 128145 [details] [review] without the icon-title-set signal
As discussed on the other bug, I think we don't want this "override_dynamic_title" handling. + if (!prop_name || prop_name == I_(TERMINAL_PROFILE_TITLE)) + terminal_screen_set_static_title (screen, + terminal_profile_get_property_string (profile, + TERMINAL_PROFILE_TITLE), + FALSE); + This will make us use the profile's title when it's changed while running, over a --title argument, won't it? I think the title formatting code should treat NULL and "" equally, not just check for != NULL. This code does a lot of string dup'ing the static/dynamic title when in KEEP or REPLACE mode; do you think you could make the code use simply the already-stored string in these cases?
Created attachment 128362 [details] [review] sixth attempt (In reply to comment #13) > As discussed on the other bug, I think we don't want this > "override_dynamic_title" handling. Removed. > This will make us use the profile's title when it's changed while running, over > a --title argument, won't it? Fixed. > I think the title formatting code should treat NULL and "" equally, not just > check for != NULL. Added. > This code does a lot of string dup'ing the static/dynamic title when in KEEP or > REPLACE mode; do you think you could make the code use simply the > already-stored string in these cases? I added strings and pointers comparison to avoid useless duplications in that case.
Reviewing attachment 128362 [details] [review]: Seems mostly ready, just a few things: - if (priv->override_title) - g_key_file_set_string (key_file, group, TERMINAL_CONFIG_TERMINAL_PROP_TITLE, priv->override_title); + g_key_file_set_string (key_file, group, TERMINAL_CONFIG_TERMINAL_PROP_TITLE, priv->static_title); Should only do this if static_title != NULL, I think. + gtk_entry_set_text (GTK_ENTRY (entry), terminal_screen_get_static_title (priv->active_screen)); And here. + gdk_window_set_icon_name (GTK_WIDGET (window)->window, terminal_screen_get_title (screen)); And here. Etc. for all users of terminal_screen_get_*_title since that used to never return NULL. Or perhaps return "" if NULL in get_static_title ? terminal_screen_format_title logic wrt. return value is a bit confusing to follow; I liked the old concept of the inout char** better, but I won't insist on it.
Doesn't really make sense. Never mind.