Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit 5630eef

Browse files
authored
Search: surface pattern type in query input (#63326)
We plan to remove the 'Keyword Search' toggle as part of bringing the feature to GA. Once the toggle is removed, the search UX will only represent `keyword` (the default pattern type) and `regexp` (through the regex toggle), with no visual indication for other pattern types. So if a user clicks on a link using `patterntype:standard`, the search will just behave differently, without any indication in the UX as to why. This PR surfaces the `patterntype` filter in the search bar whenever it's not `keyword` or `regexp`. That way, users can see an old pattern type is being used and understand why the search behavior may be different. Relates to SPLF-68
1 parent 15ea951 commit 5630eef

File tree

5 files changed

+37
-83
lines changed

5 files changed

+37
-83
lines changed

client/web/src/integration/search-aggregation.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ describe('Search aggregation', () => {
307307
const origQuery = 'context:global insights('
308308

309309
await driver.page.goto(
310-
`${driver.sourcegraphBaseUrl}/search?q=${encodeURIComponent(origQuery)}&patternType=literal`
310+
`${driver.sourcegraphBaseUrl}/search?q=${encodeURIComponent(origQuery)}&patternType=keyword`
311311
)
312312

313313
await driver.page.evaluate(() => {

client/web/src/search/index.test.ts

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -138,29 +138,6 @@ describe('search/index', () => {
138138
searchMode: SearchMode.Precise,
139139
})
140140
})
141-
142-
test('parseSearchURL preserves literal search compatibility', () => {
143-
expect(parseSearchURL('q=/a literal pattern/&patternType=literal')).toStrictEqual({
144-
query: 'content:"/a literal pattern/"',
145-
patternType: SearchPatternType.standard,
146-
caseSensitive: false,
147-
searchMode: SearchMode.Precise,
148-
})
149-
150-
expect(parseSearchURL('q=not /a literal pattern/&patternType=literal')).toStrictEqual({
151-
query: 'not content:"/a literal pattern/"',
152-
patternType: SearchPatternType.standard,
153-
caseSensitive: false,
154-
searchMode: SearchMode.Precise,
155-
})
156-
157-
expect(parseSearchURL('q=un.*touched&patternType=literal')).toStrictEqual({
158-
query: 'un.*touched',
159-
patternType: SearchPatternType.standard,
160-
caseSensitive: false,
161-
searchMode: SearchMode.Precise,
162-
})
163-
})
164141
})
165142

166143
describe('repoFilterForRepoRevision escapes values with spaces', () => {

client/web/src/search/index.ts

Lines changed: 5 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,7 @@ import { memoizeObservable } from '@sourcegraph/common'
77
import { SearchPatternType } from '@sourcegraph/shared/src/graphql-operations'
88
import { SearchMode } from '@sourcegraph/shared/src/search'
99
import { discreteValueAliases, escapeSpaces } from '@sourcegraph/shared/src/search/query/filters'
10-
import { stringHuman } from '@sourcegraph/shared/src/search/query/printer'
1110
import { findFilter, FilterKind, getGlobalSearchContextFilter } from '@sourcegraph/shared/src/search/query/query'
12-
import { scanSearchQuery } from '@sourcegraph/shared/src/search/query/scanner'
13-
import { createLiteral } from '@sourcegraph/shared/src/search/query/token'
1411
import { omitFilter } from '@sourcegraph/shared/src/search/query/transformer'
1512
import type { AggregateStreamingSearchResults, StreamSearchOptions } from '@sourcegraph/shared/src/search/stream'
1613

@@ -100,25 +97,18 @@ export function parseSearchURL(
10097
urlSearchQuery: string,
10198
{ appendCaseFilter = false }: { appendCaseFilter?: boolean } = {}
10299
): ParsedSearchURL {
103-
let queryInput = parseSearchURLQuery(urlSearchQuery) || ''
104-
let patternTypeInput = parseSearchURLPatternType(urlSearchQuery)
100+
let query = parseSearchURLQuery(urlSearchQuery) || ''
101+
let patternType = parseSearchURLPatternType(urlSearchQuery)
105102
let caseSensitive = searchURLIsCaseSensitive(urlSearchQuery)
106103
const searchMode = parseSearchURLSearchMode(urlSearchQuery)
107104

108-
const globalPatternType = findFilter(queryInput, 'patterntype', FilterKind.Global)
105+
const globalPatternType = findFilter(query, 'patterntype', FilterKind.Global)
109106
if (globalPatternType?.value && globalPatternType.value.type === 'literal') {
110107
// Any `patterntype:` filter in the query should override the patternType= URL query parameter if it exists.
111-
queryInput = omitFilter(queryInput, globalPatternType)
112-
patternTypeInput = globalPatternType.value.value as SearchPatternType
108+
query = omitFilter(query, globalPatternType)
109+
patternType = globalPatternType.value.value as SearchPatternType
113110
}
114111

115-
let query = queryInput
116-
const { queryInput: newQuery, patternTypeInput: patternType } = literalSearchCompatibility({
117-
queryInput,
118-
patternTypeInput,
119-
})
120-
query = newQuery
121-
122112
const globalCase = findFilter(query, 'case', FilterKind.Global)
123113
if (globalCase?.value && globalCase.value.type === 'literal') {
124114
// Any `case:` filter in the query should override the case= URL query parameter if it exists.
@@ -166,45 +156,6 @@ export function quoteIfNeeded(string: string): string {
166156
return string
167157
}
168158

169-
interface QueryCompatibility {
170-
queryInput: string
171-
patternTypeInput?: SearchPatternType
172-
}
173-
174-
export function literalSearchCompatibility({ queryInput, patternTypeInput }: QueryCompatibility): QueryCompatibility {
175-
if (patternTypeInput === undefined || patternTypeInput !== SearchPatternType.literal) {
176-
return { queryInput, patternTypeInput }
177-
}
178-
const tokens = scanSearchQuery(queryInput, false, SearchPatternType.standard)
179-
if (tokens.type === 'error') {
180-
return { queryInput, patternTypeInput }
181-
}
182-
183-
if (!tokens.term.find(token => token.type === 'pattern' && token.delimited)) {
184-
// If no /.../ pattern exists in this literal search, just return the query as-is.
185-
return { queryInput, patternTypeInput: SearchPatternType.standard }
186-
}
187-
188-
const newQueryInput = stringHuman(
189-
tokens.term.map(token =>
190-
token.type === 'pattern' && token.delimited
191-
? {
192-
type: 'filter',
193-
range: { start: 0, end: 0 },
194-
field: createLiteral('content', { start: 0, end: 0 }, false),
195-
value: createLiteral(`/${token.value}/`, { start: 0, end: 0 }, true),
196-
negated: false /** if `NOT` was used on this pattern, it's already preserved */,
197-
}
198-
: token
199-
)
200-
)
201-
202-
return {
203-
queryInput: newQueryInput,
204-
patternTypeInput: SearchPatternType.standard,
205-
}
206-
}
207-
208159
export interface SearchStreamingProps {
209160
streamSearch: (
210161
queryObservable: Observable<string>,

client/web/src/stores/navbarSearchQueryState.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,16 @@ describe('navbar query state', () => {
9595

9696
expect(useNavbarQueryState.getState().searchPatternType).toBe(SearchPatternType.keyword)
9797
})
98+
99+
it('should add patterntype to query if not keyword or regexp', () => {
100+
setQueryStateFromURL(parseSearchURL('q=hello!&patternType=keyword'))
101+
expect(useNavbarQueryState.getState().queryState.query).toBe('hello!')
102+
expect(useNavbarQueryState.getState().searchPatternType).toBe(SearchPatternType.keyword)
103+
104+
setQueryStateFromURL(parseSearchURL('q=hello!!&patternType=standard'))
105+
expect(useNavbarQueryState.getState().queryState.query).toBe('hello!! patterntype:standard')
106+
expect(useNavbarQueryState.getState().searchPatternType).toBe(SearchPatternType.standard)
107+
})
98108
})
99109

100110
describe('state initialization precedence', () => {

client/web/src/stores/navbarSearchQueryState.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@ import create from 'zustand'
88
import {
99
type BuildSearchQueryURLParameters,
1010
canSubmitSearch,
11-
type SearchQueryState,
12-
updateQuery,
1311
InitialParametersSource,
1412
SearchMode,
13+
type SearchQueryState,
14+
updateQuery,
1515
} from '@sourcegraph/shared/src/search'
16+
import { FilterType } from '@sourcegraph/shared/src/search/query/filters'
1617
import type { Settings, SettingsCascadeOrError } from '@sourcegraph/shared/src/settings/settings'
1718
import { buildSearchURLQuery } from '@sourcegraph/shared/src/util/url'
1819

@@ -27,6 +28,11 @@ import {
2728

2829
export interface NavbarQueryState extends SearchQueryState {}
2930

31+
const explicitPatternTypes = new Set([
32+
SearchPatternType.keyword,
33+
SearchPatternType.regexp,
34+
SearchPatternType.structural,
35+
])
3036
export const useNavbarQueryState = create<NavbarQueryState>((set, get) => ({
3137
parametersSource: InitialParametersSource.DEFAULT,
3238
queryState: { query: '' },
@@ -68,7 +74,10 @@ export const useNavbarQueryState = create<NavbarQueryState>((set, get) => ({
6874
}))
6975

7076
export function setSearchPatternType(searchPatternType: SearchPatternType): void {
71-
useNavbarQueryState.setState({ searchPatternType })
77+
// When changing the patterntype, we also need to reset the query to strip out any potential patterntype: filter
78+
const state = useNavbarQueryState.getState()
79+
const query = state.searchQueryFromURL ?? state.queryState.query
80+
useNavbarQueryState.setState({ searchPatternType, queryState: { query } })
7281
}
7382

7483
export function setSearchCaseSensitivity(searchCaseSensitivity: boolean): void {
@@ -108,8 +117,15 @@ export function setQueryStateFromURL(parsedSearchURL: ParsedSearchURL, query = p
108117
// Only update flags if the URL contains a search query.
109118
newState.parametersSource = InitialParametersSource.URL
110119
newState.searchCaseSensitivity = parsedSearchURL.caseSensitive
111-
if (parsedSearchURL.patternType !== undefined) {
112-
newState.searchPatternType = parsedSearchURL.patternType
120+
121+
const parsedPatternType = parsedSearchURL.patternType
122+
if (parsedPatternType !== undefined) {
123+
newState.searchPatternType = parsedPatternType
124+
// Only keyword, regexp, and structural are represented in the UI. For other pattern types, we make
125+
// sure to surface them in the query input itself.
126+
if (!explicitPatternTypes.has(parsedPatternType)) {
127+
query = `${query} ${FilterType.patterntype}:${parsedPatternType}`
128+
}
113129
}
114130
newState.queryState = { query }
115131
newState.searchQueryFromURL = parsedSearchURL.query

0 commit comments

Comments
 (0)