Merge lp:~artem-anufrij/scratch/Bugfix-1032565 into lp:~elementary-apps/scratch/scratch

Proposed by Artem Anufrij
Status: Merged
Approved by: Artem Anufrij
Approved revision: 1409
Merged at revision: 1405
Proposed branch: lp:~artem-anufrij/scratch/Bugfix-1032565
Merge into: lp:~elementary-apps/scratch/scratch
Diff against target: 188 lines (+63/-30)
2 files modified
plugins/terminal/terminal.vala (+57/-29)
src/MainWindow.vala (+6/-1)
To merge this branch: bzr merge lp:~artem-anufrij/scratch/Bugfix-1032565
Reviewer Review Type Date Requested Status
Danielle Foré Needs Fixing
Robert Roth (community) code Approve
elementary UX ux Pending
Review via email: mp+240365@code.launchpad.net

Commit message

Press CTRL+ALT+t for switching between terminal and editor.

Description of the change

Press CTRL+ALT+t for switching between terminal and editor.

To post a comment you must log in.
1403. By artem-anufrij

diconnect key_event on deactivate terminal plug in

1404. By artem-anufrij

improved code design

1405. By artem-anufrij

code design was improved

1406. By artem-anufrij

code design was improved

1407. By artem-anufrij

check if windown != null

Revision history for this message
Robert Roth (evfool) wrote :

Code-wise this looks OK, but we need an UX review based on comment https://bugs.launchpad.net/scratch/+bug/1032565/comments/2 tomake sure they are OK with F12 (I also use that shortcut for my dropdown terminal).

review: Approve (code)
Revision history for this message
Robert Roth (evfool) wrote :

The proposal is almost perfect code-wise, see my only inline comment :)

Revision history for this message
Cody Garver (codygarver) wrote :

I'm concerned about hardcoding this to F12. As commented on the bug report, that's a popular key. Is it possible to make this easily configurable somehow in dconf? Not only for the users, but we (OS team) could even end up reconfiguring this in the future at Design Team's request and a dconf entry is convenient for all who are concerned.

If it's not simple, let's further discuss if F12 is what we really want before merging this.

Revision history for this message
Artem Anufrij (artem-anufrij) wrote :

Hey Cody. Yes, it's possible. We need to create a new settings item, than I can use it for this plug in.

Revision history for this message
Danielle Foré (danrabbit) wrote :

Yeah I'm -1 on using F keys. Especially because on keyboards with media keys this can mean you need to hold down the "Fn" key first. So it's kind of inconvenient.

We should probably be using something Ctrl based. If it's not already being used for something in Scratch, it seems like "Ctrl T" would be the most logical.

review: Needs Fixing
Revision history for this message
Artem Anufrij (artem-anufrij) wrote :

Give me 5 minutes ... :)

1408. By artem-anufrij

Hotkey: CTRL+ALT+t

Revision history for this message
Artem Anufrij (artem-anufrij) wrote :

Hotkey: CTRL+ALT+t

1409. By artem-anufrij

