Merge lp:~superscript18/scratch/bugfix-1390607 into lp:~elementary-apps/scratch/scratch

Proposed by SuperScript
Status: Rejected
Rejected by: Cody Garver
Proposed branch: lp:~superscript18/scratch/bugfix-1390607
Merge into: lp:~elementary-apps/scratch/scratch
Diff against target: 220 lines (+59/-81)
3 files modified
data/scratch-text-editor.desktop (+1/-1)
src/MainWindow.vala (+2/-46)
src/Scratch.vala (+56/-34)
To merge this branch: bzr merge lp:~superscript18/scratch/bugfix-1390607
Reviewer Review Type Date Requested Status
Jeremy Wootten Needs Fixing
Danielle Foré Needs Fixing
Review via email: mp+250651@code.launchpad.net

This proposal supersedes a proposal from 2015-02-10.

Description of the change

I have reworked the logic in launching Scratch. This new method is quite a bit cleaner and fixes several problems:

1. Scratch opens temporary (unsaved) documents when you open a second window. This is a pain because they are opened twice and cannot be closed in either instance without losing data.

2. When you opened a new window in Scratch, the previous window would try to get your attention (`window.present ()`) resulting in an unneeded red icon in Plank.

3. Middle-clicking on the Scratch icon in Plank did nothing. Now it should open a blank new window ready for you to work.

I've included elementary-design to decide if all this functionality is correct.

This fixes bugs:
Bug #1420549: middle-click on Plank does nothing
Bug #1390607: don't open unsaved files in new windows

To post a comment you must log in.
Revision history for this message
Danielle Foré (danrabbit) wrote :

* Segfault when closing a second new window with nothing open while having a file open in the first window

*Can confirm that unsaved files don't open in new windows.

review: Needs Fixing
Revision history for this message
SuperScript (superscript18) wrote :

@DanielFore: I can't reproduce that SEGFAULT. Can you post some more detail instructions?

Also, should we have Pantheon Files open a new window by default, too?

Revision history for this message
Jeremy Wootten (jeremywootten) wrote :

Conflicts with trunk r1618 and does not compile with vala-0.28.

After fixing conflicts compiles on valac 0.28. Note careful resolving of conflict in Scratch.vala required.

Using the merged branch:

Cannot reproduce bug 1390607 in latest trunk (or in this branch).

New window on middle click on Plank icon works.

Could not reproduce crash mentioned by Dan.

See inline comment regarding unhandled errors and spelling

Needs fixing but potentially mergeable.

review: Needs Fixing

Unmerged revisions

1451. By SuperScript

 isn't needed anymore.

1450. By SuperScript

fixed tab/space to match elementary guidelines

1449. By SuperScript

