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

Don't instantly delete an unreferenced attachment #3103

Open
hungrysnake opened this issue Jul 3, 2019 · 7 comments
Open

Don't instantly delete an unreferenced attachment #3103

hungrysnake opened this issue Jul 3, 2019 · 7 comments
Labels
improvement request 🔨 Issue concerns an existing feature that needs improvement.

Comments

@hungrysnake
Copy link

hungrysnake commented Jul 3, 2019

Current behavior

image

when the note text with images is deleted, undoing the delete will only bring back the text, not the

Expected behavior

able to undo the deletion of images

Steps to reproduce

  1. paste an image to a note
  2. delete the line for the image
  3. Ctrl-Z to undo

the text will come back but the image is not loaded because it is deleted in the filesystem

Environment

  • Version : Boostnote 0.11.17
  • OS Version and name : Win10
@ZeroX-DG ZeroX-DG added bug 🐛 Issue concerns a bug. level 1 ❕ A bug that caused minor damage: App is still usable, but minor features might not work. improvement request 🔨 Issue concerns an existing feature that needs improvement. and removed level 1 ❕ A bug that caused minor damage: App is still usable, but minor features might not work. bug 🐛 Issue concerns a bug. labels Jul 3, 2019
ehhc added a commit to ehhc/Boostnote that referenced this issue Jul 5, 2019
@ehhc
Copy link
Contributor

ehhc commented Jul 5, 2019

I don't have a solution for this problem (i don't even know if there is a technical solution on how to do that..) but #3107 gives you 30 seconds to decide that you want to undo the change before the attachment is deleted :)
Maybe that's better than nothing :)

@hungrysnake
Copy link
Author

just throwing some idea.
could it be implemented this way?
when the image is deleted, temporarily rename the file to image.jpg.deleted.
when undo is triggered, rename it back to image.jpg
after a while (2 days maybe) delete all the *.deleted files.

@ehhc
Copy link
Contributor

ehhc commented Jul 5, 2019

the problem ist the last part: when and how do you crawl all folders with images/attachments in and delete unneeded ones? How do you trigger that job?

i think debouncing the deletion can be a good workaround. Maybe it's enough :)

@hungrysnake
Copy link
Author

i think the triggering point would be the moment of closing the application because undo can't be done after app close anyway.
another point would be the moment of opening application to make sure if there is any temp file left over

@ehhc
Copy link
Contributor

ehhc commented Jul 8, 2019

@hungrysnake at least the first idea dosen't work: after closing the app, the process is terminated. As far as i konw, there is no way to hook in at this point.
Scanning all notes at the start of the app (inclusive scanning the folders containing the attachments for all scanned notes) seems to be quite slow. Therefore the start of the app would be slowed down.. I don't know whether that's worth the benfefit of being able to undo changes..

@Flexo013
Copy link
Contributor

Flexo013 commented Jul 8, 2019

@ehhc I'm not quite sure how it works in Electron, but I think it should be possible to hook in on process termination. Similar in how certain programs will ask you "Are you sure you want to exit?".

A force termination of the process would be harder to hook into, so in that case we do need some clean-up of these files.


To expand on @hungrysnake's suggestion: We could move the deleted image to a separate designated .deleted_images folder. If the user undos then we simply move the image back to the orginal location. Upon closing (or starting) of Boostnote we check the .delete_images folder and clean it up.

Rokt33r pushed a commit that referenced this issue Jul 10, 2019
@Flexo013 Flexo013 changed the title undo deleted note losing image Don't instantly delete an unreferenced attachment Aug 6, 2019
@Flexo013
Copy link
Contributor

Flexo013 commented Aug 6, 2019

While this is now slightly mitigated I still think that the feature as I suggested in my previous comment would be really helpful.

It would solve #3203 for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement request 🔨 Issue concerns an existing feature that needs improvement.
Projects
None yet
Development

No branches or pull requests

4 participants