Hi all, I've just made a commit which changes plugin API behaviour, which I think is necessary. Please see the full log message below for justification.
It's possible plugins may need updating, but after a quick grep it seems perhaps only the core Save Actions plugin. For auto-saving it may want to ignore documents without an absolute path to avoid showing the Save As dialog. I'll make this change next.
Note that file templates and other features (geanyVC?) create documents with a filename but no path, hence this change. Remembering to show the Save As dialog in all code for the same reason was not reliable enough. That code can now be removed, but won't cause users any problems if left.
On 02/11/2011 15:27, Nick Treleaven wrote:
Branch: refs/heads/master Home: https://github.com/geany/geany
Commit: de559ef5d4150e2485ef3ffb865e9c9d3249bcd8 https://github.com/geany/geany/commit/de559ef5d4150e2485ef3ffb865e9c9d3249bc... Author: Nick Treleavennick.treleaven@btinternet.com Date: 2011-11-02 (Wed, 02 Nov 2011)
Changed paths: M src/build.c M src/callbacks.c M src/dialogs.c M src/document.c
Log Message:
Make document_save_file() show the Save As dialog when necessary
Previously an error message was shown if doc->file_name is NULL.
The Save As dialog is now shown if the document does not have an absolute path. This is because the user should confirm where to save the document in this case.
Although this changes plugin API behaviour, it seems the best way to ensure the Save As dialog is always shown when needed so the user knows where the document has been saved.
On 02/11/2011 15:44, Nick Treleaven wrote:
Remembering to show the Save As dialog in all code for the same reason was not reliable enough. That code can now be removed, but won't cause users any problems if left.
BTW the only plugin that seems to use dialogs_show_save_as() is geanysendmail - I'll make a patch to remove the now unnecessary code.
Hi Nick,
Good idea, I have one question below.
[...]
It's possible plugins may need updating, but after a quick grep it seems perhaps only the core Save Actions plugin. For auto-saving it may want to ignore documents without an absolute path to avoid showing the Save As dialog. I'll make this change next.
Does this skip any documents that the user might reasonably expect to autosave?
Obviously it skips new as yet unsaved files, but IMHO thats ok, is there anything else?
If so we might want to indicate the fact that this file isn't being autosaved to warn that there is a risk of data loss. (maybe with new files too?)
Cheers Lex
[...]
On 02/11/2011 23:22, Lex Trotman wrote:
Hi Nick,
Good idea, I have one question below.
[...]
It's possible plugins may need updating, but after a quick grep it seems perhaps only the core Save Actions plugin. For auto-saving it may want to ignore documents without an absolute path to avoid showing the Save As dialog. I'll make this change next.
Does this skip any documents that the user might reasonably expect to autosave?
Obviously it skips new as yet unsaved files, but IMHO thats ok, is there anything else?
Well, as documents that have been saved now have to have an absolute path, I don't think it would skip saving any document that exists on disk. Although I'd better check that opening documents enforces an absolute path too, I think so.
The autosave check could actually be against doc->real_path, which is only set if the file exists on disk, rather than checking for an absolute path.
If so we might want to indicate the fact that this file isn't being autosaved to warn that there is a risk of data loss. (maybe with new files too?)
For modified documents the user can see this already, but for generated documents that don't set the modified flag data could be lost, but this probably tends to be things which the user isn't concerned about. E.g. with a geanyVC diff, you usually want to just close the document and ignore the data.
On 4 November 2011 01:17, Nick Treleaven nick.treleaven@btinternet.com wrote:
On 02/11/2011 23:22, Lex Trotman wrote:
Hi Nick,
Good idea, I have one question below.
[...]
It's possible plugins may need updating, but after a quick grep it seems perhaps only the core Save Actions plugin. For auto-saving it may want to ignore documents without an absolute path to avoid showing the Save As dialog. I'll make this change next.
Does this skip any documents that the user might reasonably expect to autosave?
Obviously it skips new as yet unsaved files, but IMHO thats ok, is there anything else?
Well, as documents that have been saved now have to have an absolute path, I don't think it would skip saving any document that exists on disk. Although I'd better check that opening documents enforces an absolute path too, I think so.
I think so to, but I did get a bit lost and I'm not sure of the implications for GVFS-FUSE mounted remote files, do they have an absolute or real path? Flakey remote connections are of course the most likely place where autosave could save your bacon.
The autosave check could actually be against doc->real_path, which is only set if the file exists on disk, rather than checking for an absolute path.
Question as above.
[...]
For modified documents the user can see this already, but for generated documents that don't set the modified flag data could be lost, but this probably tends to be things which the user isn't concerned about. E.g. with a geanyVC diff, you usually want to just close the document and ignore the data.
Yes, if its generated, presumably it can be re-generated.
Cheers Lex