Implement autosave timer. #7036
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #7036 +/- ##
===========================================
- Coverage 63.68% 63.68% -0.00%
===========================================
Files 330 330
Lines 41605 41636 +31
===========================================
+ Hits 26493 26512 +19
- Misses 15112 15124 +12
Continue to review full report at Codecov.
|
|
Note that the issue says "every X minutes", not "every X seconds". |
|
I deliberately chose to use seconds instead to give users more fine grained control. I do however see the point of using minutes instead. Whats your opinion on that? |
|
Should be minutes. If you are going to be sub 1-minute you might as well enable auto save after every change. The default should be 5 minutes. I am actually going to use this feature right away, it is an excellent compromise between auto-save after every change and being able to make a bunch of changes in a row without the delays. |
Set default to 5 minutes. Disable timer by default.
|
I've implemented the changes. I've also set the timer to being disabled by default, just to avoid any surprises. |
|
Sorry to play devil's advocate, but why exactly do we need this? What's wrong with normal autosave? |
|
This is a compromise between saving every single change and only saving on lock. Think of this like a setting that extends the delay time for auto save to kick in. |
|
But then, do we need a setting for that? |
|
We could rebrand this as a timeout for autosave |
|
I'm more for some sort of intelligent backoff mechanism. If two saves happen in short succession, a delay is added before the next one. |
|
I could be down with that, but sometimes being too smart gets you in trouble |
|
Maybe it makes more sense to consider this with #7035 in mind, more precisely the time based formatting of backup files.
This is - essentially - how this is implemented right now. The timer resets when the database is successfully saved - no matter what triggered the save. The timer essentially gives a guarantee that the last save happend at most X minutes ago. |
|
I just don't like unnecessary settings that are relevant only for 0.001% of expert users and confuse everybody else. |
|
Looking at the screenshot it is confusing to have auto save after every change AND this setting. I would prefer to combine the two and if the timeout is set to 0 then it behaves as the original "after every change". Otherwise you can increment the delay in whole minute increments. |
|
So the autosave timer feature should not periodically save the database, but instead throttle saves such that at most a single save happens every X minutes? Im not sure if this is the right way to go, especially because this may compromise safety unless we include several other settings (should manual saves overwrite this behaviour? etc.). I do see the benefit of this throttling feature but i believe that it will be hard to communicate the exact semantics and implications to the user. Either way, we should probably move this discussion to a separate issue and focus on the PR here. Regarding the autosave timer: I believe it can make sense to periodically save the DB when other autosave features are disabled, especially since this does not interfere with "manual" saving. |
No, on the contrary. Saving is a risky operation if there are changes and if there are no changes, then there is also no point for a periodic timer. |
What if the file is located on e.g. a network drive such that saving takes a long time and you make frequent edits? Perhaps the connection even drops from time to time and you find the "save as" dialog appearing constantly annoying? I can see how saving periodically, say every 5 minutes, could be more convenient in such cases. Maybe the user also wants to make sure that the timestamp of the DB file is updated regularly for whatever reason. I mean, this is a quite common feature in other programs so some users seem to like this behavior? Does the drawback of added complexity really outweigh the potential use cases? If so, can we extend the feature to capture more use cases such that its worth it's weight? |
|
On a network drive with unstable connection, periodic saves are the very last thing I want. |
|
So how do we proceed? Should we keep it as is? Refactor it to a delay feature? Get rid of it entirely? |
|
Refactor the ui as a delay but retain the code implementation. It makes the most sense to me. |
As a delay? But that's not how the code behaves currently. Right now, the timer simply saves the database when it runs out. Successful saves, either by the timer itself, the user, or other autosave mechanisms, restart the timer. So saves triggered by e.g. editing an entry are executed immediately. The feature essentially ensures that at least one save happen within the configured time interval. |
|
OK I did not look at the code close enough, you are correct. This is actually not the way in which this was meant to be implemented. Also, after discussion with other devs we are not going to incorporate a save timer feature. Thank you for your efforts. |
Resolves #4152.
Adds a timer that automatically saves currently opened databases every X seconds. The interval length is configurable (seconds). Each database has a separate timer, but the interval is configured globally. Saves triggered externally (i.e., by the user or other auto save mechanisms) reset the timer.
Screenshots
Testing strategy
Tested manually with QDebug() and several test databases.
Limitations: (Comment please)
Type of change