Merge lp:~artem-anufrij/scratch/Bugfix-1085718 into lp:~elementary-apps/scratch/scratch
- Bugfix-1085718
- Merge into scratch
Status: | Merged |
---|---|
Approved by: | Raphael Isemann |
Approved revision: | 1409 |
Merged at revision: | 1420 |
Proposed branch: | lp:~artem-anufrij/scratch/Bugfix-1085718 |
Merge into: | lp:~elementary-apps/scratch/scratch |
Diff against target: |
264 lines (+86/-23) 4 files modified
plugins/terminal/terminal.vala (+70/-18) src/MainWindow.vala (+11/-3) src/Widgets/SplitView.vala (+3/-1) src/Widgets/ToolBar.vala (+2/-1) |
To merge this branch: | bzr merge lp:~artem-anufrij/scratch/Bugfix-1085718 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Raphael Isemann (community) | functionality, code-style | Approve | |
Danielle Foré | ux | Approve | |
Artem Anufrij | (ux) | Pending | |
Review via email: mp+242706@code.launchpad.net |
This proposal supersedes a proposal from 2014-11-12.
Commit message
Allow to set the position of the terminal via context-menu.
Description of the change
Switching terminal location via context menu.
Danielle Foré (danrabbit) wrote : Posted in a previous version of this proposal | # |
Artem Anufrij (artem-anufrij) wrote : Posted in a previous version of this proposal | # |
Hey Dan,
I didn't get any feedback on slack.com about this behavior. I use since a week this branch on my system and it's very helpful, although it will be checked the window width in pix, and not line width.
I think we can merge this branch.
What do you think about?
Artem Anufrij (artem-anufrij) wrote : Posted in a previous version of this proposal | # |
Cody Garver (codygarver) wrote : Posted in a previous version of this proposal | # |
The terminal starts too slim, no? http://
Artem Anufrij (artem-anufrij) wrote : Posted in a previous version of this proposal | # |
@Cody:
It's the currently width of the right bar.
Danielle Foré (danrabbit) wrote : Posted in a previous version of this proposal | # |
Yeah that's way too thin. And again, I don't think it's acceptable to hardcode a px size. We need to be smart about what a sane minimum size is for the terminal and also about what space we have available. hardcoding a px size doesn't take into account other plugins or when settings have been changed by the user.
Artem Anufrij (artem-anufrij) wrote : Posted in a previous version of this proposal | # |
Dan, what do you think about a button in the toolbar for manual switching the terminal location?
Danielle Foré (danrabbit) wrote : Posted in a previous version of this proposal | # |
If we can't do it automatically, perhaps a context menu on the Terminal would be a little more subtle.
Artem Anufrij (artem-anufrij) wrote : Posted in a previous version of this proposal | # |
Could you please review the branch...
- 1409. By artem-anufrij
-
Change Menu items: Terminal on Right | Terminal on Bottom
Danielle Foré (danrabbit) wrote : | # |
Saw screenshots on Slack. Looks solid :)
Raphael Isemann (teemperor) wrote : | # |
Looks fine to me in terms of functionality and code-style.
Preview Diff
1 | === modified file 'plugins/terminal/terminal.vala' | |||
2 | --- plugins/terminal/terminal.vala 2014-11-10 18:09:21 +0000 | |||
3 | +++ plugins/terminal/terminal.vala 2014-11-24 21:01:57 +0000 | |||
4 | @@ -26,7 +26,13 @@ | |||
5 | 26 | public class Scratch.Plugins.Terminal : Peas.ExtensionBase, Peas.Activatable { | 26 | public class Scratch.Plugins.Terminal : Peas.ExtensionBase, Peas.Activatable { |
6 | 27 | 27 | ||
7 | 28 | MainWindow window = null; | 28 | MainWindow window = null; |
8 | 29 | |||
9 | 29 | Gtk.Notebook? bottombar = null; | 30 | Gtk.Notebook? bottombar = null; |
10 | 31 | Gtk.Notebook? contextbar = null; | ||
11 | 32 | |||
12 | 33 | Gtk.RadioMenuItem location_bottom = null; | ||
13 | 34 | Gtk.RadioMenuItem location_right = null; | ||
14 | 35 | |||
15 | 30 | Vte.Terminal terminal; | 36 | Vte.Terminal terminal; |
16 | 31 | Gtk.Grid grid; | 37 | Gtk.Grid grid; |
17 | 32 | 38 | ||
18 | @@ -37,12 +43,13 @@ | |||
19 | 37 | } | 43 | } |
20 | 38 | 44 | ||
21 | 39 | public void activate () { | 45 | public void activate () { |
22 | 46 | |||
23 | 40 | plugins = (Scratch.Services.Interface) object; | 47 | plugins = (Scratch.Services.Interface) object; |
24 | 41 | 48 | ||
25 | 42 | plugins.hook_window.connect ((w) => { | 49 | plugins.hook_window.connect ((w) => { |
26 | 43 | if (window != null) | 50 | if (window != null) |
27 | 44 | return; | 51 | return; |
29 | 45 | 52 | ||
30 | 46 | window = w; | 53 | window = w; |
31 | 47 | window.key_press_event.connect (switch_focus); | 54 | window.key_press_event.connect (switch_focus); |
32 | 48 | }); | 55 | }); |
33 | @@ -50,38 +57,66 @@ | |||
34 | 50 | plugins.hook_notebook_bottom.connect ((n) => { | 57 | plugins.hook_notebook_bottom.connect ((n) => { |
35 | 51 | if (bottombar == null) { | 58 | if (bottombar == null) { |
36 | 52 | this.bottombar = n; | 59 | this.bottombar = n; |
42 | 53 | on_hook (this.bottombar); | 60 | } |
43 | 54 | } | 61 | switch_terminal_location (); |
44 | 55 | }); | 62 | }); |
45 | 56 | if (bottombar != null) | 63 | |
46 | 57 | on_hook (this.bottombar); | 64 | plugins.hook_notebook_context.connect ((n) => { |
47 | 65 | if (contextbar == null) { | ||
48 | 66 | this.contextbar = n; | ||
49 | 67 | } | ||
50 | 68 | }); | ||
51 | 69 | |||
52 | 70 | on_hook (); | ||
53 | 58 | } | 71 | } |
54 | 59 | 72 | ||
55 | 60 | public void deactivate () { | 73 | public void deactivate () { |
56 | 61 | if (terminal != null) | 74 | if (terminal != null) |
57 | 62 | grid.destroy (); | 75 | grid.destroy (); |
60 | 63 | if (window != null) | 76 | |
61 | 64 | window.key_press_event.disconnect (switch_focus); | 77 | window.key_press_event.disconnect (switch_focus); |
62 | 78 | } | ||
63 | 79 | |||
64 | 80 | void switch_terminal_location () { | ||
65 | 81 | |||
66 | 82 | if (bottombar.page_num (grid) == -1 && this.location_bottom.active) { | ||
67 | 83 | |||
68 | 84 | contextbar.remove_page (contextbar.page_num (grid)); | ||
69 | 85 | bottombar.append_page (grid, new Gtk.Label (_("Terminal"))); | ||
70 | 86 | debug ("Move Terminal: BOTTOMBAR."); | ||
71 | 87 | |||
72 | 88 | } else if (contextbar.page_num (grid) == -1 && this.location_right.active) { | ||
73 | 89 | |||
74 | 90 | bottombar.remove_page (bottombar.page_num (grid)); | ||
75 | 91 | contextbar.append_page (grid, new Gtk.Label (_("Terminal"))); | ||
76 | 92 | debug ("Move Terminal: CONTEXTBAR."); | ||
77 | 93 | |||
78 | 94 | } | ||
79 | 65 | } | 95 | } |
80 | 66 | 96 | ||
81 | 67 | bool switch_focus (Gdk.EventKey event) { | 97 | bool switch_focus (Gdk.EventKey event) { |
82 | 68 | if (event.keyval == Gdk.Key.t | 98 | if (event.keyval == Gdk.Key.t |
83 | 69 | && Gdk.ModifierType.MOD1_MASK in event.state | 99 | && Gdk.ModifierType.MOD1_MASK in event.state |
84 | 70 | && Gdk.ModifierType.CONTROL_MASK in event.state) { | 100 | && Gdk.ModifierType.CONTROL_MASK in event.state) { |
85 | 101 | |||
86 | 71 | if (terminal.has_focus && window.get_current_document () != null) { | 102 | if (terminal.has_focus && window.get_current_document () != null) { |
87 | 103 | |||
88 | 72 | window.get_current_document ().focus (); | 104 | window.get_current_document ().focus (); |
89 | 73 | debug ("Move focus: EDITOR."); | 105 | debug ("Move focus: EDITOR."); |
90 | 74 | return true; | 106 | return true; |
91 | 107 | |||
92 | 75 | } else if (window.get_current_document () != null && window.get_current_document ().source_view.has_focus) { | 108 | } else if (window.get_current_document () != null && window.get_current_document ().source_view.has_focus) { |
93 | 109 | |||
94 | 76 | terminal.grab_focus (); | 110 | terminal.grab_focus (); |
95 | 77 | debug ("Move focus: TERMINAL."); | 111 | debug ("Move focus: TERMINAL."); |
96 | 78 | return true; | 112 | return true; |
97 | 113 | |||
98 | 79 | } | 114 | } |
99 | 80 | } | 115 | } |
100 | 81 | return false; | 116 | return false; |
101 | 82 | } | 117 | } |
102 | 83 | 118 | ||
104 | 84 | void on_hook (Gtk.Notebook notebook) { | 119 | void on_hook () { |
105 | 85 | this.terminal = new Vte.Terminal (); | 120 | this.terminal = new Vte.Terminal (); |
106 | 86 | this.terminal.scrollback_lines = -1; | 121 | this.terminal.scrollback_lines = -1; |
107 | 87 | 122 | ||
108 | @@ -147,19 +182,39 @@ | |||
109 | 147 | // Popup menu | 182 | // Popup menu |
110 | 148 | var menu = new Gtk.Menu (); | 183 | var menu = new Gtk.Menu (); |
111 | 149 | 184 | ||
113 | 150 | var copy = new Gtk.MenuItem.with_label (_("Copy")); | 185 | // COPY |
114 | 186 | Gtk.MenuItem copy = new Gtk.MenuItem.with_label (_("Copy")); | ||
115 | 151 | copy.activate.connect (() => { | 187 | copy.activate.connect (() => { |
116 | 152 | terminal.copy_clipboard (); | 188 | terminal.copy_clipboard (); |
117 | 153 | }); | 189 | }); |
118 | 154 | menu.append (copy); | 190 | menu.append (copy); |
119 | 155 | copy.show (); | ||
120 | 156 | 191 | ||
122 | 157 | var paste = new Gtk.MenuItem.with_label (_("Paste")); | 192 | // PASTE |
123 | 193 | Gtk.MenuItem paste = new Gtk.MenuItem.with_label (_("Paste")); | ||
124 | 158 | paste.activate.connect (() => { | 194 | paste.activate.connect (() => { |
125 | 159 | terminal.paste_clipboard (); | 195 | terminal.paste_clipboard (); |
126 | 160 | }); | 196 | }); |
127 | 161 | menu.append (paste); | 197 | menu.append (paste); |
129 | 162 | paste.show (); | 198 | |
130 | 199 | // SEPARATOR | ||
131 | 200 | menu.append (new Gtk.SeparatorMenuItem ()); | ||
132 | 201 | |||
133 | 202 | // ON RIGHT | ||
134 | 203 | location_right = new Gtk.RadioMenuItem.with_label (null, _("Terminal on Right")); | ||
135 | 204 | location_right.toggled.connect (() => { | ||
136 | 205 | switch_terminal_location (); | ||
137 | 206 | }); | ||
138 | 207 | menu.append (location_right); | ||
139 | 208 | |||
140 | 209 | // ON BOTTOM | ||
141 | 210 | location_bottom = new Gtk.RadioMenuItem.with_label (location_right.get_group (), _("Terminal on Bottom")); | ||
142 | 211 | location_bottom.active = true; | ||
143 | 212 | location_bottom.toggled.connect (() => { | ||
144 | 213 | switch_terminal_location (); | ||
145 | 214 | }); | ||
146 | 215 | menu.append (location_bottom); | ||
147 | 216 | |||
148 | 217 | menu.show_all (); | ||
149 | 163 | 218 | ||
150 | 164 | this.terminal.button_press_event.connect ((event) => { | 219 | this.terminal.button_press_event.connect ((event) => { |
151 | 165 | if (event.button == 3) { | 220 | if (event.button == 3) { |
152 | @@ -168,7 +223,7 @@ | |||
153 | 168 | } | 223 | } |
154 | 169 | return false; | 224 | return false; |
155 | 170 | }); | 225 | }); |
157 | 171 | 226 | ||
158 | 172 | try { | 227 | try { |
159 | 173 | this.terminal.fork_command_full (Vte.PtyFlags.DEFAULT, "~/", { Vte.get_user_shell () }, null, GLib.SpawnFlags.SEARCH_PATH, null, null); | 228 | this.terminal.fork_command_full (Vte.PtyFlags.DEFAULT, "~/", { Vte.get_user_shell () }, null, GLib.SpawnFlags.SEARCH_PATH, null, null); |
160 | 174 | } catch (GLib.Error e) { | 229 | } catch (GLib.Error e) { |
161 | @@ -184,10 +239,7 @@ | |||
162 | 184 | terminal.vexpand = true; | 239 | terminal.vexpand = true; |
163 | 185 | terminal.hexpand = true; | 240 | terminal.hexpand = true; |
164 | 186 | 241 | ||
165 | 187 | notebook.append_page (grid, new Gtk.Label (_("Terminal"))); | ||
166 | 188 | |||
167 | 189 | grid.show_all (); | 242 | grid.show_all (); |
168 | 190 | |||
169 | 191 | } | 243 | } |
170 | 192 | } | 244 | } |
171 | 193 | 245 | ||
172 | @@ -196,4 +248,4 @@ | |||
173 | 196 | var objmodule = module as Peas.ObjectModule; | 248 | var objmodule = module as Peas.ObjectModule; |
174 | 197 | objmodule.register_extension_type (typeof (Peas.Activatable), | 249 | objmodule.register_extension_type (typeof (Peas.Activatable), |
175 | 198 | typeof (Scratch.Plugins.Terminal)); | 250 | typeof (Scratch.Plugins.Terminal)); |
176 | 199 | } | ||
177 | 200 | \ No newline at end of file | 251 | \ No newline at end of file |
178 | 252 | } | ||
179 | 201 | 253 | ||
180 | === modified file 'src/MainWindow.vala' | |||
181 | --- src/MainWindow.vala 2014-11-23 10:44:26 +0000 | |||
182 | +++ src/MainWindow.vala 2014-11-24 21:01:57 +0000 | |||
183 | @@ -214,13 +214,21 @@ | |||
184 | 214 | 214 | ||
185 | 215 | this.contextbar = new Gtk.Notebook (); | 215 | this.contextbar = new Gtk.Notebook (); |
186 | 216 | this.contextbar.no_show_all = true; | 216 | this.contextbar.no_show_all = true; |
187 | 217 | this.contextbar.page_added.connect (() => { on_plugin_toggled (contextbar); }); | ||
188 | 218 | this.contextbar.page_removed.connect (() => { on_plugin_toggled (contextbar); }); | 217 | this.contextbar.page_removed.connect (() => { on_plugin_toggled (contextbar); }); |
189 | 218 | this.contextbar.page_added.connect (() => { | ||
190 | 219 | if (!this.split_view.is_empty ()) | ||
191 | 220 | on_plugin_toggled (contextbar); | ||
192 | 221 | }); | ||
193 | 222 | |||
194 | 223 | |||
195 | 219 | 224 | ||
196 | 220 | this.bottombar = new Gtk.Notebook (); | 225 | this.bottombar = new Gtk.Notebook (); |
197 | 221 | this.bottombar.no_show_all = true; | 226 | this.bottombar.no_show_all = true; |
198 | 222 | this.bottombar.page_added.connect (() => { on_plugin_toggled (bottombar); }); | ||
199 | 223 | this.bottombar.page_removed.connect (() => { on_plugin_toggled (bottombar); }); | 227 | this.bottombar.page_removed.connect (() => { on_plugin_toggled (bottombar); }); |
200 | 228 | this.bottombar.page_added.connect (() => { | ||
201 | 229 | if (!this.split_view.is_empty ()) | ||
202 | 230 | on_plugin_toggled (bottombar); | ||
203 | 231 | }); | ||
204 | 224 | 232 | ||
205 | 225 | hp1 = new Granite.Widgets.ThinPaned (); | 233 | hp1 = new Granite.Widgets.ThinPaned (); |
206 | 226 | hp2 = new Granite.Widgets.ThinPaned (); | 234 | hp2 = new Granite.Widgets.ThinPaned (); |
207 | @@ -287,7 +295,7 @@ | |||
208 | 287 | set_widgets_sensitive (!split_view.is_empty ()); | 295 | set_widgets_sensitive (!split_view.is_empty ()); |
209 | 288 | } | 296 | } |
210 | 289 | 297 | ||
212 | 290 | private void on_plugin_toggled (Gtk.Notebook notebook) { | 298 | private void on_plugin_toggled (Gtk.Notebook notebook) { |
213 | 291 | var pages = notebook.get_n_pages (); | 299 | var pages = notebook.get_n_pages (); |
214 | 292 | notebook.set_show_tabs (pages > 1); | 300 | notebook.set_show_tabs (pages > 1); |
215 | 293 | notebook.no_show_all = (pages == 0); | 301 | notebook.no_show_all = (pages == 0); |
216 | 294 | 302 | ||
217 | === modified file 'src/Widgets/SplitView.vala' | |||
218 | --- src/Widgets/SplitView.vala 2014-10-26 08:26:36 +0000 | |||
219 | +++ src/Widgets/SplitView.vala 2014-11-24 21:01:57 +0000 | |||
220 | @@ -35,6 +35,7 @@ | |||
221 | 35 | public signal void welcome_shown (); | 35 | public signal void welcome_shown (); |
222 | 36 | public signal void welcome_hidden (); | 36 | public signal void welcome_hidden (); |
223 | 37 | public signal void document_change (Scratch.Services.Document document); | 37 | public signal void document_change (Scratch.Services.Document document); |
224 | 38 | public signal void views_changed (uint count); | ||
225 | 38 | 39 | ||
226 | 39 | private weak MainWindow window; | 40 | private weak MainWindow window; |
227 | 40 | 41 | ||
228 | @@ -126,7 +127,6 @@ | |||
229 | 126 | 127 | ||
230 | 127 | // Enbale/Disable useless GtkActions about views | 128 | // Enbale/Disable useless GtkActions about views |
231 | 128 | check_actions (); | 129 | check_actions (); |
232 | 129 | |||
233 | 130 | return view; | 130 | return view; |
234 | 131 | } | 131 | } |
235 | 132 | 132 | ||
236 | @@ -199,6 +199,8 @@ | |||
237 | 199 | private void check_actions () { | 199 | private void check_actions () { |
238 | 200 | window.main_actions.get_action ("NewView").sensitive = (views.length () < 2); | 200 | window.main_actions.get_action ("NewView").sensitive = (views.length () < 2); |
239 | 201 | window.main_actions.get_action ("RemoveView").sensitive = (views.length () > 1); | 201 | window.main_actions.get_action ("RemoveView").sensitive = (views.length () > 1); |
240 | 202 | |||
241 | 203 | views_changed (views.length ()); | ||
242 | 202 | } | 204 | } |
243 | 203 | } | 205 | } |
244 | 204 | } | 206 | } |
245 | 205 | \ No newline at end of file | 207 | \ No newline at end of file |
246 | 206 | 208 | ||
247 | === modified file 'src/Widgets/ToolBar.vala' | |||
248 | --- src/Widgets/ToolBar.vala 2014-10-28 18:20:18 +0000 | |||
249 | +++ src/Widgets/ToolBar.vala 2014-11-24 21:01:57 +0000 | |||
250 | @@ -91,6 +91,7 @@ | |||
251 | 91 | pack_start (revert_button); | 91 | pack_start (revert_button); |
252 | 92 | pack_start (new SeparatorToolItem ()); | 92 | pack_start (new SeparatorToolItem ()); |
253 | 93 | pack_start (find_button); | 93 | pack_start (find_button); |
254 | 94 | pack_start (find_button); | ||
255 | 94 | 95 | ||
256 | 95 | pack_end (share_app_menu); | 96 | pack_end (share_app_menu); |
257 | 96 | pack_end (zoom_default); | 97 | pack_end (zoom_default); |
258 | @@ -106,4 +107,4 @@ | |||
259 | 106 | 107 | ||
260 | 107 | } | 108 | } |
261 | 108 | } | 109 | } |
262 | 109 | } | ||
263 | 110 | \ No newline at end of file | 110 | \ No newline at end of file |
264 | 111 | } |
I would prefer not to set a static px limit here. We should change based on the side of the Line Width Guide and also if there are other panes open like the Folder Browser.