merge lp:scratch

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/terminal/terminal.vala'
2--- plugins/terminal/terminal.vala 2014-08-07 00:33:31 +0000
3+++ plugins/terminal/terminal.vala 2014-11-09 10:53:41 +0000
4@@ -1,21 +1,21 @@
5 // -*- Mode: vala; indent-tabs-mode: nil; tab-width: 4 -*-
6 /***
7 BEGIN LICENSE
8-
9+
10 Copyright (C) 2011-2013 Mario Guerriero <mario@elementaryos.org>
11- This program is free software: you can redistribute it and/or modify it
12- under the terms of the GNU Lesser General Public License version 3, as published
13+ This program is free software: you can redistribute it and/or modify it
14+ under the terms of the GNU Lesser General Public License version 3, as published
15 by the Free Software Foundation.
16-
17- This program is distributed in the hope that it will be useful, but
18- WITHOUT ANY WARRANTY; without even the implied warranties of
19- MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
20+
21+ This program is distributed in the hope that it will be useful, but
22+ WITHOUT ANY WARRANTY; without even the implied warranties of
23+ MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
24 PURPOSE. See the GNU General Public License for more details.
25-
26- You should have received a copy of the GNU General Public License along
27- with this program. If not, see <http://www.gnu.org/licenses/>
28-
29- END LICENSE
30+
31+ You should have received a copy of the GNU General Public License along
32+ with this program. If not, see <http://www.gnu.org/licenses/>
33+
34+ END LICENSE
35 ***/
36
37 using Vte;
38@@ -24,20 +24,46 @@
39 public const string DESCRIPTION = N_("A terminal in your text editor");
40
41 public class Scratch.Plugins.Terminal : Peas.ExtensionBase, Peas.Activatable {
42-
43+
44+ MainWindow window = null;
45 Gtk.Notebook? bottombar = null;
46 Vte.Terminal terminal;
47 Gtk.Grid grid;
48-
49+ ulong? connect_handler;
50+
51 Scratch.Services.Interface plugins;
52 public Object object { owned get; construct; }
53-
54+
55 public void update_state () {
56 }
57
58 public void activate () {
59- plugins = (Scratch.Services.Interface) object;
60- plugins.hook_notebook_bottom.connect ((n) => {
61+ plugins = (Scratch.Services.Interface) object;
62+
63+ plugins.hook_window.connect ((w) => {
64+ if (window != null)
65+ return;
66+
67+ window = w;
68+ connect_handler = window.key_press_event.connect ((event) => {
69+ if (event.keyval == Gdk.Key.t
70+ && Gdk.ModifierType.MOD1_MASK in event.state
71+ && Gdk.ModifierType.CONTROL_MASK in event.state) {
72+ if (terminal.has_focus && window.get_current_document () != null) {
73+ window.get_current_document ().focus ();
74+ debug ("Move focus: EDITOR.");
75+ return true;
76+ } else if (window.get_current_document () != null && window.get_current_document ().source_view.has_focus) {
77+ terminal.grab_focus ();
78+ debug ("Move focus: TERMINAL.");
79+ return true;
80+ }
81+ }
82+ return false;
83+ });
84+ });
85+
86+ plugins.hook_notebook_bottom.connect ((n) => {
87 if (bottombar == null) {
88 this.bottombar = n;
89 on_hook (this.bottombar);
90@@ -50,12 +76,14 @@
91 public void deactivate () {
92 if (terminal != null)
93 grid.destroy ();
94+ if (connect_handler != null)
95+ window.disconnect ((ulong)connect_handler);
96 }
97-
98+
99 void on_hook (Gtk.Notebook notebook) {
100 this.terminal = new Vte.Terminal ();
101 this.terminal.scrollback_lines = -1;
102-
103+
104 // Set terminal font to system default font
105 var system_settings = new GLib.Settings ("org.gnome.desktop.interface");
106 string font_name = system_settings.get_string ("monospace-font-name");
107@@ -117,21 +145,21 @@
108
109 // Popup menu
110 var menu = new Gtk.Menu ();
111-
112+
113 var copy = new Gtk.MenuItem.with_label (_("Copy"));
114 copy.activate.connect (() => {
115 terminal.copy_clipboard ();
116 });
117 menu.append (copy);
118 copy.show ();
119-
120+
121 var paste = new Gtk.MenuItem.with_label (_("Paste"));
122 paste.activate.connect (() => {
123 terminal.paste_clipboard ();
124 });
125 menu.append (paste);
126 paste.show ();
127-
128+
129 this.terminal.button_press_event.connect ((event) => {
130 if (event.button == 3) {
131 menu.select_first (false);
132@@ -139,24 +167,24 @@
133 }
134 return false;
135 });
136-
137+
138 try {
139- this.terminal.fork_command_full (Vte.PtyFlags.DEFAULT, "~/", { Vte.get_user_shell () }, null, GLib.SpawnFlags.SEARCH_PATH, null, null);
140+ this.terminal.fork_command_full (Vte.PtyFlags.DEFAULT, "~/", { Vte.get_user_shell () }, null, GLib.SpawnFlags.SEARCH_PATH, null, null);
141 } catch (GLib.Error e) {
142 warning (e.message);
143 }
144-
145+
146 grid = new Gtk.Grid ();
147 var sb = new Gtk.Scrollbar (Gtk.Orientation.VERTICAL, terminal.vadjustment);
148 grid.attach (terminal, 0, 0, 1, 1);
149 grid.attach (sb, 1, 0, 1, 1);
150-
151+
152 // Make the terminal occupy the whole GUI
153 terminal.vexpand = true;
154 terminal.hexpand = true;
155-
156+
157 notebook.append_page (grid, new Gtk.Label (_("Terminal")));
158-
159+
160 grid.show_all ();
161
162 }
163@@ -167,4 +195,4 @@
164 var objmodule = module as Peas.ObjectModule;
165 objmodule.register_extension_type (typeof (Peas.Activatable),
166 typeof (Scratch.Plugins.Terminal));
167-}
168+}
169\ No newline at end of file
170
171=== modified file 'src/MainWindow.vala'
172--- src/MainWindow.vala 2014-10-30 19:06:19 +0000
173+++ src/MainWindow.vala 2014-11-09 10:53:41 +0000
174@@ -334,8 +334,13 @@
175
176 // Get current document
177 public Scratch.Services.Document? get_current_document () {
178+ Scratch.Services.Document? doc = null;
179+
180 var view = this.get_current_view ();
181- return view.get_current_document ();
182+ if (view != null)
183+ doc = view.get_current_document ();
184+
185+ return doc;
186 }
187
188 // Add new view

Subscribers

People subscribed via source and target branches