Skip to content
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

[EXPLORER] File Search #1869

Merged
merged 52 commits into from Sep 15, 2019

Conversation

@b-r-o-c-k
Copy link
Contributor

commented Aug 21, 2019

Purpose

Google Summer of Code 2019

JIRA issue: CORE-9279

Proposed changes

This pull request adds search functionality to the file explorer. Here are the main features:

  • Search is case-insensitive and recurses all sub-folders
  • Support for UTF-16 encoded text files
  • File name filtering with support for wildcards
  • "In Folder" column shows the folder each file is located in
  • Right-click menu includes "Open Containing Folder" option
  • Progress is indicated in the status bar at the bottom of the window

Screenshots

screenshot2

screenshot1

@tkreuzer tkreuzer added this to New PRs in ReactOS PRs Aug 21, 2019

base/shell/explorer/lang/en-US.rc Show resolved Hide resolved
dll/win32/browseui/lang/en-US.rc Show resolved Hide resolved
dll/win32/browseui/lang/en-US.rc Show resolved Hide resolved

ReactOS PRs automation moved this from New PRs to WIP / Waiting on contributor Aug 22, 2019

@learn-more
Copy link
Member

left a comment

Very nice overall!
Some small comments

dll/win32/browseui/shellbars/CSearchBar.cpp Outdated Show resolved Hide resolved
dll/win32/browseui/shellbars/CSearchBar.cpp Outdated Show resolved Hide resolved
dll/win32/browseui/shellbars/CSearchBar.cpp Outdated Show resolved Hide resolved
dll/win32/browseui/shellbars/CSearchBar.cpp Outdated Show resolved Hide resolved
dll/win32/browseui/shellbars/CSearchBar.h Outdated Show resolved Hide resolved
dll/win32/browseui/shellbars/CSearchBar.cpp Outdated Show resolved Hide resolved
dll/win32/shell32/folders/CFindFolder.cpp Outdated Show resolved Hide resolved
dll/win32/browseui/shellbars/CSearchBar.h Outdated Show resolved Hide resolved
dll/win32/browseui/shellbars/CSearchBar.cpp Outdated Show resolved Hide resolved
dll/win32/browseui/shellbars/CSearchBar.cpp Outdated Show resolved Hide resolved
dll/win32/browseui/shellbars/CSearchBar.cpp Outdated Show resolved Hide resolved
dll/win32/browseui/shellbars/CSearchBar.cpp Outdated Show resolved Hide resolved
dll/win32/shell32/folders/CFindFolder.cpp Outdated Show resolved Hide resolved
dll/win32/shell32/folders/CFindFolder.cpp Outdated Show resolved Hide resolved
dll/win32/shell32/folders/CFindFolder.cpp Outdated Show resolved Hide resolved
dll/win32/shell32/folders/CFindFolder.cpp Outdated Show resolved Hide resolved
dll/win32/shell32/folders/CFindFolder.cpp Outdated Show resolved Hide resolved
@b-r-o-c-k

This comment has been minimized.

Copy link
Contributor Author

commented Aug 30, 2019

@yagoulas has requested that I move the code into a new library called shellfind and add prefixes to the commit messages, so I am going to rebase this branch.

@b-r-o-c-k b-r-o-c-k force-pushed the b-r-o-c-k:search branch from 691315f to 99d7d2f Aug 30, 2019

@yagoulas

This comment has been minimized.

Copy link
Member

commented Sep 5, 2019

Unless anyone has objections, I'm going to merge this PR in master this weekend.

@nagayev

This comment has been minimized.

Copy link

commented Sep 9, 2019

@yagoulas ping.

modules/rostests/apitests/com/shell32.c Outdated Show resolved Hide resolved

class CFindFolder :
public CWindowImpl<CFindFolder>,
public CComCoClass<CFindFolder, &CLSID_FindFolder>,

This comment has been minimized.

Copy link
@ThFabba

ThFabba Sep 10, 2019

Member

You have this (and CLSID_FileSearchBand) in browseui now, but the com_apitest seems to indicate it belongs in shell32. How hard would it be to move them?

This comment has been minimized.

Copy link
@yagoulas

yagoulas Sep 12, 2019

Member

It was my idea to move it here because the search components are tightly integrated and search isnt much related to the core of the shell

dll/win32/browseui/shellfind/CFindFolder.cpp Outdated Show resolved Hide resolved
@ThFabba

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

I don't think my last few comments necessarily have to block merge, they can be fixed after if desired.

@BenNottelling

This comment has been minimized.

Copy link
Contributor

commented Sep 14, 2019

End users won't know to add an * to search for part of a file name, which will result in a lot of JIRA issues for search "not working", so a comment or a toggle to add it might be a good idea.

@yagoulas

This comment has been minimized.

Copy link
Member

commented Sep 14, 2019

End users won't know to add an * to search for part of a file name, which will result in a lot of JIRA issues for search "not working", so a comment or a toggle to add it might be a good idea.

doesn't it work exactly the same way in windows?

@BenNottelling

This comment has been minimized.

Copy link
Contributor

commented Sep 14, 2019

doesn't it work exactly the same way in windows?

No idea about XP / 2003, from what I remember as a little kid, no, but anything vista and above it doesn't behave that way, so I'd still expect lot of people getting confused

@HBelusca

This comment has been minimized.

Copy link
Contributor

commented Sep 15, 2019

The regular * ? characters in the search is what existed since more than 20 years and what also exist on Linux (besides the regexps), so we expect people to know what to use there. No-one knows exactly how to use the search on Vista+ as efficiently as before anyways, so I don't think adding extra help would be needed. At most one could program a tooltip/balloon showing when the user points on the search box, telling the quick tips how to use it.

@yagoulas yagoulas merged commit 9b75b67 into reactos:master Sep 15, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

ReactOS PRs automation moved this from WIP / Waiting on contributor to Done Sep 15, 2019

@b-r-o-c-k b-r-o-c-k deleted the b-r-o-c-k:search branch Sep 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.