Skip to content

fix: Write in a temp file before replacing the original file, fixes #903 #1370

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Midou36O
Copy link

@Midou36O Midou36O commented Feb 25, 2025

I'm not experienced in rust but i thought saving in a temporary file would help avoid corrupting the original file if the save fails. Tested on my laptop and it does avoid corrupting the original file, which is the intended behavior.
This should fix #903

@Doublonmousse
Copy link
Collaborator

This also fixes #1128

@Midou36O
Copy link
Author

( Mini bump in case it got missed, or needs changes :p )

@Doublonmousse
Copy link
Collaborator

This looks good!
Initially, something similar was explored in #1177, but as this other PR touches on the file format (and we don't want to change it every morning) it'll take time before this other PR is merged.

So it makes sense to merge this PR as it fixes your issue now, knowing that this may be refactored further down the line when it comes time to merge #1177.

The only concern is how this change affect the file watcher, so that we don't have a "file modified on disk" when saving and keep the protection of the window not closing until the file is actually saved #1096. (So some testing is in order to verify this isn't affected)

@Doublonmousse
Copy link
Collaborator

I'm testing so see whether this works well with the file watcher and I'm encountering an issue. For now I'm doing the following

  • open rnote
  • edit the document
  • save to a new document
  • wait for the save to finish/popups to disappear (success!)
  • CTRL + S again
  • "Opened file was removed" and rnote treats the opened tab as a yet-to-be saved.

This is expected because the file watcher sees the file being removed when replacing the file and acts on it. It's actually the same thing for writing to the file (the file watcher sees the file being modified/written to) but its output is suppressed by doing

self.set_output_file_expect_write(true);

and preventing anything happening here (so that we don't see that the file is modified on disk when saving)

if canvas.output_file_expect_write() {
// While writing is in progress, multiple modify events might occur.
return;
}

(and a little further down here
if canvas.output_file_expect_write() {
// Own file writing has finished
canvas.set_output_file_expect_write(false);
}
)

So you probably need to check what kind of events get sent in that enum when saving a file for the second time and suppress this additional event

@anesthetice
Copy link
Contributor

You might be interested in this:

But basically to get around the file watcher issue I just set the output file to the "new" one:

self.set_output_file(Some(gio::File::for_path(&filepath)));

@Midou36O
Copy link
Author

Midou36O commented May 2, 2025

Thanks, I just don't have time right to look into it right now but once I get back to it I'll try to replicate what you've done.

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.

saving when hard drive being full corrupts the note file.
3 participants