Search: Remove unnecessary whitespace in case if we remove part of the string (#41456)

* Remove unnecessary whitespace in case if we remove part of the string

* Use omit filter instead of replace string for deleting filters from search queries

* Fix parseSearchURL unit tests

* Fix buildSearchURLQuery unit tests

* Fix after rebasing
This commit is contained in:
Vova Kulikov 2022-09-08 21:10:59 +03:00 committed by GitHub
parent e9bac92bf9
commit af0137dc5b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 28 additions and 25 deletions

View File

@ -41,7 +41,7 @@ export function dedupeWhitespace(value: string): string {
}
/**
* Checkes whether a given string is quoted.
* Checks whether a given string is quoted.
*
* @param value string to check against
*/

View File

@ -51,12 +51,12 @@ describe('omitFilter', () => {
test('omit context filter from the end of the query', () => {
const query = 'bar context:foo'
expect(omitFilter(query, getGlobalContextFilter(query))).toMatchInlineSnapshot('bar ')
expect(omitFilter(query, getGlobalContextFilter(query))).toMatchInlineSnapshot('bar')
})
test('omit context filter from the middle of the query', () => {
const query = 'bar context:foo bar1'
expect(omitFilter(query, getGlobalContextFilter(query))).toMatchInlineSnapshot('bar bar1')
expect(omitFilter(query, getGlobalContextFilter(query))).toMatchInlineSnapshot('bar bar1')
})
})

View File

@ -12,8 +12,13 @@ export function appendContextFilter(query: string, searchContextSpec: string | u
: query
}
/**
* Deletes the filter from a given query string by the filter's range.
*/
export function omitFilter(query: string, filter: Filter): string {
return replaceRange(query, filter.range).trimStart()
const { start, end } = filter.range
return `${query.slice(0, start).trimEnd()} ${query.slice(end).trimStart()}`.trim()
}
const succeedScan = (query: string): Token[] => {

View File

@ -406,15 +406,15 @@ describe('buildSearchURLQuery', () => {
))
it('appends the case parameter if `case:yes` exists in the query', () =>
expect(buildSearchURLQuery('foo case:yes', SearchPatternType.standard, false, undefined)).toBe(
'q=foo+&patternType=standard&case=yes'
'q=foo&patternType=standard&case=yes'
))
it('removes the case parameter if using a quoted value', () =>
expect(buildSearchURLQuery('foo case:"yes"', SearchPatternType.standard, true, undefined)).toBe(
'q=foo+&patternType=standard&case=yes'
'q=foo&patternType=standard&case=yes'
))
it('removes the case parameter case:no exists in the query and caseSensitive is true', () =>
expect(buildSearchURLQuery('foo case:no', SearchPatternType.standard, true, undefined)).toBe(
'q=foo+&patternType=standard'
'q=foo&patternType=standard'
))
})

View File

@ -5,7 +5,6 @@ import {
findLineKeyInSearchParameters,
formatSearchParameters,
LineOrPositionOrRange,
replaceRange,
toPositionOrRangeQueryParameter,
toViewStateHash,
} from '@sourcegraph/common'
@ -15,7 +14,7 @@ import { WorkspaceRootWithMetadata } from '../api/extension/extensionHostApi'
import { SearchPatternType } from '../graphql-operations'
import { discreteValueAliases } from '../search/query/filters'
import { findFilter, FilterKind } from '../search/query/query'
import { appendContextFilter } from '../search/query/transformer'
import { appendContextFilter, omitFilter } from '../search/query/transformer'
export interface RepoSpec {
/**
@ -538,9 +537,8 @@ export function buildSearchURLQuery(
const globalPatternType = findFilter(queryParameter, 'patterntype', FilterKind.Global)
if (globalPatternType?.value) {
const { start, end } = globalPatternType.range
patternTypeParameter = globalPatternType.value.value
queryParameter = replaceRange(queryParameter, { start: Math.max(0, start - 1), end }).trim()
queryParameter = omitFilter(queryParameter, globalPatternType)
}
const globalCase = findFilter(queryParameter, 'case', FilterKind.Global)
@ -548,7 +546,7 @@ export function buildSearchURLQuery(
// When case:value is explicit in the query, override any previous value of caseParameter.
const globalCaseParameterValue = globalCase.value.value
caseParameter = discreteValueAliases.yes.includes(globalCaseParameterValue) ? 'yes' : 'no'
queryParameter = replaceRange(queryParameter, globalCase.range)
queryParameter = omitFilter(queryParameter, globalCase)
}
if (searchContextSpec) {

View File

@ -12,7 +12,7 @@ describe('search/index', () => {
expect(
parseSearchURL('q=TEST+repo:sourcegraph/sourcegraph+case:yes&patternType=standard&case=yes')
).toStrictEqual({
query: 'TEST repo:sourcegraph/sourcegraph ',
query: 'TEST repo:sourcegraph/sourcegraph',
patternType: SearchPatternType.standard,
caseSensitive: true,
})
@ -20,7 +20,7 @@ describe('search/index', () => {
expect(
parseSearchURL('q=TEST+repo:sourcegraph/sourcegraph+case:no&patternType=standard&case=yes')
).toStrictEqual({
query: 'TEST repo:sourcegraph/sourcegraph ',
query: 'TEST repo:sourcegraph/sourcegraph',
patternType: SearchPatternType.standard,
caseSensitive: false,
})
@ -28,13 +28,13 @@ describe('search/index', () => {
expect(
parseSearchURL('q=TEST+repo:sourcegraph/sourcegraph+patternType:regexp&patternType=literal&case=yes')
).toStrictEqual({
query: 'TEST repo:sourcegraph/sourcegraph ',
query: 'TEST repo:sourcegraph/sourcegraph',
patternType: SearchPatternType.regexp,
caseSensitive: true,
})
expect(parseSearchURL('q=TEST+repo:sourcegraph/sourcegraph+case:yes&patternType=standard')).toStrictEqual({
query: 'TEST repo:sourcegraph/sourcegraph ',
query: 'TEST repo:sourcegraph/sourcegraph',
patternType: SearchPatternType.standard,
caseSensitive: true,
})
@ -44,7 +44,7 @@ describe('search/index', () => {
'q=TEST+repo:sourcegraph/sourcegraph+case:no+patternType:regexp&patternType=literal&case=yes'
)
).toStrictEqual({
query: 'TEST repo:sourcegraph/sourcegraph ',
query: 'TEST repo:sourcegraph/sourcegraph',
patternType: SearchPatternType.regexp,
caseSensitive: false,
})
@ -62,7 +62,7 @@ describe('search/index', () => {
appendCaseFilter: true,
})
).toStrictEqual({
query: 'TEST repo:sourcegraph/sourcegraph case:yes',
query: 'TEST repo:sourcegraph/sourcegraph case:yes',
patternType: SearchPatternType.standard,
caseSensitive: true,
})
@ -72,7 +72,7 @@ describe('search/index', () => {
appendCaseFilter: true,
})
).toStrictEqual({
query: 'TEST repo:sourcegraph/sourcegraph ',
query: 'TEST repo:sourcegraph/sourcegraph',
patternType: SearchPatternType.standard,
caseSensitive: false,
})
@ -82,7 +82,7 @@ describe('search/index', () => {
appendCaseFilter: true,
})
).toStrictEqual({
query: 'TEST repo:sourcegraph/sourcegraph case:yes',
query: 'TEST repo:sourcegraph/sourcegraph case:yes',
patternType: SearchPatternType.regexp,
caseSensitive: true,
})
@ -92,7 +92,7 @@ describe('search/index', () => {
appendCaseFilter: true,
})
).toStrictEqual({
query: 'TEST repo:sourcegraph/sourcegraph case:yes',
query: 'TEST repo:sourcegraph/sourcegraph case:yes',
patternType: SearchPatternType.standard,
caseSensitive: true,
})
@ -103,7 +103,7 @@ describe('search/index', () => {
{ appendCaseFilter: true }
)
).toStrictEqual({
query: 'TEST repo:sourcegraph/sourcegraph ',
query: 'TEST repo:sourcegraph/sourcegraph',
patternType: SearchPatternType.regexp,
caseSensitive: false,
})

View File

@ -1,13 +1,13 @@
import { escapeRegExp } from 'lodash'
import { Observable } from 'rxjs'
import { replaceRange } from '@sourcegraph/common'
import { SearchPatternType } from '@sourcegraph/shared/src/graphql-operations'
import { discreteValueAliases, escapeSpaces } from '@sourcegraph/shared/src/search/query/filters'
import { stringHuman } from '@sourcegraph/shared/src/search/query/printer'
import { findFilter, FilterKind } from '@sourcegraph/shared/src/search/query/query'
import { scanSearchQuery } from '@sourcegraph/shared/src/search/query/scanner'
import { createLiteral } from '@sourcegraph/shared/src/search/query/token'
import { omitFilter } from '@sourcegraph/shared/src/search/query/transformer'
import { AggregateStreamingSearchResults, StreamSearchOptions } from '@sourcegraph/shared/src/search/stream'
/**
@ -79,7 +79,7 @@ export function parseSearchURL(
const globalPatternType = findFilter(queryInput, 'patterntype', FilterKind.Global)
if (globalPatternType?.value && globalPatternType.value.type === 'literal') {
// Any `patterntype:` filter in the query should override the patternType= URL query parameter if it exists.
queryInput = replaceRange(queryInput, globalPatternType.range)
queryInput = omitFilter(queryInput, globalPatternType)
patternTypeInput = globalPatternType.value.value as SearchPatternType
}
@ -93,7 +93,7 @@ export function parseSearchURL(
const globalCase = findFilter(query, 'case', FilterKind.Global)
if (globalCase?.value && globalCase.value.type === 'literal') {
// Any `case:` filter in the query should override the case= URL query parameter if it exists.
query = replaceRange(query, globalCase.range)
query = omitFilter(query, globalCase)
if (discreteValueAliases.yes.includes(globalCase.value.value)) {
caseSensitive = true