Redid Scratch's startup procedure

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'data/scratch-text-editor.desktop'
--- data/scratch-text-editor.desktop 2014-04-12 23:26:29 +0000
+++ data/scratch-text-editor.desktop 2015-02-23 16:40:09 +0000
@@ -3,7 +3,7 @@
3Name=Scratch3Name=Scratch
4Comment=Edit text files4Comment=Edit text files
5GenericName=Text Editor5GenericName=Text Editor
6Exec=scratch-text-editor %U6Exec=scratch-text-editor --new-window %U
7Icon=accessories-text-editor7Icon=accessories-text-editor
8Terminal=false8Terminal=false
9Categories=GNOME;GTK;Utility;TextEditor;9Categories=GNOME;GTK;Utility;TextEditor;
1010
=== modified file 'src/MainWindow.vala'
--- src/MainWindow.vala 2015-01-22 21:59:26 +0000
+++ src/MainWindow.vala 2015-02-23 16:40:09 +0000
@@ -94,9 +94,6 @@
94 // Restore session94 // Restore session
95 restore_saved_state_extra ();95 restore_saved_state_extra ();
9696
97 // Crate folder for unsaved documents
98 create_unsaved_documents_directory ();
99
100#if HAVE_ZEITGEIST97#if HAVE_ZEITGEIST
101 // Set up the Data Source Registry for Zeitgeist98 // Set up the Data Source Registry for Zeitgeist
102 registry = new DataSourceRegistry ();99 registry = new DataSourceRegistry ();
@@ -263,17 +260,13 @@
263260
264 this.search_revealer.set_reveal_child (false);261 this.search_revealer.set_reveal_child (false);
265262
266 main_actions.get_action ("OpenTemporaryFiles").visible = this.has_temporary_files ();
267 main_actions.get_action ("SaveFile").visible = !settings.autosave;263 main_actions.get_action ("SaveFile").visible = !settings.autosave;
268 main_actions.get_action ("Templates").visible = plugins.plugin_iface.template_manager.template_available;264 main_actions.get_action ("Templates").visible = plugins.plugin_iface.template_manager.template_available;
269 plugins.plugin_iface.template_manager.notify["template_available"].connect ( () => {265 plugins.plugin_iface.template_manager.notify["template_available"].connect ( () => {
270 main_actions.get_action ("Templates").visible = plugins.plugin_iface.template_manager.template_available;266 main_actions.get_action ("Templates").visible = plugins.plugin_iface.template_manager.template_available;
271 });267 });
272268
273 if (has_temporary_files ())269 this.split_view.show_welcome ();
274 action_open_temporary_files ();
275 else
276 this.split_view.show_welcome ();
277270
278 // Plugins hook271 // Plugins hook
279 HookFunc hook_func = () => {272 HookFunc hook_func = () => {
@@ -422,18 +415,6 @@
422 return split_view.is_empty ();415 return split_view.is_empty ();
423 }416 }
424417
425 public bool has_temporary_files () {
426 FileEnumerator enumerator = File.new_for_path (app.data_home_folder_unsaved).enumerate_children (FILE_ATTRIBUTE_STANDARD_NAME, 0, null);
427 var fileinfo = enumerator.next_file (null);
428 while (fileinfo != null) {
429 if (!fileinfo.get_name ().has_suffix ("~")) {
430 return true;
431 }
432 fileinfo = enumerator.next_file (null);
433 }
434 return false;
435 }
436
437 // Check if there no unsaved changes418 // Check if there no unsaved changes
438 private bool check_unsaved_changes () {419 private bool check_unsaved_changes () {
439 if (!is_empty ()) {420 if (!is_empty ()) {
@@ -471,16 +452,6 @@
471 vp.set_position (Scratch.saved_state.vp_size);452 vp.set_position (Scratch.saved_state.vp_size);
472 }453 }
473454
474 private void create_unsaved_documents_directory () {
475 File directory = File.new_for_path (app.data_home_folder_unsaved);
476 if (!directory.query_exists ()) {
477 debug ("create 'unsaved' directory: %s", directory.get_path ());
478 directory.make_directory_with_parents ();
479 return;
480 }
481 debug ("'unsaved' directory already exists.");
482 }
483
484 private void update_saved_state () {455 private void update_saved_state () {
485456
486 // Save window state457 // Save window state
@@ -648,21 +619,6 @@
648 filech.close ();619 filech.close ();
649 }620 }
650621
651 void action_open_temporary_files () {
652 FileEnumerator enumerator = File.new_for_path (app.data_home_folder_unsaved).enumerate_children (FILE_ATTRIBUTE_STANDARD_NAME, 0, null);
653 var fileinfo = enumerator.next_file (null);
654 while (fileinfo != null) {
655 if (!fileinfo.get_name ().has_suffix ("~")) {
656 debug ("open temporary file: %s", fileinfo.get_name ());
657 var file = File.new_for_path (app.data_home_folder_unsaved + fileinfo.get_name ());
658 var doc = new Scratch.Services.Document (this.main_actions, file);
659 this.open_document (doc);
660 }
661 // Next file info
662 fileinfo = enumerator.next_file (null);
663 }
664 }
665
666 void action_save () {622 void action_save () {
667 this.get_current_document ().save ();623 this.get_current_document ().save ();
668 }624 }
669625
=== modified file 'src/Scratch.vala'
--- src/Scratch.vala 2014-11-29 11:46:53 +0000
+++ src/Scratch.vala 2015-02-23 16:40:09 +0000
@@ -125,22 +125,56 @@
125125
126 return Posix.EXIT_FAILURE;126 return Posix.EXIT_FAILURE;
127 }127 }
128128
129 bool is_app_launch = (get_last_window () == null);129 var window = get_last_window ();
130130
131 // Create (or show) the first window131 bool is_app_launch = (window == null);
132 activate ();132
133133 if (create_new_window || is_app_launch) {
134 // Create a next window if requested and it's not the app launch
135 if (create_new_window && !is_app_launch) {
136 create_new_window = false;134 create_new_window = false;
137 this.new_window ();135 window = this.new_window ();
136 }
137
138 // Crate folder for unsaved documents
139 create_unsaved_documents_directory ();
140
141 if (is_app_launch) {
142 // restore opened documents
143 if (settings.show_at_start == "last-tabs") {
144 window.start_loading ();
145
146 string[] uris = settings.schema.get_strv ("opened-files");
147
148 foreach (string uri in uris) {
149 if (uri != "") {
150 var file = File.new_for_uri (uri);
151 if (file.query_exists ()) {
152 var doc = new Scratch.Services.Document (window.main_actions, file);
153 window.open_document (doc);
154 }
155 }
156 }
157 window.stop_loading ();
158 }
159
160 // restore temporary files
161 FileEnumerator enumerator = File.new_for_path (data_home_folder_unsaved).enumerate_children (FILE_ATTRIBUTE_STANDARD_NAME, 0, null);
162 var fileinfo = enumerator.next_file (null);
163 while (fileinfo != null) {
164 if (!fileinfo.get_name ().has_suffix ("~")) {
165 debug ("open temporary file: %s", fileinfo.get_name ());
166 var file = File.new_for_path (data_home_folder_unsaved + fileinfo.get_name ());
167 var doc = new Scratch.Services.Document (window.main_actions, file);
168 window.open_document (doc);
169 }
170 // Next file info
171 fileinfo = enumerator.next_file (null);
172 }
138 }173 }
139174
140 // Create a new document if requested175 // Create a new document if requested
141 if (create_new_tab) {176 if (create_new_tab) {
142 create_new_tab = false;177 create_new_tab = false;
143 var window = get_last_window ();
144 window.main_actions.get_action ("NewTab").activate ();178 window.main_actions.get_action ("NewTab").activate ();
145 }179 }
146180
@@ -162,32 +196,20 @@
162 return Posix.EXIT_SUCCESS;196 return Posix.EXIT_SUCCESS;
163 }197 }
164198
199 private void create_unsaved_documents_directory () {
200 File directory = File.new_for_path (data_home_folder_unsaved);
201 if (!directory.query_exists ()) {
202 debug ("create 'unsaved' directory: %s", directory.get_path ());
203 directory.make_directory_with_parents ();
204 return;
205 }
206 debug ("'unsaved' directory already exists.");
207 }
208
165 protected override void activate () {209 protected override void activate () {
166 var window = get_last_window ();210 var window = get_last_window();
167 if (window == null) {211 if (window != null)
168 window = this.new_window ();
169 window.show ();
170 // Restore opened documents
171 if (settings.show_at_start == "last-tabs") {
172 window.start_loading ();
173
174 string[] uris = settings.schema.get_strv ("opened-files");
175
176 foreach (string uri in uris) {
177 if (uri != "") {
178 var file = File.new_for_uri (uri);
179 if (file.query_exists ()) {
180 var doc = new Scratch.Services.Document (window.main_actions, file);
181 window.open_document (doc);
182 }
183 }
184 }
185 window.stop_loading ();
186 }
187 } else {
188 window.present ();212 window.present ();
189 }
190
191 }213 }
192214
193 protected override void open (File[] files, string hint) {215 protected override void open (File[] files, string hint) {

Subscribers

People subscribed via source and target branches