Fix exclusion of Windows Index search for files and folders #386
Conversation
jjw24
requested review from
JohnTheGr8,
SysC0mp,
gdziedzic and
taooceros
and removed request for
JohnTheGr8,
SysC0mp,
gdziedzic and
taooceros
|
Do we exclude any path before this pr? |
|
Yeah we do, but only for directory search, meaning if you search 'c:\users\user1\', and that path is excluded, then directory info search will kick in. However if you search 'user1', windows index search will kick in even if you have added that path in the exclusion list. This pr is to fix it so windows index search won't kick in either if you search 'user1' folder. |
So why not merge them into one filter before returning the result list? |
|
What do you mean? The union? |
I mean merge the exclusion filter for index search and directory search together by changing the place filtering to the end of SearchAsync method. We can do the filter together instead of separate in directory search and windows search. We can remove the check for exclusion in Directory search and use the one we use here to do it by putting that here. Flow.Launcher/Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs Lines 100 to 102 in e73b2a0 |
|
Geat suggestion actually. What about adding it in the index search class, makes more sense right. |
But I think that class doesn't include direct directory traverse without index, right? |
|
No but this exclusion is only for Windows Index searches |
But that will make folder with Indexes have different behavior comparing to those folder without indexes when traversing? Because all index search calls this method. |
|
Yeah that's what the exclusion is for. When the path is added in the list, searching directly in the folder will not use windows index but will use directoryinfo search. When searching for a file or folder, you will not get any matches if their path is in the list. It's like turning off that path from indexing options, this is just a shortcut to do it. |
Yeah I mean the indexSearch class is both for directoryInfo search and global folder and file search if the folder is inside the index. We are supposed to only filter result for global index search right? |
|
No, DirectoryInfo search is in its own class. These two are seperated |
Sorry I misremember the name of the class. However, both directory search and index search can be used for > search in a folder (index search will be used when the folder is in the index). Therefore, putting the exclusion to index search can make these two behave differently, though should not be significant. |
Oh good point abt '>'. But this would be consistent behaviour right? In that, if it's excluded, it's excluded. If it returns results then doesnt that mean this is a bypass function for exclusion? Although, having said all that, I have been thinking to change '>' to bypass index search so this is a functions to always get results regardless of the exclusion. Like an override function |
|
Additionally, content search will be excluded too as it as it uses index search |
|
Maybe in the the future we can add in options for users to choose whether to apply the exclusion for:
|
|
@taooceros I have moved the exclusion method. Let me know what you think. |
|
LGTM, I will approve it. |
| var constructedQuery = constructQuery(searchString); | ||
| return await ExecuteWindowsIndexSearchAsync(constructedQuery, connectionString, query, token); | ||
| return await RemoveResultsInExclusionList( | ||
| await ExecuteWindowsIndexSearchAsync(constructedQuery, connectionString, query, token), |
There was a problem hiding this comment.
@taooceros should ConfigureAwait(false) and cancellation token be added for the two methods?
'Index Search Excluded' path does not kick in when using Windows Index to search for files and folders. This adds an exclusion call to filter out the index search result from exclusion list.
closing #384