This repository was archived by the owner on Sep 30, 2024. It is now read-only.
Search: always respect default pattern type#63410
Merged
Merged
Conversation
4 tasks
jtibshirani
commented
Jun 20, 2024
Contributor
Author
There was a problem hiding this comment.
Here's the other important change, we now check the default before deciding whether to show the patterntype in the query,
jtibshirani
commented
Jun 20, 2024
Contributor
Author
There was a problem hiding this comment.
Here is the most important change. The rest of the PR is prop drilling :)
jtibshirani
commented
Jun 20, 2024
Contributor
Author
There was a problem hiding this comment.
This part isn't strictly necessary, but seemed nice and made this PR easier to reason about.
7fbd327 to
b47ff46
Compare
stefanhengl
added a commit
that referenced
this pull request
Jun 27, 2024
I noticed that the regexp toggle doesn't work anymore if `"search.defaultPatternType": "regexp"`. This is related to a recent change #63410. We also append `patternType:keyword` in that case which I don't think we want, because we have an UI element to indicate that keyword search is active. The question I don't know how to answer is: What should happen if `regexp` is the default and the user toggles keyword search off. Should we go back to `regexp` or to `standard`? Test plan: - new unit test - manual testing with default pattern type set to "keyword" and "regexp".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, enabling then disabling the regexp toggle would always set the
patterntype to
keyword, even when the user has setsearch.defaultPatternType: standard. Now, toggles always revert back to thedefault pattern type.
To support this, this PR adds
defaultPatternTypeto the nav bar query state,which is updated every time there's a settings update. Having
defaultPatternTypeavailable also lets us fix another bug: whenthe default pattern type has been set to
standardwe no longer awkwardly showpatterntype: standardin the search bar. (This confusing behavior wasintroduced recently in #63326.)
Overall, this PR set us up to remove
experimentalFeatures.keywordSearch,along with the keyword search toggle. To opt out of keyword search, users can
just set
search.defaultPatternType, and have it work everywhere.Relates to SPLF-68
Test plan
Adapted unit tests. Lots of manual testing:
search.defaultPatternType: standardexperimentalFeatures.keywordSearch: false