search: fix regexp toggle if regexp is default (#63489)

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 commit is contained in:
Stefan Hengl 2024-06-27 12:50:12 +02:00 committed by GitHub
parent 4767166e1e
commit cb41db37cb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 68 additions and 4 deletions

View File

@ -1,5 +1,5 @@
import { screen } from '@testing-library/react'
import { describe, expect, test } from 'vitest'
import { screen, fireEvent } from '@testing-library/react'
import { describe, expect, test, vi } from 'vitest'
import { SearchPatternType } from '@sourcegraph/shared/src/graphql-operations'
import { SearchMode } from '@sourcegraph/shared/src/search'
@ -83,5 +83,34 @@ describe('Toggles', () => {
)
expect(screen.getAllByRole('checkbox', { name: 'Regular expression toggle' })).toMatchSnapshot()
})
test('Regex toggles off even if defaultPatternType is regexp', () => {
const setPatternType = vi.fn()
renderWithBrandedContext(
<Toggles
navbarSearchQuery="foo.*bar"
patternType={SearchPatternType.regexp}
defaultPatternType={SearchPatternType.regexp}
setPatternType={setPatternType}
caseSensitive={false}
setCaseSensitivity={() => undefined}
searchMode={SearchMode.Precise}
setSearchMode={() => undefined}
telemetryService={NOOP_TELEMETRY_SERVICE}
telemetryRecorder={noOpTelemetryRecorder}
/>
)
// Initially, the regexp toggle should be checked
expect(screen.getByRole('checkbox', { name: 'Regular expression toggle' })).toMatchSnapshot()
// Toggle the regexp off
fireEvent.click(screen.getByRole('checkbox', { name: 'Regular expression toggle' }))
// Verify that setPatternType was called with patternType keyword
expect(setPatternType).toHaveBeenCalledWith(SearchPatternType.keyword)
})
})
})

View File

@ -77,7 +77,13 @@ export const Toggles: React.FunctionComponent<React.PropsWithChildren<TogglesPro
}, [caseSensitive, setCaseSensitivity, submitOnToggle, telemetryRecorder])
const toggleRegexp = useCallback((): void => {
const newPatternType = patternType !== SearchPatternType.regexp ? SearchPatternType.regexp : defaultPatternType
const newPatternType =
patternType !== SearchPatternType.regexp
? SearchPatternType.regexp
: // Handle the case where the user has regexp configured as the default pattern type.
defaultPatternType === SearchPatternType.regexp
? SearchPatternType.keyword
: defaultPatternType
setPatternType(newPatternType)
submitOnToggle({ newPatternType })

View File

@ -1,5 +1,30 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
exports[`Toggles > Query input toggle state > Regex toggles off even if defaultPatternType is regexp 1`] = `
<div
aria-checked="true"
aria-disabled="false"
aria-label="Regular expression toggle"
class="btn btnIcon toggle test-regexp-toggle regularExpressionToggle toggleActive"
role="checkbox"
tabindex="0"
>
<svg
aria-hidden="true"
class="mdi-icon iconInline"
fill="currentColor"
height="24"
role="img"
viewBox="0 0 24 24"
width="24"
>
<path
d="M16,16.92C15.67,16.97 15.34,17 15,17C14.66,17 14.33,16.97 14,16.92V13.41L11.5,15.89C11,15.5 10.5,15 10.11,14.5L12.59,12H9.08C9.03,11.67 9,11.34 9,11C9,10.66 9.03,10.33 9.08,10H12.59L10.11,7.5C10.3,7.25 10.5,7 10.76,6.76V6.76C11,6.5 11.25,6.3 11.5,6.11L14,8.59V5.08C14.33,5.03 14.66,5 15,5C15.34,5 15.67,5.03 16,5.08V8.59L18.5,6.11C19,6.5 19.5,7 19.89,7.5L17.41,10H20.92C20.97,10.33 21,10.66 21,11C21,11.34 20.97,11.67 20.92,12H17.41L19.89,14.5C19.7,14.75 19.5,15 19.24,15.24V15.24C19,15.5 18.75,15.7 18.5,15.89L16,13.41V16.92H16V16.92M5,19A2,2 0 0,1 7,17A2,2 0 0,1 9,19A2,2 0 0,1 7,21A2,2 0 0,1 5,19H5Z"
/>
</svg>
</div>
`;
exports[`Toggles > Query input toggle state > case toggle for case subexpressions 1`] = `
Array [
<div

View File

@ -141,7 +141,11 @@ export function showPatternTypeInQuery(
return patternType !== defaultPatternType && !explicitPatternTypes.has(patternType)
}
const explicitPatternTypes = new Set([SearchPatternType.regexp, SearchPatternType.structural])
const explicitPatternTypes = new Set([
SearchPatternType.regexp,
SearchPatternType.structural,
SearchPatternType.keyword,
])
/**
* Update or initialize query state related data from settings