feat: setting to show/hide hidden files

[jrvidal]

This is a fix for #1245. It adds a new showHidden setting to control whether hidden files/folders are displayed to the user or not.

• "Hidden" means "starts with a dot", a la UNIX. I'm not sure how much we care about a hypothetical notes folder living within a Windows FS that contains windows-like hidden files. (I supposed the answer is "not much").

• The setting is only respected in the main web interface, and ignored for API methods. This is a quick-n-dirty compromise between letting clients handle hidden folders/files as they see fit on their own; vs. exposing this setting as public and store it per client (which goes beyond my knowledge of NC internals).

• The setting also affects other entrypoints that transitively depend on NotesService#getAll(), such as the search provider and the top-level notes. This seems appropriate to me, given its main usecase is probably metadata files generated by 3rd party apps.

• I've set the default to true so it is "backwards compatible". I'm not sure how acceptable would be to switch it to false.

• I haven't been able to get a proper setup to run tests locally, but I do want to add some tests.

[jrvidal]

I'm getting fancy here and trying to avoid full in-place reloads until they're absolutely needed, instead of using onChangeSettingsReload(). I'm not sure it's a good fit for NC's general UX approach.

[juliusknorr]

Seems good to me. 👍

[juliusknorr]

Thanks a lot for your PR. I've restarted tests and think there are some static analysis errors that need to be addressed, but the general PR seems almost ready to merge. A few small things commented inline and some feedback on your comments:

"Hidden" means "starts with a dot", a la UNIX. I'm not sure how much we care about a hypothetical notes folder living within a Windows FS that contains windows-like hidden files. (I supposed the answer is "not much").

It is fine to assume that but good that you raised it as a question. Our web frontend and file system layers only handle files starting with a dot as hidden. I'm not sure how that propagates to the desktop client but we do not have any special logic for hidden files in other backend areas. For SMB as external storage we are able to detect the hidden flag but this would then already be filtered out in the storage layers (https://github.com/nextcloud/server/blob/e9e67cbc50d0b1efb02a734b4c858b32ef8832c0/apps/files_external/lib/Lib/Storage/SMB.php#L237)

The setting is only respected in the main web interface, and ignored for API methods. This is a quick-n-dirty compromise between letting clients handle hidden folders/files as they see fit on their own; vs. exposing this setting as public and store it per client (which goes beyond my knowledge of NC internals).

For the mobile apps we could extend the API within notes to also pass the value along in

notes/lib/Controller/NotesApiController.php

Line 70 in e19b79b

$showHidden = true;

As long as the mobile apps for notes are using the dedicated endpoints and not WebDAV I'd say they can also respect the setting.

The setting also affects other entrypoints that transitively depend on NotesService#getAll(), such as the search provider and the top-level notes. This seems appropriate to me, given its main usecase is probably metadata files generated by 3rd party apps.

Would agree with that as well.

I've set the default to true so it is "backwards compatible". I'm not sure how acceptable would be to switch it to false.

I'd be fine to even switch the default, but not sure what @marcoambrosini would think about this as designer? Also @enjeck @juliusknorr as primary maintainers of the app?

I haven't been able to get a proper setup to run tests locally, but I do want to add some tests.

I remember that was a bit more tricky, they also are not that standard compared to other Nextcloud apps. Something we could follow up later on.

[juliusknorr]

Seems good to me. 👍

[enjeck]

Looks good, just nitpicks that you can ignore

[jrvidal]

@juliusknorr

For the mobile apps we could extend the API within notes to also pass the value along in

notes/lib/Controller/NotesApiController.php

Line 70 in e19b79b

$showHidden = true;

As long as the mobile apps for notes are using the dedicated endpoints and not WebDAV I'd say they can also respect the setting.

Sorry I'm a bit confused by this comment. Are you saying we should not hardcode $showHidden = true for the API endpoints, and read from settings instead? What do you mean by the "API within notes"?

Re: mobile apps using WebDAV: I assume you mean that we have no control over apps that use WebDAV, correct? i.e. that there's no way to propagate this setting for them (I assume the WebDAV handling is part of files itself).

[juliusknorr]

Sorry I'm a bit confused by this comment. Are you saying we should not hardcode $showHidden = true for the API endpoints, and read from settings instead? What do you mean by the "API within notes"?

Yes, sorry if that was not clear enough. I'd read the setting there instead of the hardcoded one as well.

Re: mobile apps using WebDAV: I assume you mean that we have no control over apps that use WebDAV, correct? i.e. that there's no way to propagate this setting for them (I assume the WebDAV handling is part of files itself).

Yes, that is in the responsibility of webdav / Nextcloud server then to handle.

[marcoambrosini]

I'd be fine to even switch the default, but not sure what @marcoambrosini would think about this as designer?

I'd agree with defaulting with not showing it.