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

Local trash / recylce bin #674

Closed
florianklumb opened this Issue Aug 15, 2017 · 30 comments

Comments

Projects
None yet
5 participants
@florianklumb

florianklumb commented Aug 15, 2017

Actual behaviour

Notes can only be undeleted when connected to OwnCloud.

Expected behaviour

Please consider adding an option, maybe just a subfolder, for a local trash.

@pbek pbek added the enhancement label Aug 15, 2017

@pbek

This comment has been minimized.

Owner

pbek commented Aug 15, 2017

Thank you for your suggestion.

@pbek

This comment has been minimized.

Owner

pbek commented Feb 6, 2018

Any suggestions?

  • A sub-folder inside the note folder (that would also get synced and viewed by Nextcould notes) or a sub-folder inside the settings folder (that would be global over all note-folders)?
  • That if two notes have the same name and should be put in the trash?
  • Should notes that are getting renamed copied to the trash (Nextcloud would do that)?

@florianklumb, @Maboroshy, @ludenticus

@ludenticus

This comment has been minimized.

Contributor

ludenticus commented Feb 6, 2018

1. Where should we put the deleted notes?

First we must define whether Nextcloud should see and sync these deleted notes or not.

a. Nextcloud should sync

In this case, QON has to have access to a subfolder Trash, which, on the other hand doesn't have to be seen by the user interface. Pros: deleted notes are always kept online, even if you don't have the “Versions” app enabled. Cons: it can be messy with subfolders, since every subfolder will have its Trash subfolder.

b. Nextcloud shouldn't sync

Provided that the Nextcloud client ignores by default some files, QON could either (b.1) create a Note.md~ or ._deleted_Note.md, or (b.2) put every deleted note under a ._Deleted_Notes folder. Pros: if you have the Nextcloud “Versions” app, you avoid duplicate info. And you can always modify the “Ignored files” settings in the client. I wouldn't put it under the settings folder. Cons: user could miss online the deleted notes.

2. What if two notes have the same name?

To avoid this problem, I see three options:

a. Add date/time of deletion in file name

Note.md could be renamed 20180205092123_Note.md when deleted. Of course, this is dependent on the place and method to store deleted notes. Pros: deleted notes are unique; notes are sorted according to the time of deletion. Cons: file names could be too long.

b. Add date/time of deletion within the note

First or last line of any deleted note now has the date and time of deletion. If restored, this line is erased. Pros: No matter where and how you store the deleted note, you can see when it was deleted. Cons: the implementation can be bothersome. There is a risk if the deleted note is modified.

c. Add a number

If two deleted notes have the same name, the latest deleted will have an added number. For instance, Note.md, Note_v1.md, Note_v2.md, etc. This is similar to the linux default when you have two or more files with the same name: the system adds (copy). The number system can be more efficient than adding (copy) each time for every repeated note. Pros: user could easily spot notes with the same title. Cons: unless the note has the date, user can't know when was deleted the note.

3. Should renamed notes be copied to the trash?

a. Yes

Pros: you keep track of name changes. Best if you have info of date/time of deletion. Cons: the list of deleted notes can easily grow if you are undecided regarding a note title.

b. No

Pros: your deleted notes are kept minimal. Cons: you lose track of title changes.


Regarding 1, I'd go (b.2) the hidden–folder way by default.

Regarding 2, I'd go (a), adding the date/time of deletion to the file name.

Regarding 3, I'd go (b). I don't see the need to keep track of title changes.

@Maboroshy

This comment has been minimized.

Contributor

Maboroshy commented Feb 6, 2018

As usual I see that the issue can be resolved as a rather simple script, that can do whatever user wants. That's the "customize everything" way, good for geeks, can be scary for casual users. To properly go that way there should be an ability to assign script to app's default actions, such as "delete note".

If we need the casual way I'd go with hidden directory in note folder with note names prepended with deletion time as @ludenticus wrote. The trash needs dedicated windows that will show clear note names and deletion time parsed from name. Maybe sub-folder too but that raises question how to store it. I think that trash should be synced since that's what user expects and is the safest scenario.

Personally I prefer versioning to trash bins. Trash bins are like versioning implemented half way. Yet I think that app simply should not do that, especially having powerful scripting features. But I'm a Linux user, I'm used to having many apps do simple things. I already have trash bin in OS i can use, Resilio Sync I use has versioning, Synology NAS that stores my notes has versioning... I don't think I need note app trash/versioning on top of that. But yet again, that's me.

@pbek

This comment has been minimized.

Owner

pbek commented Feb 6, 2018

@pbek

This comment has been minimized.

Owner

pbek commented Feb 7, 2018

