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

Commit

Permalink
code monitors: respect default pattern type (#63333)
Browse files Browse the repository at this point in the history
This is part of the Keyword Search GA project (see background below). 

The core change is that we use the default pattern type consistently for
the query input field and preview. Before, we hardcoded `literal` as the
default and used `standard` for previews.

This is does not affect existing code monitors.

Other fixes:
- highlight keyword queries correctly

Background:
- "keyword" will soon be the new default pattern type
- the default pattern type can be overridden in the user/global settings
- query fields in all of our products should respect the default

Test Plan:
- The unit test is currently "skipped" with the following comment
```
// TODO: these tests trigger an error with CodeMirror, complaining about being
// loaded twice, see uiwjs/react-codemirror#506
```
- Manual testing:
- I created several code monitors with and without pattern type and
checked in the DB that the correct pattern type was appended.
- I configured a new default pattern type in my user settings and
verified that the setting changes the default pattern type for code
monitors.

Co-authored-by: Felix Kling <[email protected]>
  • Loading branch information
2 people authored and peterguy committed Jun 26, 2024
1 parent ab1a65b commit 4ba7000
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 20 deletions.
6 changes: 5 additions & 1 deletion client/branded/src/search-ui/input/CodeMirrorQueryInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { queryDiagnostic } from './codemirror/diagnostics'
import { HISTORY_USER_EVENT, searchHistory as searchHistoryFacet } from './codemirror/history'
import { useMutableValue, useOnValueChanged, useUpdateInputFromQueryState } from './codemirror/react'
import { tokenInfo } from './codemirror/token-info'
import { filterDecoration } from './experimental/codemirror/syntax-highlighting'
import type { QueryInputProps } from './QueryInput'

import styles from './CodeMirrorQueryInput.module.scss'
Expand Down Expand Up @@ -211,7 +212,10 @@ export const CodeMirrorMonacoFacade: React.FunctionComponent<CodeMirrorQueryInpu
])
)

const extensions = useMemo(() => [autocompletion, dynamicExtensions], [autocompletion, dynamicExtensions])
const extensions = useMemo(
() => [filterDecoration, autocompletion, dynamicExtensions],
[autocompletion, dynamicExtensions]
)

