#596 #501 Auto-save untitled buffers - patch

[Hrotkó Gábor]

I created a patch for the feature request: "#501 Auto-save untitled buffers".
Altough the settings "Autosave untitled buffers" exists, but not working like described in the feature request.
My patch extends this feature, when "Autosave untitled buffers" checked (default):
- the untitled buffers will be autosaved when closing jedit, without a question prompt
- the autosave for an untitled buffer saved to <jedit_settings_dir>/autosave, and reloaded after a restart (the state persisted to the perspective.xml)
- closing an untitled buffer by hand removes the autosave file</jedit_settings_dir>

Tested on windows7 and linux also.
The patch is against revision 24609.
Please review.
Thanks!

[Alan Ezust]

Be sure to update to r24713 because I made more changes to the code that you modified previously.

[Alan Ezust]

It seems something else that has changed since jEdit 5.4 is the creation of a new untitled buffer when projectviewer changes projects (in the case where only project files were opened, and we have close open project files on project change, and also remember open files enabled).
There is a thread on jedit-users now subject: "Project Viewer opens an empty buffer"
I am not yet sure if it is your code or mine.

[Hrotkó Gábor]

I created a patch for "Project Viewer opens an empty buffer".
For some reason, I modified BufferSet.addBufferAt, but it can be reverted, and cured this problem.

For untitled backup: "when an autosave untitled is restored from backup", I need a little clarification. I do not see that any backup is saved for untitled buffers. Should we have backups also for untitled buffers?

And last:
me: if the backup, autosave dir set: new untitled buffers will be saved to that location
you: I don't want that setting to change jEdit's behavior when creating new untitled buffers.

Can we have an option like:
"Save untitled buffer to Autosave directory if Autosave directory is set" default off
with tooltip:
"When this option is checked, and the Autosave directory is set, save the untitled buffer to that directory.
Otherwise, it will be saved to the same place as the currently opened buffer (at the time when the untitled buffer was created)."

just for my sake? :-)

[Alan Ezust]

What i meant to say is "when an autosaved untitled is restored from the backup/autosave directory, its previous path should be restored, instead of the path to the autosave/backup location."

If you really want to add a checkbox, I'm still not sure of its title yet. Let's get previous behavior restored properly before we add new options.

[Alan Ezust]

Why would someone want untitled autosaves to be stored differently from regular autosaves?

Is having separate options for backup vs autosave directories more appropriate?

[Hrotkó Gábor]

I had some misunderstanding of your wording lastly, but now I understand all requirements, and created the patch for that.
The only "big" change is that if no autosave/backup directory specified, the backup dir will be: jedit settings dir + backup, to separate those files.
Please test this patch.

[Alan Ezust]

I imagine some users will want autosaves/backups in the same directory, and others will not.

The current documentation and the feature requests I am trying to implement are trying to make both kinds of users happy.

If no autosave/backup is specified, it should use the same directory as the original file!

If an autosave/backup is specified, then the files are separate, and we can specify the location.

Why do you want to make things more complicated than that? And why do you want to hard-code the location of the default?

[Alan Ezust]

It sounds like the option you want is "location of new untitled buffers", regardless of autosave/backup dir.

[Alan Ezust]

I don't like this patch. You are adding a new field to the perspective when it is not necessary. Just save the correct location of the original UNTITLED buffer in the perspective, and then SEARCH for autosaves checking where getAutosaveDirectory() tells you to go for that buffer.
I don't like untitled buffers being autosaved in a different way from regular autosaves.

[Hrotkó Gábor]

Ok, from your perspective, what is left to be done for this feature to be accepted?

[Alan Ezust]

I just want to see the correct buffer path saved and restored from the perspective, untitled or not when autosave is used.
I recommend you make a feature request to describe what other thing you want.

[Hrotkó Gábor]

I think, now the code in trunk, restores the correct path. I tested it now.
We can forget my feature request.

[Alan Ezust]

Trunk does not save the correct location of the untitled buffer to the perspective.
Instead it saves the location of the backup/autosave directory (not necessary since it is already obtained from properties), so the original untitled buffer path is lost.

[Hrotkó Gábor]

I tested your requirement, and the full behavior is only possible with my patch: saveUntitled_24718.patch.
I tried to prepare another patch with less code modifications, but there is a problem: if you change the backup/autosave while creating untitled buffers, you cannot get the right paths back without storing it into the perspective.
Please try to create an untitled buffer, then restart, then change the backup/autosave path, then create another untitled buffer, then restart ...
That is why I created my patch that way.

[Alan Ezust]

The problem you are trying to solve: allowing for the user to change autosave directories and using the old one for existing buffers, is not a solution that works for non-untitled buffers.
Personally, I don't think it is an important problem to solve - when the autosave happens, it could read the autosave directory again from the properties, and then use the latest location for subsequent autosaves. Then it would work the same way as it does for non-untitled buffers, and after 30 seconds you don't care what the old value was anymore.

[Alan Ezust]

I am testing your patch and so far, it seems to work as I asked as long as I don't look into the perspective file :-)

[Alan Ezust]

There are a lot of public functions you've added to jEdit's API just to make it possible to treat untitled buffers differently from regular ones. I personally think many of them can be removed if we store untitled autosaves the same way as regular autosaves.
But for the ones that can not, you pasted the @since from the original function, and that is not correct. They need to have @since jEdit 5.5pre1 in the javadoc comments.

[Hrotkó Gábor]

Thanx for the $JEDIT_SETTINGS commit, after that I could create my patch to modify the behaviour to match the desired. I also removed the methods that we do need to handle untitled autsaves separately.
I think changing the bacup/autosave dir while running is an important thing, because it can lead to data loss. So I also added the feature to move the autosaves to the correct location, when the user change the bacup/autosave dir.
Please test the attached patch.

[Alan Ezust]

Thank you for following up on this. After a quick read of the .diff I have this comment first. I will apply and test it shortly.

Adding a new jEdit API method "getSettingsOrBackupDirectory()" is wrong and unnecessary.

MiscUtilities.prepareAutosaveDirectory() and prepareBackupDirectory() are separate API functions at the moment, although they are used from different parts of jEdit code, and it allows for a different location to be set for the autosave vs backup directory. If you wanna combine the two concepts into one, then we should be able to SIMPLIFY the API, instead of making it more complex.

We can add a getBackupDirectory() function to MiscUtilities and then call that from prepareBackupDirectory().

Then every call goes through a single function for getting that location.

We can remove prepareAutosaveDirectory() from MiscUtilities entirely and call prepareBackupDirectory() instead.

And finally, do NOT set any default if the property is empty. "empty property" behavior is to use the SAME directory as the original buffer.

I want to avoid adding new API to jEdit whenever possible.

[Alan Ezust]

This changes the behavior of jEdit:

@@ -1933,8 +1927,7 @@
{
path = editPane.getBuffer().getDirectory();
} else {
- File autosaveDir = MiscUtilities.prepareAutosaveDirectory(System.getProperty("user.home"), true);
- path = autosaveDir.getPath();
+ path = (new File(jEdit.getBackupOrSettingsDirectory())).getPath();
}
VFS vfs = VFSManager.getVFSForPath(path);

The idea behind this code was to pass user.home as the "default location" for the first new buffer, in case there is no buffer open at all.

[Alan Ezust]

Those changes to the savebackupOptionPane should not be necessary.
First of all, the getAutosaveDirecotry() method seems to be doing the expandVariables() anyway, and if you call that method from the right places that need it, you shouldn't need to call miscUtilities.expandVariables() anywhere else.

Second, if I set the backup directory to a variable-prefix value from option pane, there is no reason to expand it before setting it back to properties.

[Hrotkó Gábor]

I made changes, according to your comments. Please review.

[Alan Ezust]

Very nice! Testing now.

[Alan Ezust]

Committed revision 24725.
Thank you for your very fine work! It is a pleasure working with you.
I hope you can find other things about jEdit you wanna work on :-)