I was brain-storming about something like that

  • new menu entry Local trash
    • dialog with list of trashed files
    • check on restoration if destination file already exists
  • one trash folder per note-folder
    • needs to be ignored in MainWindow::buildNotesIndex
  • trash table in note-folder database
  • trashed files are renamed to id of trash table
    • no file-extension, so it gets ignored in Nextcloud Notes app
  • real name is stored in trash table
  • trash table fields
    • id
    • filename
    • subfolder
    • deletedate
  • trashed files are deleted after 30 days per default
    • setting: days, off
  • setting to turn trashing off
@Maboroshy

This comment has been minimized.

Contributor

Maboroshy commented Feb 7, 2018

trashed files are renamed to id of trash table
no file-extension, so it gets ignored in Nextcloud Notes app

Why not use deletiondate-notename as ID, so trash can be portable? ID can be anything, so why not make it mean something as long as it's unique? I think that extension thing can be optional. It won't add too much complexity, but allow to sync trash. I still think that syncing trash should be default behaviour as it's safer.

@pbek

This comment has been minimized.

Owner

pbek commented Feb 7, 2018

The id will be unique, because it's stored in the note-folder database, plus I have no troubles with long filenames and that the notes will be shown in NC Notes, while still syncing them...

@pbek

This comment has been minimized.

Owner

pbek commented Feb 12, 2018

18.02.1

  • there now is a local trash to gather your removed notes
    • this also works when a note gets renamed because you are modifying the
      headline of the note
    • there is a new panel Local trash in the setting where you can
      turn the local trash off
    • there is a new menu entry for the Local trash that opens a dialog to
      view your trashed notes and to restore or remove them
    • trashed files are stored per note folder in a new folder trash
      • additional information about the trashed files is stored in the
        sqlite database of the note folder

@pbek pbek added this to the 18.02.1 milestone Feb 12, 2018

@pbek

This comment has been minimized.

Owner

pbek commented Feb 12, 2018

There now is a new release, could you please test it and report if it works for you?

@pbek

This comment has been minimized.

Owner

pbek commented Feb 12, 2018

Todo

  • trashed files are deleted after 30 days per default
    • setting: days, off
  • also copy file to trash on manual note rename
@Maboroshy

This comment has been minimized.

Contributor

Maboroshy commented Feb 12, 2018

Looks like trashed files list doesn't check if a file is in trash directory completely relying on db.

@pbek

This comment has been minimized.

Owner

pbek commented Feb 12, 2018

If the file isn't present (yet?) the file can't be restored but it can be removed. If I don't show it they couldn't be removed...

@Maboroshy

This comment has been minimized.

Contributor

Maboroshy commented Feb 13, 2018

Shouldn't missing files be flushed from db on detection? Is there a point to store them in db if they can't be viewed of restored anymore?

@pbek

This comment has been minimized.

Owner

pbek commented Feb 13, 2018

What if those files are not synced yet from elsewhere but the database already is?

@pbek

This comment has been minimized.

Owner

pbek commented Feb 13, 2018

18.02.2

  • files that are missing in the local trash-folder will be now be marked in the
    Local trash dialog so that you know that they can't be restored
@Maboroshy

This comment has been minimized.

Contributor

Maboroshy commented Feb 13, 2018

What if those files are not synced yet from elsewhere but the database already is?

True. Unlikely, but possible. I think the mark you've implemented is the best solution.

@pbek

This comment has been minimized.

Owner

pbek commented Feb 13, 2018

18.02.2

  • added more local trash features
    • files that are missing in the local trash-folder will be now be marked in
      the Local trash dialog so that you know that they can't be restored
    • the old note is now also added to the local trash if it was renamed
      manually in the note list
    • sorting by trashing date in the trash dialog now works correctly
    • trashed items are now by default expired after 30 days
      • you can turn that off in the Local trash settings and also set
        the amount of days there

@pbek pbek modified the milestones: 18.02.1, 18.02.2 Feb 13, 2018

@pbek

This comment has been minimized.

Owner

pbek commented Feb 13, 2018

There now is a new release, could you please test it and report if it works for you?

@ludenticus

This comment has been minimized.

Contributor

ludenticus commented Feb 14, 2018

Yesterday SourceForge was offline —“static mode”—, but now it's back.

I've tested the new trash options and they all seem to work as intended.

@pbek

This comment has been minimized.

Owner

pbek commented Feb 14, 2018

Great, thank you for testing!

@pbek pbek closed this Feb 14, 2018

@FBachofner

This comment has been minimized.

FBachofner commented Feb 15, 2018

