Proposal: remove "Remote content search" checkbox from Search dialog and set it always to "true"
Closed, ResolvedPublic

Description

When I want to search for files (Ctrl+S) while using "Containing text" functionality, I fill the "Text" field and press search. When searching in remote location (ftp, smb...) I need to also check "Remote content search" to actually search in file contents, too. It took me quite some time to realize that so I think it's unnecessarily complicated.

Therefore I'd like to propose removal of this checkbox and act as it is always to true.

What do you think? Is there any reason I'm missing, why this option is there?

abika added a subscriber: abika.Aug 15 2016, 1:13 PM

The checkbox is used for setting the variable containOnRemote in VFS/krQuery. This is a switch to omit content search for non-local files. Most likely to prevent heavy unwanted network IO.

I agree that this doesn't make much sense: You either want to search network locations or you don't. But for later you wouldn't have added a remote search location.

Imo feel free to remove it but please with all the code remains that were connected to it.

asensi added a subscriber: asensi.Aug 15 2016, 8:56 PM

I agree.

If it may be useful: just for trying, I mounted a remote filesystem using ~/temporary, and when searching inside ~/temporary: disabling the "Remote content search" checkbox had no effect.

abika added a comment.Aug 15 2016, 9:14 PM

I mounted a remote filesystem using ~/temporary, and when searching inside ~/temporary: disabling the "Remote content search" checkbox had no effect.

Well, yes, thats why its so useless: The check for the remote location is done with

vf->vfile_getUrl().isLocalFile()

And the docs for "isLocalFile()" says

Returns true if this URL is pointing to a local file path. A URL is a local file path if the scheme is "file".

So, all files on a mounted network filesystem are "local" to this definition. A nice example for a good idea that turns out to be quite useless if the implementation went wrong.

A real solution is more complicated and could be done with QStorageInfo. I'm not sure if its worth the effort.

gengisdave added a subscriber: gengisdave.EditedAug 16 2016, 6:08 AM

I agree, the analysis is right; it is also needed to search into archives (because they have the tar: krarc: or zip: scheme).

The QStorageInfo seems to be the final solution, but I don't know if it worths.

Looking in krusader/VFS/krquery.cpp:324, the flag always true seems to have no impact on local files.

I will search in old commits, it seems to be an old hack to avoid freezes on low bandwitdh connection.

EDIT : commit found, no relevant info https://quickgit.kde.org/?p=krusader.git&a=commit&h=0beda3af9f2590340a3681802c5c7c1258743f13

Thanks a lot for guidance and information from all of you. I'll remove the checkbox along with all the connected code. The found commit will be helpful, thanks Davide! I'll provide a diff soon.

martinkostolny moved this task from TODO to In progress on the Krusader board.Aug 17 2016, 10:10 PM
martinkostolny closed this task as Resolved.Aug 31 2016, 5:49 AM
gengisdave moved this task from In progress to Done on the Krusader board.Oct 10 2016, 8:47 PM