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

Implement advanced search #1797

Merged
merged 6 commits into from Nov 17, 2018

Conversation

Projects
None yet
6 participants
@droidmonkey
Copy link
Member

droidmonkey commented Mar 31, 2018

Description

Fully implements advanced searching and other search related features/issues. Eliminates the searching of group names and notes. End search after selecting a folder if global search is active (Fixes #1501). Also took the opportunity to clean up the ugly/messy entry view and model code surrounding searching.

Fixes #239 using the following syntax:

[modifiers][field:]["]term["]

Where modifiers can be any combo of:

  • - exclude this term from results
  • + match this term exactly
  • * term is handled as a regex

Where field can be one of:

  • title
  • user[name]
  • pass[word]
  • url
  • notes
  • attr[ibute]
  • attach[ment]

If field is omitted or non-existent the default is to search title, username, url, and notes.

Where term can contain wildcard terms (if NOT regex) of:

  • * match anything (or nothing)
  • ? match any one character
  • | logical OR

Sample search queries:

  • user:jon url:google Search username for jon and url for google
  • user:jon|hifi Search username for jon OR hifi
  • +user:jon -url:google *notes:"secret note \d" Search username exactly jon, url cannot contain google, and notes contains secret note [digit]

Motivation and context

Long requested feature

How has this been tested?

Manually and with new search term parser tests.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have read the CONTRIBUTING document. [REQUIRED]
  • My code follows the code style of this project. [REQUIRED]
  • All new and existing tests passed. [REQUIRED]
  • I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]
  • My change requires a change to the documentation and I have updated it accordingly.
  • I have added tests to cover my changes.

@droidmonkey droidmonkey added this to the v2.4.0 milestone Mar 31, 2018

@droidmonkey droidmonkey requested a review from keepassxreboot/core-developers Mar 31, 2018

}

// Short circuit if we failed to match or we matched and are excluding this term
if (!found || term->exclude) {
return false;

This comment has been minimized.

Copy link
@TheZ3ro

TheZ3ro Mar 31, 2018

Member

Shouldn't this be a continue instead? It is into a for and returning here will skip all next term in the for if the current is excluded

This comment has been minimized.

Copy link
@droidmonkey

droidmonkey Mar 31, 2018

Author Member

It is a short circuit because this function only returns true if all the terms match this particular entry (implicit AND of terms).

This comment has been minimized.

Copy link
@TheZ3ro

TheZ3ro Mar 31, 2018

Member

ok make sense

@yan12125

This comment has been minimized.

Copy link
Contributor

yan12125 commented Mar 31, 2018

* term is handled as a regex

I like this idea ;-) Looking forward to reusing this for #1769

@droidmonkey

This comment has been minimized.

Copy link
Member Author

droidmonkey commented Mar 31, 2018

Right, forgot about that, lemme extract the wildcard scrub and regex builder into the tools class

@droidmonkey droidmonkey force-pushed the feature/better-search branch 2 times, most recently from f6d24da to b0a8551 Mar 31, 2018

@insideClaw

This comment has been minimized.

Copy link

insideClaw commented May 7, 2018

Sounds like exactly what was a deal-maker for me to start using KeePassXC, that it doesn't currently match a lot of sites despite similarities in the name and URL. +1

@phoerious
Copy link
Member

phoerious left a comment

Looks good. One or two nitpicks. And some of the methods could use doc blocks.

Show resolved Hide resolved src/core/EntrySearcher.cpp Outdated
Show resolved Hide resolved src/core/EntrySearcher.h Outdated
Show resolved Hide resolved src/core/Tools.cpp Outdated

@droidmonkey droidmonkey force-pushed the feature/better-search branch 2 times, most recently from 665000a to 6cad130 Sep 16, 2018

Changes made and committed

@droidmonkey droidmonkey self-assigned this Sep 25, 2018

@droidmonkey droidmonkey force-pushed the feature/better-search branch from 6cad130 to 3b993ef Oct 9, 2018

@droidmonkey droidmonkey referenced this pull request Oct 19, 2018

Closed

Search by Password #2397

@phoerious
Copy link
Member

phoerious left a comment

What's the state of this?

@droidmonkey

This comment has been minimized.

Copy link
Member Author

droidmonkey commented Oct 21, 2018

Going to make a couple additions and then it will be ready

@intika

This comment has been minimized.

Copy link

intika commented Oct 31, 2018

@droidmonkey Great work this is very useful tested it and it seem to work as expected...
Just one thing, the search syntax is not intuitive you should add a hint in the search box... instead of "Search..." something like "Search... (+pass:keyword)" or "Search... (+user:keyword)"... easy modification...
or a drop-down list but this would be for a future PR i guess

@droidmonkey

This comment has been minimized.

Copy link
Member Author

droidmonkey commented Nov 2, 2018

I made a pretty awesome search help pop-up. Any feedback?

search_help

