Search: always respect default pattern type (#63410)

jtibshirani · web-flow · commit 37b6ded5f1f6 · 2024-06-25T09:43:14.000-07:00
Previously, enabling then disabling the regexp toggle would always set the patterntype to `keyword`, even when the user has set `search.defaultPatternType: standard`. Now, toggles always revert back to the default pattern type. To support this, this PR adds `defaultPatternType` to the nav bar query state, which is updated every time there's a settings update. Having `defaultPatternType` available also lets us fix another bug: when the default pattern type has been set to `standard` we no longer awkwardly show `patterntype: standard` in the search bar. (This confusing behavior was introduced 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.
diff --git a/client/branded/src/search-ui/input/SearchBox.story.tsx b/client/branded/src/search-ui/input/SearchBox.story.tsx
@@ -28,7 +28,8 @@ const defaultProps: SearchBoxProps = {
     telemetryRecorder: noOpTelemetryRecorder,
     queryState: { query: 'hello repo:test' },
     isSourcegraphDotCom: false,
-    patternType: SearchPatternType.standard,
+    patternType: SearchPatternType.keyword,
+    defaultPatternType: SearchPatternType.keyword,
     setPatternType: () => {},
     caseSensitive: false,
     setCaseSensitivity: () => {},
@@ -61,11 +62,21 @@ export const SearchBoxStory: StoryFn = () => (
                     <SearchBox {...defaultProps} patternType={SearchPatternType.regexp} />
                 </div>
 
+                <H2>Standard enabled</H2>
+                <div className="w-100 d-flex my-2">
+                    <SearchBox {...defaultProps} patternType={SearchPatternType.standard} />
+                </div>
+
                 <H2>Structural enabled</H2>
                 <div className="w-100 d-flex my-2">
                     <SearchBox {...defaultProps} patternType={SearchPatternType.structural} />
                 </div>
 
+                <H2>Default patterntype</H2>
+                <div className="w-100 d-flex my-2">
+                    <SearchBox {...defaultProps} defaultPatternType={SearchPatternType.standard} />
+                </div>
+
                 <H2>Case sensitivity enabled</H2>
                 <div className="w-100 d-flex my-2">
                     <SearchBox {...defaultProps} caseSensitive={true} />
diff --git a/client/branded/src/search-ui/input/SearchBox.tsx b/client/branded/src/search-ui/input/SearchBox.tsx
@@ -194,6 +194,7 @@ export const SearchBox: FC<SearchBoxProps> = props => {
                     {props.showKeywordSearchToggle ? (
                         <Toggles
                             patternType={props.patternType}
+                            defaultPatternType={props.defaultPatternType}
                             setPatternType={props.setPatternType}
                             caseSensitive={props.caseSensitive}
                             setCaseSensitivity={props.setCaseSensitivity}
diff --git a/client/branded/src/search-ui/input/toggles/Toggles.test.tsx b/client/branded/src/search-ui/input/toggles/Toggles.test.tsx
@@ -15,7 +15,8 @@ describe('Toggles', () => {
             renderWithBrandedContext(
                 <Toggles
                     navbarSearchQuery="(case:yes foo) or (case:no bar)"
-                    patternType={SearchPatternType.standard}
+                    patternType={SearchPatternType.keyword}
+                    defaultPatternType={SearchPatternType.keyword}
                     setPatternType={() => undefined}
                     caseSensitive={false}
                     setCaseSensitivity={() => undefined}
@@ -33,7 +34,8 @@ describe('Toggles', () => {
             renderWithBrandedContext(
                 <Toggles
                     navbarSearchQuery="(foo patterntype:literal) or (bar patterntype:structural)"
-                    patternType={SearchPatternType.standard}
+                    patternType={SearchPatternType.keyword}
+                    defaultPatternType={SearchPatternType.keyword}
                     setPatternType={() => undefined}
                     caseSensitive={false}
                     setCaseSensitivity={() => undefined}
@@ -50,7 +52,26 @@ describe('Toggles', () => {
             renderWithBrandedContext(
                 <Toggles
                     navbarSearchQuery="(foo patterntype:literal) or (bar patterntype:structural)"
-                    patternType={SearchPatternType.standard}
+                    patternType={SearchPatternType.keyword}
+                    defaultPatternType={SearchPatternType.keyword}
+                    setPatternType={() => undefined}
+                    caseSensitive={false}
+                    setCaseSensitivity={() => undefined}
+                    searchMode={SearchMode.Precise}
+                    setSearchMode={() => undefined}
+                    telemetryService={NOOP_TELEMETRY_SERVICE}
+                    telemetryRecorder={noOpTelemetryRecorder}
+                />
+            )
+            expect(screen.getAllByRole('checkbox', { name: 'Regular expression toggle' })).toMatchSnapshot()
+        })
+
+        test('regexp toggle with default patterntype', () => {
+            renderWithBrandedContext(
+                <Toggles
+                    navbarSearchQuery="foo.*bar"
+                    patternType={SearchPatternType.keyword}
+                    defaultPatternType={SearchPatternType.standard}
                     setPatternType={() => undefined}
                     caseSensitive={false}
                     setCaseSensitivity={() => undefined}
diff --git a/client/branded/src/search-ui/input/toggles/Toggles.tsx b/client/branded/src/search-ui/input/toggles/Toggles.tsx
@@ -28,6 +28,7 @@ export interface TogglesProps
         TelemetryV2Props,
         Partial<Pick<SubmitSearchProps, 'submitSearch'>> {
     navbarSearchQuery: string
+    defaultPatternType: SearchPatternType
     className?: string
     /**
      * If set to false makes all buttons non-actionable. The main use case for
@@ -46,6 +47,7 @@ export const Toggles: React.FunctionComponent<React.PropsWithChildren<TogglesPro
     const {
         navbarSearchQuery,
         patternType,
+        defaultPatternType,
         setPatternType,
         caseSensitive,
         setCaseSensitivity,
@@ -75,23 +77,22 @@ export const Toggles: React.FunctionComponent<React.PropsWithChildren<TogglesPro
     }, [caseSensitive, setCaseSensitivity, submitOnToggle, telemetryRecorder])
 
     const toggleRegexp = useCallback((): void => {
-        const newPatternType =
-            patternType !== SearchPatternType.regexp ? SearchPatternType.regexp : SearchPatternType.keyword
+        const newPatternType = patternType !== SearchPatternType.regexp ? SearchPatternType.regexp : defaultPatternType
 
         setPatternType(newPatternType)
         submitOnToggle({ newPatternType })
         telemetryService.log('ToggleRegexpPatternType', { currentStatus: patternType === SearchPatternType.regexp })
         telemetryRecorder.recordEvent('search.regexpPatternType', 'toggle')
-    }, [patternType, setPatternType, submitOnToggle, telemetryService, telemetryRecorder])
+    }, [patternType, defaultPatternType, setPatternType, submitOnToggle, telemetryService, telemetryRecorder])
 
     const toggleStructuralSearch = useCallback((): void => {
         const newPatternType: SearchPatternType =
-            patternType !== SearchPatternType.structural ? SearchPatternType.structural : SearchPatternType.keyword
+            patternType !== SearchPatternType.structural ? SearchPatternType.structural : defaultPatternType
 
         setPatternType(newPatternType)
         submitOnToggle({ newPatternType })
         telemetryRecorder.recordEvent('search.structuralPatternType', 'toggle')
-    }, [patternType, setPatternType, submitOnToggle, telemetryRecorder])
+    }, [patternType, defaultPatternType, setPatternType, submitOnToggle, telemetryRecorder])
 
     return (
         <div className={classNames(className, styles.toggleContainer)}>
diff --git a/client/branded/src/search-ui/input/toggles/__snapshots__/Toggles.test.tsx.snap b/client/branded/src/search-ui/input/toggles/__snapshots__/Toggles.test.tsx.snap
diff --git a/client/shared/src/search/searchQueryState.tsx b/client/shared/src/search/searchQueryState.tsx
@@ -69,6 +69,7 @@ export interface SearchQueryState {
     queryState: QueryState
     searchCaseSensitivity: boolean
     searchPatternType: SearchPatternType
+    defaultPatternType: SearchPatternType
     searchQueryFromURL: string
     searchMode: SearchMode
 
diff --git a/client/vscode/src/webview/search-panel/SearchHomeView.tsx b/client/vscode/src/webview/search-panel/SearchHomeView.tsx
@@ -167,6 +167,7 @@ export const SearchHomeView: React.FunctionComponent<React.PropsWithChildren<Sea
                         caseSensitive={caseSensitive}
                         setCaseSensitivity={setCaseSensitivity}
                         patternType={patternType}
+                        defaultPatternType={SearchPatternType.standard}
                         setPatternType={setPatternType}
                         searchMode={searchMode}
                         setSearchMode={setSearchMode}
diff --git a/client/vscode/src/webview/search-panel/SearchResultsView.tsx b/client/vscode/src/webview/search-panel/SearchResultsView.tsx
@@ -17,7 +17,7 @@ import {
 import { LATEST_VERSION, type RepositoryMatch, type SearchMatch } from '@sourcegraph/shared/src/search/stream'
 import { buildSearchURLQuery } from '@sourcegraph/shared/src/util/url'
 
-import type { SearchPatternType } from '../../graphql-operations'
+import { SearchPatternType } from '../../graphql-operations'
 import type { SearchResultsState } from '../../state'
 import type { WebviewPageProps } from '../platform/context'
 
@@ -322,6 +322,7 @@ export const SearchResultsView: React.FunctionComponent<React.PropsWithChildren<
                     caseSensitive={context.submittedSearchQueryState?.searchCaseSensitivity}
                     setCaseSensitivity={setCaseSensitivity}
                     patternType={context.submittedSearchQueryState?.searchPatternType}
+                    defaultPatternType={SearchPatternType.standard}
                     setPatternType={setPatternType}
                     searchMode={context.submittedSearchQueryState?.searchMode}
                     setSearchMode={setSearchMode}
diff --git a/client/vscode/src/webview/sidebars/search/SearchSidebarView.tsx b/client/vscode/src/webview/sidebars/search/SearchSidebarView.tsx
@@ -59,6 +59,7 @@ export const SearchSidebarView: FC<SearchSidebarViewProps> = React.memo(function
                 queryState: { query: '' },
                 searchCaseSensitivity: false,
                 searchPatternType: SearchPatternType.standard,
+                defaultPatternType: SearchPatternType.standard, // Not used here
                 searchQueryFromURL: '',
                 searchMode: SearchMode.Precise,
 
diff --git a/client/web/src/nav/new-global-navigation/NewGlobalNavigationBar.tsx b/client/web/src/nav/new-global-navigation/NewGlobalNavigationBar.tsx
@@ -156,7 +156,13 @@ export const NewGlobalNavigationBar: FC<NewGlobalNavigationBar> = props => {
 
 type NavigationSearchBoxState = Pick<
     SearchQueryState,
-    'queryState' | 'setQueryState' | 'submitSearch' | 'searchCaseSensitivity' | 'searchPatternType' | 'searchMode'
+    | 'queryState'
+    | 'setQueryState'
+    | 'submitSearch'
+    | 'searchCaseSensitivity'
+    | 'searchPatternType'
+    | 'defaultPatternType'
+    | 'searchMode'
 >
 
 /**
@@ -170,6 +176,7 @@ const selectQueryState = (state: SearchQueryState): NavigationSearchBoxState =>
     submitSearch: state.submitSearch,
     searchCaseSensitivity: state.searchCaseSensitivity,
     searchPatternType: state.searchPatternType,
+    defaultPatternType: state.defaultPatternType,
     searchMode: state.searchMode,
 })
 
@@ -191,8 +198,15 @@ const NavigationSearchBox: FC<NavigationSearchBoxProps> = props => {
     const location = useLocation()
     const showKeywordSearchToggle = useKeywordSearch()
 
-    const { searchMode, queryState, searchPatternType, searchCaseSensitivity, setQueryState, submitSearch } =
-        useNavbarQueryState(selectQueryState, shallow)
+    const {
+        searchMode,
+        queryState,
+        searchPatternType,
+        defaultPatternType,
+        searchCaseSensitivity,
+        setQueryState,
+        submitSearch,
+    } = useNavbarQueryState(selectQueryState, shallow)
 
     const submitSearchOnChange = useCallback(
         (parameters: Partial<SubmitSearchParameters> = {}) => {
@@ -231,6 +245,7 @@ const NavigationSearchBox: FC<NavigationSearchBoxProps> = props => {
                 <Toggles
                     searchMode={searchMode}
                     patternType={searchPatternType}
+                    defaultPatternType={defaultPatternType}
                     caseSensitive={searchCaseSensitivity}
                     navbarSearchQuery={queryState.query}
                     structuralSearchDisabled={structuralSearchDisabled}
diff --git a/client/web/src/search/input/SearchNavbarItem.tsx b/client/web/src/search/input/SearchNavbarItem.tsx
@@ -39,11 +39,26 @@ const selectQueryState = ({
     submitSearch,
     searchCaseSensitivity,
     searchPatternType,
+    defaultPatternType,
     searchMode,
 }: NavbarQueryState): Pick<
     NavbarQueryState,
-    'queryState' | 'setQueryState' | 'submitSearch' | 'searchCaseSensitivity' | 'searchPatternType' | 'searchMode'
-> => ({ queryState, setQueryState, submitSearch, searchCaseSensitivity, searchPatternType, searchMode })
+    | 'queryState'
+    | 'setQueryState'
+    | 'submitSearch'
+    | 'searchCaseSensitivity'
+    | 'searchPatternType'
+    | 'defaultPatternType'
+    | 'searchMode'
+> => ({
+    queryState,
+    setQueryState,
+    submitSearch,
+    searchCaseSensitivity,
+    searchPatternType,
+    defaultPatternType,
+    searchMode,
+})
 
 /**
  * The search item in the navbar
@@ -52,8 +67,15 @@ export const SearchNavbarItem: React.FunctionComponent<React.PropsWithChildren<P
     const navigate = useNavigate()
     const location = useLocation()
 
-    const { queryState, setQueryState, submitSearch, searchCaseSensitivity, searchPatternType, searchMode } =
-        useNavbarQueryState(selectQueryState, shallow)
+    const {
+        queryState,
+        setQueryState,
+        submitSearch,
+        searchCaseSensitivity,
+        searchPatternType,
+        defaultPatternType,
+        searchMode,
+    } = useNavbarQueryState(selectQueryState, shallow)
 
     const [v2QueryInput] = useV2QueryInput()
 
@@ -107,6 +129,7 @@ export const SearchNavbarItem: React.FunctionComponent<React.PropsWithChildren<P
                     {props.showKeywordSearchToggle ? (
                         <Toggles
                             patternType={searchPatternType}
+                            defaultPatternType={defaultPatternType}
                             caseSensitive={searchCaseSensitivity}
                             setPatternType={setSearchPatternType}
                             setCaseSensitivity={setSearchCaseSensitivity}
@@ -153,6 +176,7 @@ export const SearchNavbarItem: React.FunctionComponent<React.PropsWithChildren<P
                 caseSensitive={searchCaseSensitivity}
                 setCaseSensitivity={setSearchCaseSensitivity}
                 patternType={searchPatternType}
+                defaultPatternType={defaultPatternType}
                 setPatternType={setSearchPatternType}
                 searchMode={searchMode}
                 setSearchMode={setSearchMode}
diff --git a/client/web/src/stores/navbarSearchQueryState.ts b/client/web/src/stores/navbarSearchQueryState.ts
@@ -28,16 +28,12 @@ import {
 
 export interface NavbarQueryState extends SearchQueryState {}
 
-const explicitPatternTypes = new Set([
-    SearchPatternType.keyword,
-    SearchPatternType.regexp,
-    SearchPatternType.structural,
-])
 export const useNavbarQueryState = create<NavbarQueryState>((set, get) => ({
     parametersSource: InitialParametersSource.DEFAULT,
     queryState: { query: '' },
     searchCaseSensitivity: false,
-    searchPatternType: SearchPatternType.standard,
+    searchPatternType: SearchPatternType.keyword,
+    defaultPatternType: SearchPatternType.keyword,
     searchMode: SearchMode.SmartSearch,
     searchQueryFromURL: '',
 
@@ -96,7 +92,8 @@ export function setSearchMode(searchMode: SearchMode): void {
  * the one contained in the URL (e.g. when the context:... filter got removed)
  */
 export function setQueryStateFromURL(parsedSearchURL: ParsedSearchURL, query = parsedSearchURL.query ?? ''): void {
-    if (useNavbarQueryState.getState().parametersSource > InitialParametersSource.URL) {
+    const currentState = useNavbarQueryState.getState()
+    if (currentState.parametersSource > InitialParametersSource.URL) {
         return
     }
 
@@ -121,9 +118,7 @@ export function setQueryStateFromURL(parsedSearchURL: ParsedSearchURL, query = p
         const parsedPatternType = parsedSearchURL.patternType
         if (parsedPatternType !== undefined) {
             newState.searchPatternType = parsedPatternType
-            // Only keyword, regexp, and structural are represented in the UI. For other pattern types, we make
-            // sure to surface them in the query input itself.
-            if (!explicitPatternTypes.has(parsedPatternType)) {
+            if (showPatternTypeInQuery(parsedPatternType, currentState.defaultPatternType)) {
                 query = `${query} ${FilterType.patterntype}:${parsedPatternType}`
             }
         }
@@ -137,6 +132,17 @@ export function setQueryStateFromURL(parsedSearchURL: ParsedSearchURL, query = p
     useNavbarQueryState.setState(newState as any)
 }
 
+// The only pattern types explicitly represented in the UI are the default one, plus regexp and structural. For
+// other pattern types, we make sure to surface them in the query input itself.
+export function showPatternTypeInQuery(
+    patternType: SearchPatternType,
+    defaultPatternType?: SearchPatternType
+): boolean {
+    return patternType !== defaultPatternType && !explicitPatternTypes.has(patternType)
+}
+
+const explicitPatternTypes = new Set([SearchPatternType.regexp, SearchPatternType.structural])
+
 /**
  * Update or initialize query state related data from settings
  */
@@ -146,7 +152,10 @@ export function setQueryStateFromSettings(settings: SettingsCascadeOrError<Setti
     }
 
     const newState: Partial<
-        Pick<NavbarQueryState, 'searchPatternType' | 'searchCaseSensitivity' | 'parametersSource' | 'searchMode'>
+        Pick<
+            NavbarQueryState,
+            'searchPatternType' | 'defaultPatternType' | 'searchCaseSensitivity' | 'parametersSource' | 'searchMode'
+        >
     > = {
         parametersSource: InitialParametersSource.USER_SETTINGS,
     }
@@ -164,6 +173,7 @@ export function setQueryStateFromSettings(settings: SettingsCascadeOrError<Setti
     const searchPatternType = defaultPatternTypeFromSettings(settings)
     if (searchPatternType) {
         newState.searchPatternType = searchPatternType
+        newState.defaultPatternType = searchPatternType
     }
 
     // The way Zustand is designed makes it difficult to build up a partial new
diff --git a/client/web/src/storm/pages/SearchPage/SearchPageInput.tsx b/client/web/src/storm/pages/SearchPage/SearchPageInput.tsx
diff --git a/client/web/src/util/settings.ts b/client/web/src/util/settings.ts