local trash does not seem to work QOwnNotes.v18.02.2.b3471.QT 5.7

This is the QT 5.7 local artifact which continues "support" for Win-XP

This is intended as an informational post only. Local trash is not important to me on this platform -- please consider focusing your impressive development efforts elsewhere

I will test the feature on Linux in a couple days

@pbek

This comment has been minimized.

Owner

pbek commented Feb 15, 2018

Strange, there shouldn't be any difference on Windows XP.

@FBachofner

This comment has been minimized.

FBachofner commented Feb 15, 2018

I tested in the following way:

  1. created (a) new note(s) and placed some text in it
  2. made sure it was really saved (using ctrl-S)
  3. deleted the note
    • using right-click, "remove notes"
    • using the trash icon
  4. clicked file, trash, show local trash

For each test this occurred within a couple seconds. Is there some kind of timer which might interfere?

Local trash is enabled in settings . . .

@pbek

This comment has been minimized.

Owner

pbek commented Feb 15, 2018

Was there any log-output (with debug log turned on)... Trashing does work on Windows 8.1.

@FBachofner

This comment has been minimized.

FBachofner commented Mar 5, 2018

Tried this again with build 3504 (on Windows XP, SP3) with QOwnNotes log on.

Immediately after opening QOwnNotes, log shows
[Mar 05 13:54:06] [debug]: loadNoteDirectoryList
and then 62 lines (equal to number of notes) of
[Mar 05 13:54:06] [warning]: fetchAllOfNote : QSqlError("", "Parameter count mismatch", "")

this warning repeats in the log as I move around to various notes.

creating and deleting a note:

[Mar 05 13:56:37] [debug]: storing note file:  "Note 2018-03-05T13.56.37.md"
[Mar 05 13:56:37] [status]: Stored current note to disk

and then again 62 errors
[Mar 05 13:56:38] [warning]: fetchAllOfNote : QSqlError("", "Parameter count mismatch", "")

When I emove the note:

[Mar 05 13:59:12] [debug]: removeNoteFile  - 'trashResult':  false
[Mar 05 13:59:12] [debug]: removeNoteFile  - 'this->fileName':  "Note 2018-03-05T13.56.37.md"
[Mar 05 13:59:12] [debug]: removeNoteFile  - 'file':  "Y:/Notes+Projects+ToDos/ToDos/Note 2018-03-05T13.56.37.md"
[Mar 05 13:59:12] [warning]: removeAllLinksToNote :  QSqlError("", "Parameter count mismatch", "")
[Mar 05 13:59:12] [debug]: Removed note  "Note 2018-03-05T13.56.37"

note does not show up in trash, likely due to
[Mar 05 13:59:12] [debug]: removeNoteFile - 'trashResult': false

Local trash is enabled in settings.

Since QON has a ToDo feature and my folder name "ToDos" might somehow interfere, I also tried the same in another folder with the same basic result (i.e. deleted item is not trashed).

@FBachofner

This comment has been minimized.

FBachofner commented Mar 5, 2018

Since there were so many SQL errors I decided to delete notes.sqlite to see what happens.

It seems to have rebuilt and now all my errors are gone.

A deleted note even shows up in the local trash window and can be undeleted.

[Mar 05 14:27:03] [debug]: doTrashing  - 'destinationFileName':  "Y:/Notes+Projects+ToDos\\trash\\1"
[Mar 05 14:27:03] [debug]: removeNoteFile  - 'trashResult':  true
[Mar 05 14:27:03] [debug]: removeNoteFile  - 'this->fileName':  "Note 2018-03-05T14.26.18.md"
[Mar 05 14:27:03] [debug]: removeNoteFile  - 'file':  "Y:/Notes+Projects+ToDos/ToDos/Note 2018-03-05T14.26.18.md"
[Mar 05 14:27:03] [debug]: Removed note  "Note 2018-03-05T14.26.18"

then:
[Mar 05 14:27:11] [debug]: Restored note "Note 2018-03-05T14.26.18.md"

The note does in fact undelete, but appears only in the Windows file manager. It does not reappear in any note folder, including "All notes" unless I reload the note folder.

Is this the expected behavior?

@pbek

This comment has been minimized.

Owner

pbek commented Mar 6, 2018

Thank you for reporting, I will try to reproduce your issue.

Is this the expected behavior?

No, the directory watcher usually would reload the folder, maybe that's not working in Windows XP.

@pbek

This comment has been minimized.

Owner

pbek commented Mar 7, 2018

And I cannot reproduce your problems on Windows 10. :/

@pbek

This comment has been minimized.

Owner

pbek commented Mar 8, 2018

Neither on Windows 8.1...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment