Merge lp:~artem-anufrij/scratch/dont-force-to-save-unsaved into lp:~elementary-apps/scratch/scratch
- dont-force-to-save-unsaved
- Merge into scratch
Status: | Merged |
---|---|
Approved by: | Robert Roth |
Approved revision: | 1414 |
Merged at revision: | 1393 |
Proposed branch: | lp:~artem-anufrij/scratch/dont-force-to-save-unsaved |
Merge into: | lp:~elementary-apps/scratch/scratch |
Diff against target: |
746 lines (+236/-186) 7 files modified
HACKING (+1/-1) src/Dialogs/PreferencesDialog.vala (+1/-5) src/MainWindow.vala (+53/-6) src/Scratch.vala (+16/-2) src/Services/Document.vala (+145/-161) src/Widgets/DocumentView.vala (+13/-3) src/Widgets/ToolBar.vala (+7/-8) |
To merge this branch: | bzr merge lp:~artem-anufrij/scratch/dont-force-to-save-unsaved |
Related bugs: | |
Related blueprints: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Robert Roth (community) | code review | Approve | |
Danielle Foré | ux | Approve | |
Review via email: mp+237154@code.launchpad.net |
Commit message
Use temporary files for unsaved files to be able to restore them at next startup
Description of the change
BLUEPRINT DESCRIPTION:
+1 for keeping in .local/
Artem Anufrij (artem-anufrij) wrote : | # |
Please check the fixes.
- temporary location: .local/
- text style was fixed.
Robert Roth (evfool) wrote : | # |
See my inline comments. I'm still not sure the scratch app instance should be made singleton just to be able to access the path variable, that's a second reason why settings-based path was suggested, as the settings object is available globally.
- 1405. By artem-anufrij
-
code fixing
Robert Roth (evfool) wrote : | # |
Ok, looks nice, as per Slack comments the path doesn't have to be configurable, so we're keeping the app instance singleton. Approved.
- 1406. By artem-anufrij
-
temporary typer
Danielle Foré (danrabbit) wrote : | # |
I think if you close the individual tab (as opposed to the whole app) you should be prompted to either save or delete. I don't think we should encourage a workflow where users can perpetually store a bunch of documents in a hidden folder. Additionally, in the current workflow I'm not sure how I'm supposed to discard a document that I've decided I don't actually want to save.
I'm also thinking that we should override the tab/title name for unsaved documents and just call them "unsaved document" or something instead of referring to them as a file name with a location inside the app.
Artem Anufrij (artem-anufrij) wrote : | # |
I have changed the functionality as agreed. Please check this out.
Robert Roth (evfool) wrote : | # |
Code style is mostly ok, see one minor request inline.
- 1410. By artem-anufrij
-
Code style has been improved.
Robert Roth (evfool) wrote : | # |
Code style fixed, approving, waiting for Dan's approval.
Danielle Foré (danrabbit) wrote : | # |
Maybe I'm missing something, but the feature seems to no longer work at all. Trying to close the app with an unsaved document prompts me to save.
Also the window title still reflects the name and location of the file
- 1411. By artem-anufrij
-
Window title fixed
App closing: save 'unsaved' files in ./local/share/... without save dialog
Tab closing: show save dialog for 'unsaved' files
Artem Anufrij (artem-anufrij) wrote : | # |
Hey Dan,
sorry I have overlooked the: "...(as opposed to the whole app)...".
Now it feels smooth.
Danielle Foré (danrabbit) wrote : | # |
Works as expected now. Thanks Artem! This is great :)
Robert Roth (evfool) wrote : | # |
Looks fine, two more indentation issues to fix before the final approval.
- 1414. By artem-anufrij
-
code style fixed
Artem Anufrij (artem-anufrij) wrote : | # |
@Robert,
line 155: this is only code style:
https:/
line 292: fixed
Robert Roth (evfool) wrote : | # |
Ok, Approving. Thanks for the several fixes and your perseverence, it is something of great value.
Preview Diff
1 | === modified file 'HACKING' |
2 | --- HACKING 2013-08-17 21:53:30 +0000 |
3 | +++ HACKING 2014-10-18 19:50:44 +0000 |
4 | @@ -152,4 +152,4 @@ |
5 | bzr branch lp:scratch scratch |
6 | bzr branch scratch/ scratch-fix-123456 |
7 | cd scratch-fix-123456 |
8 | - bzr pull lp:scratch |
9 | + bzr pull lp:scratch |
10 | \ No newline at end of file |
11 | |
12 | === modified file 'src/Dialogs/PreferencesDialog.vala' |
13 | --- src/Dialogs/PreferencesDialog.vala 2014-08-07 09:22:40 +0000 |
14 | +++ src/Dialogs/PreferencesDialog.vala 2014-10-18 19:50:44 +0000 |
15 | @@ -288,10 +288,6 @@ |
16 | var scheme = scheme_manager.get_scheme (scheme_id); |
17 | style_scheme.append (scheme.id, scheme.name); |
18 | } |
19 | - |
20 | - |
21 | } |
22 | - |
23 | } |
24 | - |
25 | -} // Namespace |
26 | +} // Namespace |
27 | \ No newline at end of file |
28 | |
29 | === modified file 'src/MainWindow.vala' |
30 | --- src/MainWindow.vala 2014-10-05 20:56:25 +0000 |
31 | +++ src/MainWindow.vala 2014-10-18 19:50:44 +0000 |
32 | @@ -86,6 +86,9 @@ |
33 | // Restore session |
34 | restore_saved_state_extra (); |
35 | |
36 | + // Crate folder for unsaved documents |
37 | + create_unsaved_documents_directory (); |
38 | + |
39 | #if HAVE_ZEITGEIST |
40 | // Set up the Data Source Registry for Zeitgeist |
41 | registry = new DataSourceRegistry (); |
42 | @@ -181,7 +184,12 @@ |
43 | path = _("Trash"); |
44 | |
45 | path = Uri.unescape_string (path); |
46 | - this.toolbar.title = doc.file.get_basename () + " (%s)".printf(path); |
47 | + |
48 | + string toolbar_title = doc.file.get_basename () + " (%s)".printf (path); |
49 | + if (doc.is_file_temporary) |
50 | + toolbar_title = "(%s)".printf (doc.get_basename ()); |
51 | + |
52 | + this.toolbar.title = toolbar_title; |
53 | } |
54 | else { |
55 | this.toolbar.title = this.app.app_cmd_name; |
56 | @@ -241,14 +249,17 @@ |
57 | |
58 | this.search_revealer.set_reveal_child (false); |
59 | |
60 | + main_actions.get_action ("OpenTemporaryFiles").visible = this.has_temporary_files (); |
61 | main_actions.get_action ("SaveFile").visible = !settings.autosave; |
62 | main_actions.get_action ("Templates").visible = plugins.plugin_iface.template_manager.template_available; |
63 | plugins.plugin_iface.template_manager.notify["template_available"].connect ( () => { |
64 | main_actions.get_action ("Templates").visible = plugins.plugin_iface.template_manager.template_available; |
65 | }); |
66 | |
67 | - // Show welcome by default |
68 | - this.split_view.show_welcome (); |
69 | + if (has_temporary_files ()) |
70 | + action_open_temporary_files (); |
71 | + else |
72 | + this.split_view.show_welcome (); |
73 | |
74 | // Plugins hook |
75 | HookFunc hook_func = () => { |
76 | @@ -268,7 +279,6 @@ |
77 | hook_func (); |
78 | }); |
79 | hook_func (); |
80 | - |
81 | } |
82 | |
83 | private void on_plugin_toggled (Gtk.Notebook notebook) { |
84 | @@ -381,13 +391,25 @@ |
85 | return split_view.is_empty (); |
86 | } |
87 | |
88 | + public bool has_temporary_files () { |
89 | + FileEnumerator enumerator = File.new_for_path (app.data_home_folder_unsaved).enumerate_children (FILE_ATTRIBUTE_STANDARD_NAME, 0, null); |
90 | + var fileinfo = enumerator.next_file (null); |
91 | + while (fileinfo != null) { |
92 | + if (!fileinfo.get_name ().has_suffix ("~")) { |
93 | + return true; |
94 | + } |
95 | + fileinfo = enumerator.next_file (null); |
96 | + } |
97 | + return false; |
98 | + } |
99 | + |
100 | // Check if there no unsaved changes |
101 | private bool check_unsaved_changes () { |
102 | if (!is_empty ()) { |
103 | foreach (var w in this.split_view.views) { |
104 | var view = w as Scratch.Widgets.DocumentView; |
105 | foreach (var doc in view.docs) { |
106 | - if (!doc.close ()) { |
107 | + if (!doc.close (true)) { |
108 | view.set_current_document (doc); |
109 | return false; |
110 | } |
111 | @@ -418,6 +440,16 @@ |
112 | vp.set_position (Scratch.saved_state.vp_size); |
113 | } |
114 | |
115 | + private void create_unsaved_documents_directory () { |
116 | + File directory = File.new_for_path (app.data_home_folder_unsaved); |
117 | + if (!directory.query_exists ()) { |
118 | + debug ("create 'unsaved' directory: %s", directory.get_path ()); |
119 | + directory.make_directory_with_parents (); |
120 | + return; |
121 | + } |
122 | + debug ("'unsaved' directory already exists."); |
123 | + } |
124 | + |
125 | private void update_saved_state () { |
126 | |
127 | // Save window state |
128 | @@ -517,6 +549,21 @@ |
129 | filech.close (); |
130 | } |
131 | |
132 | + void action_open_temporary_files () { |
133 | + FileEnumerator enumerator = File.new_for_path (app.data_home_folder_unsaved).enumerate_children (FILE_ATTRIBUTE_STANDARD_NAME, 0, null); |
134 | + var fileinfo = enumerator.next_file (null); |
135 | + while (fileinfo != null) { |
136 | + if (!fileinfo.get_name ().has_suffix ("~")) { |
137 | + debug ("open temporary file: %s", fileinfo.get_name ()); |
138 | + var file = File.new_for_path (app.data_home_folder_unsaved + fileinfo.get_name ()); |
139 | + var doc = new Scratch.Services.Document (this.main_actions, file); |
140 | + this.open_document (doc); |
141 | + } |
142 | + // Next file info |
143 | + fileinfo = enumerator.next_file (null); |
144 | + } |
145 | + } |
146 | + |
147 | void action_save () { |
148 | this.get_current_document ().save (); |
149 | } |
150 | @@ -694,7 +741,7 @@ |
151 | /* label, accelerator */ N_("Redo"), "<Control><shift>z", |
152 | /* tooltip */ N_("Redo the last undone action"), |
153 | action_redo }, |
154 | - { "Revert", Gtk.Stock.REVERT_TO_SAVED, |
155 | + { "Revert", Gtk.Stock.REVERT_TO_SAVED, |
156 | /* label, accelerator */ N_("Revert"), "<Control><shift>o", |
157 | /* tooltip */ N_("Restore this file"), |
158 | action_revert }, |
159 | |
160 | === modified file 'src/Scratch.vala' |
161 | --- src/Scratch.vala 2014-10-05 21:21:40 +0000 |
162 | +++ src/Scratch.vala 2014-10-18 19:50:44 +0000 |
163 | @@ -34,7 +34,9 @@ |
164 | private GLib.List <MainWindow> windows; |
165 | |
166 | public string app_cmd_name { get { return _app_cmd_name; } } |
167 | + public string data_home_folder_unsaved { get { return _data_home_folder_unsaved; } } |
168 | private static string _app_cmd_name; |
169 | + private static string _data_home_folder_unsaved; |
170 | private static string _cwd; |
171 | private static bool print_version = false; |
172 | private static bool create_new_tab = false; |
173 | @@ -90,6 +92,18 @@ |
174 | services = new ServicesSettings (); |
175 | windows = new GLib.List <MainWindow> (); |
176 | |
177 | + // Init data home folder for unsaved text files |
178 | + _data_home_folder_unsaved = Environment.get_user_data_dir () + "/" + exec_name + "/unsaved/"; |
179 | + } |
180 | + |
181 | + public static ScratchApp _instance = null; |
182 | + |
183 | + public static ScratchApp instance { |
184 | + get { |
185 | + if (_instance == null) |
186 | + _instance = new ScratchApp (); |
187 | + return _instance; |
188 | + } |
189 | } |
190 | |
191 | protected override int command_line (ApplicationCommandLine command_line) { |
192 | @@ -253,8 +267,8 @@ |
193 | return Posix.EXIT_SUCCESS; |
194 | } |
195 | |
196 | - var app = new ScratchApp (); |
197 | + ScratchApp app = ScratchApp.instance; |
198 | return app.run (args_primary_instance); |
199 | } |
200 | } |
201 | -} |
202 | +} |
203 | \ No newline at end of file |
204 | |
205 | === modified file 'src/Services/Document.vala' |
206 | --- src/Services/Document.vala 2014-08-06 05:45:44 +0000 |
207 | +++ src/Services/Document.vala 2014-10-18 19:50:44 +0000 |
208 | @@ -56,15 +56,23 @@ |
209 | private Gtk.InfoBar info_bar = new Gtk.InfoBar (); |
210 | |
211 | // Objects |
212 | - private File? _file = null; |
213 | - public File? file { |
214 | + private File _file = null; |
215 | + public File file { |
216 | get { return _file; } |
217 | - set { _file = value; file_changed (); } |
218 | + set { |
219 | + _file = value; |
220 | + file_changed (); |
221 | + } |
222 | } |
223 | public string original_content; |
224 | public string? last_saved_content = null; |
225 | public bool saved = true; |
226 | private bool error_shown = false; |
227 | + public bool is_file_temporary { |
228 | + get { |
229 | + return file.get_path ().has_prefix (ScratchApp.instance.data_home_folder_unsaved); |
230 | + } |
231 | + } |
232 | |
233 | // It is used to load file content on focusing |
234 | private bool loaded = false; |
235 | @@ -134,43 +142,6 @@ |
236 | |
237 | public async bool open () { |
238 | this.source_view.buffer.create_tag ("highlight_search_all", "background", "yellow", null); |
239 | - if (file == null) { |
240 | - message ("New Document opened"); |
241 | - this.source_view.focus_in_event.connect (() => { |
242 | - main_actions.get_action ("SaveFile").visible = true; |
243 | - check_file_status (); |
244 | - check_undoable_actions (); |
245 | - return false; |
246 | - }); |
247 | - uint timeout_saving = -1; |
248 | - this.source_view.buffer.changed.connect (() => { |
249 | - check_undoable_actions (); |
250 | - // If it wasn't yet saved |
251 | - if (file == null) { |
252 | - this.set_saved_status (false); |
253 | - return; |
254 | - } |
255 | - // Save if autosave is ON |
256 | - if (settings.autosave) { |
257 | - if (timeout_saving >= 0) { |
258 | - Source.remove (timeout_saving); |
259 | - timeout_saving = -1; |
260 | - } |
261 | - timeout_saving = Timeout.add (3000, () => { |
262 | - save (); |
263 | - timeout_saving = -1; |
264 | - return false; |
265 | - }); |
266 | - } |
267 | - else if (!settings.autosave || file == null) |
268 | - this.set_saved_status (false); |
269 | - }); |
270 | - this.set_saved_status (true); |
271 | - this.has_started_loading = true; |
272 | - this.loaded = true; |
273 | - |
274 | - return true; |
275 | - } |
276 | |
277 | // If it does not exists, let's create it! |
278 | if (!exists ()) { |
279 | @@ -227,14 +198,17 @@ |
280 | return true; |
281 | } |
282 | |
283 | - public new bool close () { |
284 | + public new bool close (bool app_closing = false) { |
285 | |
286 | message ("Closing \"%s\"", get_basename ()); |
287 | |
288 | bool ret_value = true; |
289 | - |
290 | + if (app_closing && is_file_temporary && !delete_temporary_file ()) { |
291 | + debug ("Save temporary file!"); |
292 | + this.save (); |
293 | + } |
294 | // Check for unsaved changes |
295 | - if (!this.saved) { |
296 | + else if (!this.saved || (!app_closing && is_file_temporary && !delete_temporary_file ())) { |
297 | debug ("There are unsaved changes, showing a Message Dialog!"); |
298 | |
299 | // Create a GtkDialog |
300 | @@ -265,42 +239,40 @@ |
301 | ret_value = false; |
302 | break; |
303 | case Gtk.ResponseType.YES: |
304 | - this.save (); |
305 | - ret_value = true; |
306 | + if (this.is_file_temporary) |
307 | + this.save_as (); |
308 | + else |
309 | + this.save (); |
310 | break; |
311 | case Gtk.ResponseType.NO: |
312 | - ret_value = true; |
313 | + if (this.is_file_temporary) |
314 | + delete_temporary_file (true); |
315 | break; |
316 | } |
317 | dialog.destroy (); |
318 | - } |
319 | + } |
320 | |
321 | - if (file != null && ret_value) { |
322 | - // Delete backup copy file |
323 | + if (ret_value) { |
324 | + // Delete backup copy file |
325 | delete_backup (); |
326 | + } |
327 | #if HAVE_ZEITGEIST |
328 | - // Zeitgeist integration |
329 | - zg_log.close_insert (file.get_uri (), get_mime_type ()); |
330 | + // Zeitgeist integration |
331 | + zg_log.close_insert (file.get_uri (), get_mime_type ()); |
332 | #endif |
333 | - } |
334 | |
335 | return ret_value; |
336 | } |
337 | |
338 | public bool save () { |
339 | - if (last_saved_content == get_text () && this.file != null) |
340 | + if (last_saved_content == get_text ()) |
341 | return false; |
342 | |
343 | if (!this.loaded) |
344 | return false; |
345 | |
346 | - // Create backup copy file if it does not still exist |
347 | - if (this.file != null) |
348 | - create_backup (); |
349 | - |
350 | - // Show save as dialog if file is null |
351 | - if (this.file == null) |
352 | - return this.save_as (); |
353 | + // Create backup copy file |
354 | + this.create_backup (); |
355 | |
356 | // Replace old content with the new one |
357 | try { |
358 | @@ -308,6 +280,7 @@ |
359 | file.replace_contents (this.source_view.buffer.text.data, null, false, 0, out s); |
360 | } catch (Error e) { |
361 | warning ("Cannot save \"%s\": %s", get_basename (), e.message); |
362 | + return false; |
363 | } |
364 | |
365 | #if HAVE_ZEITGEIST |
366 | @@ -330,6 +303,9 @@ |
367 | // New file |
368 | var filech = Utils.new_file_chooser_dialog (Gtk.FileChooserAction.SAVE, _("Save File"), null); |
369 | |
370 | + string current_file = file.get_path (); |
371 | + bool is_current_file_temporary = this.is_file_temporary; |
372 | + |
373 | if (filech.run () == Gtk.ResponseType.ACCEPT) { |
374 | this.file = File.new_for_uri (filech.get_file ().get_uri ()); |
375 | // Update last visited path |
376 | @@ -341,10 +317,20 @@ |
377 | return false; |
378 | } |
379 | |
380 | - // reset the last saved content |
381 | + // Reset the last saved content |
382 | last_saved_content = null; |
383 | |
384 | - save (); |
385 | + if (save () && is_current_file_temporary) { |
386 | + try { |
387 | + // Delete temporary file |
388 | + File.new_for_path (current_file).delete (); |
389 | + } catch (Error err) { |
390 | + message ("Temporary file cannot be deleted: %s", current_file); |
391 | + } |
392 | + } |
393 | + |
394 | + // Delete backup file |
395 | + delete_backup (current_file + "~"); |
396 | |
397 | // Change syntax highlight |
398 | this.source_view.change_syntax_highlight_from_file (this.file); |
399 | @@ -369,19 +355,15 @@ |
400 | |
401 | // Get mime type for the document |
402 | public string get_mime_type () { |
403 | - if (file == null) |
404 | - return "text/plain"; |
405 | - else { |
406 | - FileInfo info; |
407 | - string mime_type; |
408 | - try { |
409 | - info = file.query_info ("standard::*", FileQueryInfoFlags.NONE, null); |
410 | - mime_type = ContentType.get_mime_type (info.get_attribute_as_string (FileAttribute.STANDARD_CONTENT_TYPE)); |
411 | - return mime_type; |
412 | - } catch (Error e) { |
413 | - warning ("%s", e.message); |
414 | - return "undefined"; |
415 | - } |
416 | + FileInfo info; |
417 | + string mime_type; |
418 | + try { |
419 | + info = file.query_info ("standard::*", FileQueryInfoFlags.NONE, null); |
420 | + mime_type = ContentType.get_mime_type (info.get_attribute_as_string (FileAttribute.STANDARD_CONTENT_TYPE)); |
421 | + return mime_type; |
422 | + } catch (Error e) { |
423 | + warning ("%s", e.message); |
424 | + return "undefined"; |
425 | } |
426 | } |
427 | |
428 | @@ -406,17 +388,15 @@ |
429 | |
430 | // Get file uri |
431 | public string get_uri () { |
432 | - if (file == null) |
433 | - return ""; |
434 | return this.file.get_uri (); |
435 | - } |
436 | + } |
437 | |
438 | // Get file name |
439 | public string get_basename () { |
440 | - if (file != null) |
441 | + if (is_file_temporary) |
442 | + return _("New Document"); |
443 | + else |
444 | return file.get_basename (); |
445 | - else |
446 | - return _("New Document"); |
447 | } |
448 | |
449 | // Set InfoBars message |
450 | @@ -553,7 +533,7 @@ |
451 | if (this.error_shown) |
452 | return; |
453 | this.error_shown = true; |
454 | - string message = _("File \"<b>%s</b>\" cannot be read. Maybe it is corrupt\nor you do not have the necessary permissions to read it.").printf (get_basename ()); |
455 | + string message = _("File \"%s\" cannot be read. Maybe it is corrupt\nor you do not have the necessary permissions to read it.").printf ("<b>%s</b>".printf (get_basename ())); |
456 | var parent_window = source_view.get_toplevel () as Gtk.Window; |
457 | var dialog = new Gtk.MessageDialog.with_markup (parent_window, Gtk.DialogFlags.MODAL, |
458 | Gtk.MessageType.ERROR, |
459 | @@ -566,84 +546,73 @@ |
460 | |
461 | // Check if the file was deleted/changed by an external source |
462 | public void check_file_status () { |
463 | - if (file != null) { |
464 | - // If the file does not exist anymore |
465 | - if (!exists ()) { |
466 | - if (mounted == false) { |
467 | - string message = _("The location containing the file") + " \"<b>%s</b>\" ".printf (get_basename ()) + |
468 | - _("was unmounted. Do you want to save somewhere else?"); |
469 | - |
470 | - set_message (Gtk.MessageType.WARNING, message, _("Save As…"), () => { |
471 | - this.save_as (); |
472 | - hide_info_bar (); |
473 | - }); |
474 | - } else { |
475 | - string message = _("File") + " \"<b>%s</b>\" ".printf (get_basename ()) + |
476 | - _("was deleted. Do you want to save it anyway?"); |
477 | - |
478 | - set_message (Gtk.MessageType.WARNING, message, _("Save"), () => { |
479 | - this.save (); |
480 | - hide_info_bar (); |
481 | - }); |
482 | - } |
483 | - main_actions.get_action ("SaveFile").sensitive = false; |
484 | - this.source_view.editable = false; |
485 | - return; |
486 | - } |
487 | - // If the file can't be written |
488 | - if (!can_write ()) { |
489 | - string message = _("You cannot save changes on file") + " \"<b>%s</b>\". ".printf (get_basename ()) + |
490 | - _("Do you want to save the changes to this file in a different location?"); |
491 | - |
492 | - set_message (Gtk.MessageType.WARNING, message, _("Save changes elsewhere"), () => { |
493 | + // If the file does not exist anymore |
494 | + if (!exists ()) { |
495 | + if (mounted == false) { |
496 | + string message = _("The location containing the file \"%s\" was unmounted. Do you want to save somewhere else?").printf ("<b>%s</b>".printf (get_basename ())); |
497 | + |
498 | + set_message (Gtk.MessageType.WARNING, message, _("Save As…"), () => { |
499 | this.save_as (); |
500 | hide_info_bar (); |
501 | }); |
502 | - main_actions.get_action ("SaveFile").sensitive = false; |
503 | - this.source_view.editable = !settings.autosave; |
504 | - } |
505 | - else { |
506 | - main_actions.get_action ("SaveFile").sensitive = true; |
507 | - this.source_view.editable = true; |
508 | - } |
509 | - // Detect external changes |
510 | - FileHandler.load_content_from_file.begin (file, (obj, res) => { |
511 | - var text = FileHandler.load_content_from_file.end (res); |
512 | - if (text == null) { |
513 | - show_error_dialog (); |
514 | - return; |
515 | - } |
516 | - if (!text.validate()) |
517 | - text = file_content_to_utf8 (file, text); |
518 | - // Reload automatically if auto save is ON |
519 | - if (last_saved_content != null && text != last_saved_content) { |
520 | - if (settings.autosave) |
521 | - this.source_view.set_text (text, false); |
522 | - else { |
523 | - string message = _("File ") + " \"<b>%s</b>\" ".printf (get_basename ()) + |
524 | - _("was modified by an external application. Do you want to load it again or continue your editing?"); |
525 | - |
526 | - set_message (Gtk.MessageType.WARNING, message, _("Load"), () => { |
527 | - this.source_view.set_text (text, false); |
528 | - hide_info_bar (); |
529 | - }, _("Continue"), () => { |
530 | - hide_info_bar (); |
531 | - }); |
532 | - } |
533 | - } |
534 | + } else { |
535 | + string message = _("File \"%s\" was deleted. Do you want to save it anyway?").printf ("<b>%s</b>".printf (get_basename ())); |
536 | + |
537 | + set_message (Gtk.MessageType.WARNING, message, _("Save"), () => { |
538 | + this.save (); |
539 | + hide_info_bar (); |
540 | + }); |
541 | + } |
542 | + main_actions.get_action ("SaveFile").sensitive = false; |
543 | + this.source_view.editable = false; |
544 | + return; |
545 | + } |
546 | + // If the file can't be written |
547 | + if (!can_write ()) { |
548 | + string message = _("You cannot save changes on file \"%s\". Do you want to save the changes to this file in a different location?").printf ("<b>%s</b>".printf (get_basename ())); |
549 | + |
550 | + set_message (Gtk.MessageType.WARNING, message, _("Save changes elsewhere"), () => { |
551 | + this.save_as (); |
552 | + hide_info_bar (); |
553 | }); |
554 | + main_actions.get_action ("SaveFile").sensitive = false; |
555 | + this.source_view.editable = !settings.autosave; |
556 | } |
557 | else { |
558 | main_actions.get_action ("SaveFile").sensitive = true; |
559 | this.source_view.editable = true; |
560 | } |
561 | + // Detect external changes |
562 | + FileHandler.load_content_from_file.begin (file, (obj, res) => { |
563 | + var text = FileHandler.load_content_from_file.end (res); |
564 | + if (text == null) { |
565 | + show_error_dialog (); |
566 | + return; |
567 | + } |
568 | + if (!text.validate ()) |
569 | + text = file_content_to_utf8 (file, text); |
570 | + // Reload automatically if auto save is ON |
571 | + if (last_saved_content != null && text != last_saved_content) { |
572 | + if (settings.autosave) |
573 | + this.source_view.set_text (text, false); |
574 | + else { |
575 | + string message = _("File \"%s\" was modified by an external application. Do you want to load it again or continue your editing?").printf ("<b>%s</b>".printf (get_basename ())); |
576 | + set_message (Gtk.MessageType.WARNING, message, _("Load"), () => { |
577 | + this.source_view.set_text (text, false); |
578 | + hide_info_bar (); |
579 | + }, _("Continue"), () => { |
580 | + hide_info_bar (); |
581 | + }); |
582 | + } |
583 | + } |
584 | + }); |
585 | } |
586 | |
587 | // Set Undo/Redo action sensitive property |
588 | public void check_undoable_actions () { |
589 | main_actions.get_action ("Undo").sensitive = this.source_view.buffer.can_undo; |
590 | main_actions.get_action ("Redo").sensitive = this.source_view.buffer.can_redo; |
591 | - main_actions.get_action ("Revert").sensitive = (file != null && original_content != source_view.buffer.text); |
592 | + main_actions.get_action ("Revert").sensitive = (original_content != source_view.buffer.text); |
593 | } |
594 | |
595 | // Set saved status |
596 | @@ -676,35 +645,55 @@ |
597 | } |
598 | } |
599 | |
600 | - private void delete_backup () { |
601 | - var backup = File.new_for_path (this.file.get_path () + "~"); |
602 | - if (!backup.query_exists ()) |
603 | + private void delete_backup (string? backup_path = null) { |
604 | + |
605 | + string backup_file = ""; |
606 | + |
607 | + if (backup_path == null) |
608 | + backup_file = file.get_path () + "~"; |
609 | + else |
610 | + backup_file = backup_path; |
611 | + |
612 | + debug ("Backup file deleting: %s", backup_file); |
613 | + |
614 | + var backup = File.new_for_path (backup_file); |
615 | + if (backup == null || !backup.query_exists ()) { |
616 | + debug ("Backup file doesn't exists: %s", backup.get_path ()); |
617 | return; |
618 | + } |
619 | try { |
620 | backup.delete (); |
621 | + debug ("Backup file deleted: %s", backup_file); |
622 | } catch (Error e) { |
623 | warning ("Cannot delete backup for file \"%s\": %s", get_basename (), e.message); |
624 | } |
625 | } |
626 | |
627 | + private bool delete_temporary_file (bool force = false) { |
628 | + if (!is_file_temporary || (get_text ().length > 0 && !force)) |
629 | + return false; |
630 | + try { |
631 | + file.delete (); |
632 | + return true; |
633 | + } catch (Error e) { |
634 | + warning ("Cannot delete temporary file \"%s\": %s", file.get_uri (), e.message); |
635 | + } |
636 | + return false; |
637 | + } |
638 | + |
639 | // Return true if the file is writable |
640 | public bool can_write () { |
641 | FileInfo info; |
642 | |
643 | bool writable = false; |
644 | |
645 | - if (this.file == null) |
646 | - return writable = true; |
647 | - |
648 | try { |
649 | info = this.file.query_info (FileAttribute.ACCESS_CAN_WRITE, FileQueryInfoFlags.NONE, null); |
650 | writable = info.get_attribute_boolean (FileAttribute.ACCESS_CAN_WRITE); |
651 | return writable; |
652 | } catch (Error e) { |
653 | - if (this.file != null ) { |
654 | - warning ("query_info failed, but filename appears to be correct, allowing as new file"); |
655 | - writable = true; |
656 | - } |
657 | + warning ("query_info failed, but filename appears to be correct, allowing as new file"); |
658 | + writable = true; |
659 | return writable; |
660 | } |
661 | } |
662 | @@ -715,11 +704,6 @@ |
663 | } |
664 | |
665 | private void file_changed () { |
666 | - if (file == null) { |
667 | - mounted = true; |
668 | - return; |
669 | - } |
670 | - |
671 | if (mount != null) { |
672 | mount.unmounted.disconnect (unmounted_cb); |
673 | mount = null; |
674 | @@ -740,4 +724,4 @@ |
675 | mounted = false; |
676 | } |
677 | } |
678 | -} |
679 | +} |
680 | \ No newline at end of file |
681 | |
682 | === modified file 'src/Widgets/DocumentView.vala' |
683 | --- src/Widgets/DocumentView.vala 2014-06-02 16:22:42 +0000 |
684 | +++ src/Widgets/DocumentView.vala 2014-10-18 19:50:44 +0000 |
685 | @@ -85,11 +85,21 @@ |
686 | |
687 | show_all (); |
688 | } |
689 | + |
690 | + private string unsaved_file_path_builder () { |
691 | + DateTime timestamp = new DateTime.now_local (); |
692 | + string new_text_file = _("Text file from ") + timestamp.format ("%Y-%m-%d %H:%M:%S"); |
693 | + |
694 | + return ScratchApp.instance.data_home_folder_unsaved + new_text_file; |
695 | + } |
696 | |
697 | public void new_document () { |
698 | - var doc = new Document (window.main_actions); |
699 | + File file = File.new_for_path (unsaved_file_path_builder ()); |
700 | + file.create (FileCreateFlags.PRIVATE); |
701 | + |
702 | + var doc = new Document (window.main_actions, file); |
703 | doc.create_page (); |
704 | - |
705 | + |
706 | this.notebook.insert_tab (doc, -1); |
707 | this.notebook.current = doc; |
708 | |
709 | @@ -220,4 +230,4 @@ |
710 | } |
711 | } |
712 | } |
713 | -} |
714 | +} |
715 | \ No newline at end of file |
716 | |
717 | === modified file 'src/Widgets/ToolBar.vala' |
718 | --- src/Widgets/ToolBar.vala 2014-08-11 17:35:47 +0000 |
719 | +++ src/Widgets/ToolBar.vala 2014-10-18 19:50:44 +0000 |
720 | @@ -39,19 +39,18 @@ |
721 | public AppMenu app_menu; |
722 | |
723 | public Toolbar (Gtk.ActionGroup main_actions) { |
724 | - |
725 | // Toolbar properties |
726 | // compliant with elementary HIG |
727 | get_style_context ().add_class ("primary-toolbar"); |
728 | |
729 | // Create ToolButtons |
730 | - open_button = main_actions.get_action ("Open").create_tool_item() as Gtk.ToolButton; |
731 | - templates_button = main_actions.get_action ("Templates").create_tool_item() as Gtk.ToolButton; |
732 | - save_button = main_actions.get_action ("SaveFile").create_tool_item() as Gtk.ToolButton; |
733 | - save_as_button = main_actions.get_action ("SaveFileAs").create_tool_item() as Gtk.ToolButton; |
734 | - revert_button = main_actions.get_action ("Revert").create_tool_item() as Gtk.ToolButton; |
735 | - find_button = main_actions.get_action ("Fetch").create_tool_item() as Gtk.ToolButton; |
736 | - |
737 | + open_button = main_actions.get_action ("Open").create_tool_item () as Gtk.ToolButton; |
738 | + templates_button = main_actions.get_action ("Templates").create_tool_item () as Gtk.ToolButton; |
739 | + save_button = main_actions.get_action ("SaveFile").create_tool_item () as Gtk.ToolButton; |
740 | + save_as_button = main_actions.get_action ("SaveFileAs").create_tool_item () as Gtk.ToolButton; |
741 | + revert_button = main_actions.get_action ("Revert").create_tool_item () as Gtk.ToolButton; |
742 | + find_button = main_actions.get_action ("Fetch").create_tool_item () as Gtk.ToolButton; |
743 | + |
744 | // Create Share and AppMenu |
745 | share_menu = new Gtk.Menu (); |
746 | share_app_menu = new Granite.Widgets.ToolButtonWithMenu (new Image.from_icon_name ("document-export", IconSize.MENU), _("Share"), share_menu); |
Code looks mostly OK, here are some notes: folder_ unsaved seems to be the only reason you're making the Scratch instance a singleton, but that variable should be part of settings (e.g I want to configure that in dconf - or scratch Preferences if Dan allows - to use /tmp instead of my home dir) friendly, and there are more like this. Translatable strings should not be concatenated. ").printf ("<b>%s</b>".printf (get_basename ())).
* Singleton is not needed here, accessing data_home_
* string message = _("File ") + " \"<b>%s</b>\" ".printf (get_basename ()) + _("was modified by an external application. Do you want to load it again or continue your editing?");
This is not translation-
It can also be written like: _("File %s was modified by an external application.
I see that there are some of these coming from before your commits, so it's not 100% your code, but they should be fixed too (not necessarily on this branch), but at least the code we touch should get better after each touch.
* Add a description and a commit message to the merge proposal
I haven't tested this yet, this is just a code review, will test later, with the fixes.