Human readable errors on attempted deletion of non-empty directory

[AudriusButkevicius]

Given we know there are files that are not deletable and ignored, we could return a more descriptive error.
Also of we discover stuff we don't have in the index, we could force rescan.

[generalmanager]

I just created a similar ticket for the case where local files can't be read or are ignored because they are symlinks (#4480). We should probably use the same kind of UI signaling in both cases. Also 👍

[AudriusButkevicius]

This has nothing todo with UI.

[generalmanager]

What I meant is that the kind of UI I described in #4480 could also be appropriate for the case where ignored files/subdirectories or ACL prevent the deletion of a directory.
I didn't grasp all the details of #4475 and this tickets connection to it before reading it in it's entirety, so I get if you want to keep the scope of this issue focused on the error message and triggering a rescan.

I just wanted to leave a note to a possible UI improvement in case #4480 is accepted, so that this and similar cases where local stuff can't be deleted would be treated similarly to local stuff not beeing readable. Apoligies if you felt that I sidetracked from the topic.

[AudriusButkevicius]

It seems you are confused. This ticket doesn't need a UI nor does #4475.

[generalmanager]

I understand that neither an improved error message nor triggering a rescan or bumping a version on the db needs any kind of UI. I just wanted to leave a note saying that IF it should be decided to build some kind of user-facing message in regard to unaccessible files (which the issue I opened is about) then it could also make sense to use the same UI to inform the user about beeing unable to delete local folders (with ignored stuff inside it beeing one possible reason).

I agree that not beeing able to delete something isn't necessarily of utmost importance to the user, this was really only meant as a small annotation, mainly to get the tickets linked so when this is built, one takes a second to think if the UI which might be built for #4480 would also be useful for this, preventing additional work in the future. But now with all the back-and-forth it's really starting to distract from the main issue. So please feel free to delete my comments here.

[calmh]

Closed by #4493

[calmh]

@imsodin what was the resolution in relation to this issue, I'm not sure I understand the action here

[AudriusButkevicius]

It returns more readable errors and schedules scans from within delete if there are unscanned entries.

[imsodin]

More descriptive errors are introduced with this https://github.com/syncthing/syncthing/pull/4493/files#diff-feb9fde15f8dd977fe9b279a36a447a2R796 and this
https://github.com/syncthing/syncthing/pull/4493/files#diff-feb9fde15f8dd977fe9b279a36a447a2R1789.

Scanning is scheduled here: https://github.com/syncthing/syncthing/pull/4493/files#diff-feb9fde15f8dd977fe9b279a36a447a2R1777

[calmh]

So given the dir is resurrected and rescanned, which is #4475, does the user ever see or care about the error?

[calmh]

I'm trying to get this retitled to something intelligible for the release notes, or decide to not include it

[imsodin]

In the time until the scan and the next pull (timer based) happens, the folder deletion will show up as a failed item with now a descriptive error message instead of just "dir not empty" (or something similar).

I think the most important part is for ignored items in to be deleted dirs: Before you got a generic "dir not empty" type error in the list (we got a lot of support questions about that), now the error will be
directory contains ignored files (see ignore documentation for (?d) prefix).

Title sounds quite accurate, I would just replace "Dir not empty" with something a bit more descriptive, along the lines of "Give a better error description and rescan if appropriate when a directory cannot be removed" (then again, my descriptions have a history of being less than intelligible :) ).