Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove autosave files when user declines to restore from them #1962

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cme
Copy link
Contributor

@cme cme commented Mar 30, 2024

Remove autosave files when user declines to restore from them

When Hydrogen starts up, if a file has been autosaved the user will
be offered the opportunity to restore from that file. If they decline
it, Hydrogen should delete the autosaved file; it may be soon overwritten anyway by new autosave data.

This avoids the annoying cycle I often find myself in:

  1. edit file
  2. quit Hydrogen discarding edits (but autosave remains)
  3. open Hydrogen
  4. hit "Don't Save" because I didn't want those edits
  5. listen to original file without modifying (hence no autosave, but old autosave remains)
  6. quit Hydrogen (silently)
  7. goto 3

Also, when opening a file and restoring from its autosave, Hydrogen
should count this as the song being Modified because the current
content is from the autosave and is not the same as the content of
the named file.

This change also removes spurious sets of IsModified in parts of the GUI, which would setIsModified( true ) in the GUI initialisation despite not making any actual changes, as well as setModified( false ) calls that were needed to work around these spurious sets after song loading completed.

cme added 2 commits March 30, 2024 09:54
When Hydrogen starts up, if a file has been autosaved the user will
be offered the opportunity to restore from that file. If they decline
it, Hydrogen should delete the autosaved file; it may be soon overwritten anyway by new autosave data.

This avoids the annoying cycle I often find myself in:
   1. edit file
   2. quit Hydrogen discarding edits (but autosave remains)
   3. open Hydrogen
   4. hit "Don't Save" because I didn't want those edits
   5. listen to original file without modifying (hence no autosave, but old autosave remains)
   6. quit Hydrogen (silently)
   7. goto 3

Also, when opening a file and restoring from its autosave, Hydrogen
should count this as the song being Modified because the current
content is from the autosave and is not the same as the content of
the named file.
In initialising GUI elements, setting properties may set IsModified
to true, even though there is no user action and the properties are
only set to their existing value. Check these null actions and don't
set IsModified for them. Also, remove the now-unneeded sets of
IsModified to false what were needed to work around that behaviour.
@theGreatWhiteShark
Copy link
Contributor

Haha, setting autosave to 0 is also the first thing I do. It's quite annoying during development.

If they decline it, Hydrogen should delete the autosaved file

I also noted that. In #1955 I used a "No" instead of "Discard" button. But breaking the cycles sound like a very good idea.

How about we offer all three options: yes, no, discard. This way the user can view both original and modified version before deciding whether to discard or apply the changes.

@cme
Copy link
Contributor Author

cme commented Mar 31, 2024

How about we offer all three options: yes, no, discard. This way the user can view both original and modified version before deciding whether to discard or apply the changes.

Excellent idea, I like it.

Copy link
Contributor

@theGreatWhiteShark theGreatWhiteShark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. But there still seem to be some spurious calls to setIsModified during startup. When

  1. Create a new song
  2. Save it
  3. Quit Hydrogen and open it again

the empty song is marked "modified"

@cme
Copy link
Contributor Author

cme commented Apr 2, 2024

Oh good catch, I'll look into it. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants