[Feature Request] PsiStudio prompt save when exiting if any changes to dataset are made.

[xiangzhi]

Currently, if you make any changes to a Dataset in PsiStudio (e.g. deleting a partition) and quit PsiStudio. No changes are actually saved. While it is not a breaking issue (You just have to remember to save it manually), it will be nice if PsiStudio will prompt this and let users choose between (1) saving and replacing the original Dataset or (2) just remind me that changes were made and needed to be saved. A bonus feature would be the ability to choose between save and save as.

Since I kept on accidentally forgetting to save it myself and have to redo all my changes, I'd be happy to work on this on my time. Let me know what you all think!

[danbohus]

Yes, it's certainly a missing feature, and we'd welcome a PR on it. One related thought I've had would be to actually implement an auto-save functionality (would help in the scenarios I've been running into). So I think the options are:

1. Implement save functionality as you described
2. Implement auto-save (e.g. each time you add/remove a session or partition the dataset autosaves transparently w/o you having to do anything)
3. Implement both, with the ability to turn the autosave behavior on via a configurable property in the xml settings file.

I think in the long run it would be nice to have 3, but we would certainly welcome a PR on any of these options. Let us know if/which you decide to pursue and we can fill in the rest.

One additional comment if you go down the path of 1. Some "would-be-nices" are:

• display a "*" next to the dataset name when the dataset is in a "modified state" (I think this happens currently for annotation partitions that have been edited but not yet saved/written)
• adding a "Save" button on the toolbar (the current button is a save-as)

[xiangzhi]

Cool, I'm glad it is not just a bug on my end.

I'll focus on Option (2) -- autosave with a setting option in PsiStudioSetting.xml as the starting point. I got a bunch of custom visualizers and it will be nice to still save progress if they were to crash.

Here are my current thoughts on how I would implement it and its behavior:

1. The default will be off and would needed to be enabled through PsiStudioSetting.xml.
2. I'm leaning towards implementing two ways of triggering the autosave.
   • Any actions that changes the Dataset.
   • An optional timer as a backup just in case we forget to implement the hooks for certain functions.

Like you mentioned, the ideal state would be to implement both, so I'll keep that in mind when I make my changes.

I'll keep y'all posted in this issue if I make any progress or run into any walls.

[danbohus]

I'm not sure if you were planning to do this, and I haven't thought through all the implications in detail, but I wonder if the implementation shouldn't actually be done at the Dataset level, rather than in PsiStudio, e.g. Dataset takes an optional autoSaveChanges parameter (by default false) in the constructor and it does the save if required in all the API calls that touch it. PsiStudio then simply passes in a value for this parameter when it opens/creates the Dataset. The benefits would be (1) that this functionality would be available not just in PsiStudio, but also anywhere else someone is using datasets programmatically. Also, (2) it seems better from a design standpoint to have the functionality contained closer to the object it affects -- so someone does not for instance later add a new way of manipulating datasets in PsiStudio, but forgets about autosave-related issues. The potential drawback would be that a Save operation (since it involves IO) and therefore adding a session, etc. could be slow, which would mean that all these APIs for modifying the dataset would ideally have async versions as well (but we can probably worry about / add that later).

Re: timer, I'd be a bit weary of that, just seems like it could add an additional layer of concurrency concerns (what if someone adds a partition in the middle of the timer firing) ... And not clear why it would be necessary if we do the right implementation on the Dataset.

[xiangzhi]

Setting the implementation on the Dataset level seemed to be a good idea, epsecially if there is already a parameter there. However, I think the autoSaveChanges parameter might be in an internal build? I wasn't able to find it in the public repo..

I just looked more deeply at Dataset.cs and Session.cs. I think it's should be pretty straightforward except for the fact that a dataset object can be initialized without an associated save path. So, to implement autosave, it probably needs a check and keep its own path. We can also have a change flag to mark whether any changes have been made and needed to save permanently. Let me know if such change has actually been made internally

Timers - yeah, Concurrency issue and bugs are definitely going to show up. I was thinking of it like those recovery copies that you find in Word, PowerPoint etc. That's a different problem and like you say, it creates more headache. I'll skip the timer for now and see whether a reason for this would ever show up.

[xiangzhi]

One thing I just realized is that we need to be careful about how autosave is implemented for CreateDerivedPartitionAsync. If the autosave is part of the task, multiple autosave might be happening which could create a concurrency bug.

[danbohus]

There is no autoSaveChanges constructor / Load parameter yet on the Dataset, I was suggesting that a way forward might be to add it. And yes, a changed or modified flag would be good to have. I'm guessing the autosave functionality could be implemented by the dataset/session, and yes, the dataset would have to keep track of its filepath. We haven't implemented any of this yet. Finally, re: your last comment, yes, would be good to anticipate and pay attention to potential concurrency issues. Thanks!

[xiangzhi]

Oops, I totally missed the e.g in the previous message.

I'll take a crack at this in the next few days and will keep you all posted!