@droidmonkey droidmonkey force-pushed the feature/better-search branch from 3b993ef to 99cfe23 Nov 2, 2018

@intika

This comment has been minimized.

Copy link

intika commented Nov 2, 2018

Amazing indeed... that will be useful to remember the syntax

@phoerious

This comment has been minimized.

Copy link
Member

phoerious commented Nov 2, 2018

Despicable. 😉
No, great pop-up, but I would prefer if it had the QLineEdit background. And I would replace the part in brackets after the description with only "(logical AND)". Making a verb of "and" sounds rather clunky to me.

@phoerious

This comment has been minimized.

Copy link
Member

phoerious commented Nov 2, 2018

Oh, and could you perhaps make a generic help pop-up widget of it? We could use that in other parts as well.

@droidmonkey droidmonkey force-pushed the feature/better-search branch 2 times, most recently from 08fecf3 to cfdac14 Nov 15, 2018

@phoerious
Copy link
Member

phoerious left a comment

Two minor nitpicks, rest looks great.

Show resolved Hide resolved src/gui/widgets/PopupHelpWidget.cpp Outdated
Show resolved Hide resolved src/gui/widgets/PopupHelpWidget.h Outdated
Show resolved Hide resolved src/gui/widgets/PopupHelpWidget.h Outdated

droidmonkey added some commits Mar 20, 2018

Streamlined searcher code
* Remove searching of group title and notes
* End search when selecting a new group
* Correct entry searcher tests to align with new code
Add advanced search term parser
* Support quoted strings & per-field searching
* Support regex and exact matching
* Simplify search sequence
* Make search widget larger
* Add regex converter to Tools namespace
Implement search auto-clear and goto group
* Search clears if the search box does not have focus for 5 minutes (fixes #2178)
* Goto group from search results after double clicking the group name (fixes #2043)
Add search help pop-up
* Support ! modifier (same as '-')
* Create reusable PopupHelpWidget as self-contained popup that can
be positioned around a parent widget and will follow the movement
and sizing of the window
* Eliminated KEEPASSXC_MAIN_WINDOW macro and replaced with
getMainWindow() function
* Add tests to cover search help show/hide

@droidmonkey droidmonkey force-pushed the feature/better-search branch from fc090b2 to 880c3ae Nov 17, 2018

@droidmonkey

This comment has been minimized.

Copy link
Member Author

droidmonkey commented Nov 17, 2018

Done, and fixed the wording on the help popup, made background white, and fixed the border. Also squashed commits.

@droidmonkey droidmonkey merged commit 917c4cc into develop Nov 17, 2018

4 checks passed

CodeFactor No issues found.
Details
MacOS (KeepassXC) TeamCity build finished
Details
Ubuntu Linux (KeepassXC) TeamCity build finished
Details
Windows 10 (KeepassXC) TeamCity build finished
Details

@droidmonkey droidmonkey deleted the feature/better-search branch Nov 17, 2018

@droidmonkey droidmonkey referenced this pull request Jan 21, 2019

Open

Search #7

droidmonkey added a commit that referenced this pull request Mar 19, 2019

Release 2.4.0
- New Database Wizard [#1952]
- Advanced Search [#1797]
- Automatic update checker [#2648]
- KeeShare database synchronization [#2109, #1992, #2738, #2742, #2746, #2739]
- Improve favicon fetching; transition to Duck-Duck-Go [#2795, #2011, #2439]
- Remove KeePassHttp support [#1752]
- CLI: output info to stderr for easier scripting [#2558]
- CLI: Add --quiet option [#2507]
- CLI: Add create command [#2540]
- CLI: Add recursive listing of entries [#2345]
- CLI: Fix stdin/stdout encoding on Windows [#2425]
- SSH Agent: Support OpenSSH for Windows [#1994]
- macOS: TouchID Quick Unlock [#1851]
- macOS: Multiple improvements; include CLI in DMG [#2165, #2331, #2583]
- Linux: Prevent Klipper from storing secrets in clipboard [#1969]
- Linux: Use polling based file watching for NFS [#2171]
- Linux: Enable use of browser plugin in Snap build [#2802]
- TOTP QR Code Generator [#1167]
- High-DPI Scaling for 4k screens [#2404]
- Make keyboard shortcuts more consistent [#2431]
- Warn user if deleting referenced entries [#1744]
- Allow toolbar to be hidden and repositioned [#1819, #2357]
- Increase max allowed database timeout to 12 hours [#2173]
- Password generator uses existing password length by default [#2318]
- Improve alert message box button labels [#2376]
- Show message when a database merge makes no changes [#2551]
- Browser Integration Enhancements [#1497, #2253, #1904, #2232, #1850, #2218, #2391, #2396, #2542, #2622, #2637, #2790]
- Overall Code Improvements [#2316, #2284, #2351, #2402, #2410, #2419, #2422, #2443, #2491, #2506, #2610, #2667, #2709, #2731]
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.