After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 691997 - Code cleanups to terminal-screen/terminal-window
Code cleanups to terminal-screen/terminal-window
Status: RESOLVED FIXED
Product: gnome-terminal
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: gnome-3-8
Assigned To: GNOME Terminal Maintainers
GNOME Terminal Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-01-18 06:19 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2013-03-04 18:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
terminal-screen: Remove style-updated handlers (7.30 KB, patch)
2013-01-18 06:19 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
terminal-window: Remove terminal_window_set_size_force_grid (2.63 KB, patch)
2013-01-18 06:19 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
terminal-window: Remove screen argument from terminal_window_set_size() (3.97 KB, patch)
2013-01-18 06:19 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
terminal-window: Rename terminal_window_set_size (3.91 KB, patch)
2013-01-18 06:19 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2013-01-18 06:19:31 UTC
This fell out of the style_updated stuff I was doing to fix the resize
issue. You were faster to beat me, but I figured I might as well put
these on Bugzilla for inspection.
Comment 1 Jasper St. Pierre (not reading bugmail) 2013-01-18 06:19:32 UTC
Created attachment 233726 [details] [review]
terminal-screen: Remove style-updated handlers

Let the TerminalWindow tell us when the style is updated.
Comment 2 Jasper St. Pierre (not reading bugmail) 2013-01-18 06:19:35 UTC
Created attachment 233727 [details] [review]
terminal-window: Remove terminal_window_set_size_force_grid

It's effectively unused.
Comment 3 Jasper St. Pierre (not reading bugmail) 2013-01-18 06:19:38 UTC
Created attachment 233728 [details] [review]
terminal-window: Remove screen argument from terminal_window_set_size()

It's always the active screen.
Comment 4 Jasper St. Pierre (not reading bugmail) 2013-01-18 06:19:40 UTC
Created attachment 233729 [details] [review]
terminal-window: Rename terminal_window_set_size

terminal_window_update_size more accurately describes what it does.
Comment 5 Christian Persch 2013-01-18 12:39:49 UTC
Review of attachment 233726 [details] [review]:

::: src/terminal-window.c
@@ +1963,3 @@
+  TerminalWindow *window = TERMINAL_WINDOW (widget);
+  TerminalWindowPrivate *priv = window->priv;
+

Need to chain up to the parent class's handler.

@@ +1964,3 @@
+  TerminalWindowPrivate *priv = window->priv;
+
+  terminal_screen_update_style (priv->active_screen);

Lt's check for priv->active_screen != NULL first (it's theoretically possible a restyle happens before the first tab is added to the window).
Comment 6 Christian Persch 2013-01-18 12:42:18 UTC
The other patches are all fine; ok to commit the whole lot with the above comments fixed. Thanks!
Comment 7 Jasper St. Pierre (not reading bugmail) 2013-01-19 04:02:09 UTC
Attachment 233726 [details] pushed as f38b805 - terminal-screen: Remove style-updated handlers
Attachment 233728 [details] pushed as 837af8b - terminal-window: Remove screen argument from terminal_window_set_size()


Pushed with suggested changes.
Comment 8 Christian Persch 2013-03-03 21:13:55 UTC
Reopening; this broke resizing the window when changing zoom (ctrl-+ / ctrl--).

f38b805cce31a2c2ceb2fd93c11e04f0c18e2242 is the first bad commit
commit f38b805cce31a2c2ceb2fd93c11e04f0c18e2242
Author: Jasper St. Pierre <jstpierre@mecheye.net>
Date:   Fri Jan 18 01:08:05 2013 -0500

    terminal-screen: Remove style-updated handlers
    
    Let the TerminalWindow tell us when the style is updated.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=691997
Comment 9 Christian Persch 2013-03-04 17:54:53 UTC
Same problem also when changing the font / font size from the profile prefs for the tab's current profile.
Comment 10 Jasper St. Pierre (not reading bugmail) 2013-03-04 18:02:03 UTC
That's strange. Are you investigating, or do you want me to fix it?
Comment 11 Christian Persch 2013-03-04 18:53:27 UTC
The problem is that before the patch the terminal_screen_change_font directly updated the window geometry; and this function is called when the font or the font scale changes. After the patch, the window geometry is only updated when the style context changes (which doesn't happen with font or font scale changes).

Maybe the easiest solution would be to listen to notify::font-desc on the screen from the window and update the geometry if it changes. Let me try that... works.