// Always focus the editor on 'selectedSearchContextSpec' change
useOnValueChanged(selectedSearchContextSpec, () => {
Expand Down
10 changes: 6 additions & 4 deletions client/branded/src/search-ui/input/codemirror/parsedQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { type EditorState, type Extension, Facet, StateEffect, StateField } from
import { SearchPatternType } from '@sourcegraph/shared/src/graphql-operations'
import { decorate, type DecoratedToken } from '@sourcegraph/shared/src/search/query/decoratedToken'
import { type ParseResult, parseSearchQuery, type Node } from '@sourcegraph/shared/src/search/query/parser'
import { scanSearchQuery } from '@sourcegraph/shared/src/search/query/scanner'
import { detectPatternType, scanSearchQuery } from '@sourcegraph/shared/src/search/query/scanner'
import type { Filter, Token } from '@sourcegraph/shared/src/search/query/token'

export interface QueryTokens {
Expand Down Expand Up @@ -74,12 +74,14 @@ export function parseInputAsQuery(initialParseOptions: ParseOptions): Extension
// recomputed whenever one of those values changes.
return queryTokens.compute(['doc', parseOptions], state => {
const textDocument = state.sliceDoc()
const { patternType, interpretComments } = state.field(parseOptions)
const options = state.field(parseOptions)
if (!textDocument) {
return { patternType, tokens: [] }
return { patternType: options.patternType, tokens: [] }
}

const result = scanSearchQuery(textDocument, interpretComments, patternType)
const patternType = detectPatternType(textDocument) || options.patternType
const result = scanSearchQuery(textDocument, options.interpretComments, patternType)

return {
patternType,
tokens: result.type === 'success' ? result.term : [],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'

import { mdiCheck, mdiRadioboxBlank, mdiHelpCircle, mdiOpenInNew } from '@mdi/js'
import { mdiCheck, mdiHelpCircle, mdiOpenInNew, mdiRadioboxBlank } from '@mdi/js'
import { VisuallyHidden } from '@reach/visually-hidden'
import classNames from 'classnames'

import { LazyQueryInput } from '@sourcegraph/branded'
import type { QueryState } from '@sourcegraph/shared/src/search'
import { FilterType, resolveFilter, validateFilter } from '@sourcegraph/shared/src/search/query/filters'
import { scanSearchQuery } from '@sourcegraph/shared/src/search/query/scanner'
import { useSettingsCascade } from '@sourcegraph/shared/src/settings/settings'
import { buildSearchURLQuery } from '@sourcegraph/shared/src/util/url'
import { Button, Link, Card, Icon, Checkbox, Code, H3, Tooltip } from '@sourcegraph/wildcard'
import { Button, Card, Checkbox, Code, H3, Icon, Link, Tooltip } from '@sourcegraph/wildcard'

import { SearchPatternType } from '../../../graphql-operations'
import { defaultPatternTypeFromSettings } from '../../../util/settings'

import styles from './FormTriggerArea.module.scss'

Expand All @@ -28,7 +30,10 @@ interface TriggerAreaProps {
}

const isDiffOrCommit = (value: string): boolean => value === 'diff' || value === 'commit'
const isValidPatternType = (value: string): boolean => value === 'keyword' || value === 'literal' || value === 'regexp'

// Code monitors don't support pattern type "structural"
const isValidPatternType = (value: string): boolean =>
value === 'keyword' || value === 'standard' || value === 'literal' || value === 'regexp'

const ValidQueryChecklistItem: React.FunctionComponent<
React.PropsWithChildren<{
Expand Down Expand Up @@ -161,8 +166,7 @@ export const FormTriggerArea: React.FunctionComponent<React.PropsWithChildren<Tr
validateFilter(filter.field.value, filter.value)
)

// No explicit patternType filter means we default
// to patternType:literal
// No explicit patternType filter means we use the default pattern type
hasValidPatternTypeFilter =
!hasPatternTypeFilter ||
filters.some(
Expand All @@ -180,14 +184,16 @@ export const FormTriggerArea: React.FunctionComponent<React.PropsWithChildren<Tr
setHasValidPatternTypeFilter(hasValidPatternTypeFilter)
}, [queryState.query])

const defaultPatternType: SearchPatternType = defaultPatternTypeFromSettings(useSettingsCascade())

const completeForm: React.FormEventHandler = useCallback(
event => {
event.preventDefault()
closeCard()
setTriggerCompleted(true)
onQueryChange(`${queryState.query}${hasPatternTypeFilter ? '' : ' patternType:literal'}`)
onQueryChange(`${queryState.query}${hasPatternTypeFilter ? '' : ` patternType:${defaultPatternType}`}`)
},
[closeCard, setTriggerCompleted, onQueryChange, queryState.query, hasPatternTypeFilter]
[closeCard, setTriggerCompleted, onQueryChange, queryState.query, hasPatternTypeFilter, defaultPatternType]
)

const cancelForm: React.FormEventHandler = useCallback(
Expand Down Expand Up @@ -231,7 +237,7 @@ export const FormTriggerArea: React.FunctionComponent<React.PropsWithChildren<Tr
>
<LazyQueryInput
className="test-trigger-input"
patternType={SearchPatternType.standard}
patternType={defaultPatternType}
isSourcegraphDotCom={isSourcegraphDotCom}
caseSensitive={false}
queryState={queryState}
Expand All @@ -242,11 +248,7 @@ export const FormTriggerArea: React.FunctionComponent<React.PropsWithChildren<Tr
</div>
<div className={styles.queryInputPreviewLink}>
<Link
to={`/search?${buildSearchURLQuery(
queryState.query,
SearchPatternType.standard,
false
)}`}
to={`/search?${buildSearchURLQuery(queryState.query, defaultPatternType, false)}`}
target="_blank"
rel="noopener noreferrer"
className="test-preview-link"
Expand All @@ -265,10 +267,11 @@ export const FormTriggerArea: React.FunctionComponent<React.PropsWithChildren<Tr
<li>
<ValidQueryChecklistItem
checked={hasValidPatternTypeFilter}
hint="Code monitors support literal and regex search. Searches are literal by default."
hint={`Code monitors support keyword, standard, literal and regex search. The default is ${defaultPatternType}`}
dataTestid="patterntype-checkbox"
>
Is <Code>patternType:keyword</Code>, <Code>literal</Code> or <Code>regexp</Code>
Is <Code>patternType:keyword</Code>, <Code>standard</Code>, <Code>literal</Code> or{' '}
<Code>regexp</Code>
</ValidQueryChecklistItem>
</li>
<li>
Expand Down

0 comments on commit 4ba7000

Please sign in to comment.