feat(svelte): Make fuzzy finder matching more reliable (#63397)

Currently the fuzzy finder filters and ranks results received from the
server locally. This was done to improve performance since local
filtering is much faster.
However it can introduce inconsistencies (as reported) because the local
filtering logic works differently that the one on the server. That means
the shown results is depedent on on the local cache, which is not
obvious to the user.

This commit removes the client side filtering and ranking and instead
relies only on the server for this. This makes things more consistent
and predictable, at the expense of being a little slower. However it
still feels quite fast to me.

Note that I didn't implement some aspects due to the limitations of the
GraphQL-based search API:
- No match highlighting. Personally I didn't miss is it so far. I don't
know if the highlighting actually provides a lot of value.
- No total result count. It seems since we are adding `count:50`, the
server cannot actually give use an approximate total count. But we
should probably still convey somehow that we are limiting results to the
top 50.

Because we don't have locally cached data anymore that can be shown
immediately I decided to increase the throttle time to prevent the
result list flickering in and out when typing with a moderate speed.

This change enables three additional features: 'search all' mode, multi
word search and regex search via `/.../` literals (just like for normal
search queries). This is consistent with our existing search query
language (currently regex literals are not syntax highlighted, but we
should consider doing that).

Fixes srch-139
Fixes srch-133
Fixes srch-134
Fixes srch-543



https://github.com/sourcegraph/sourcegraph/assets/179026/81e24345-9e06-4df6-bb4a-8a55e433bfd1


## Test plan

Manual testing.

## Changelog

- Add 'search all' tab
- Support multi-word search
- Support regular expression patterns
- Fix matching reliability
This commit is contained in:
Felix Kling 2024-06-21 22:23:48 +02:00 committed by GitHub
parent 7a9d2b02e4
commit ce7531f060
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 251 additions and 235 deletions

View File

@ -613,3 +613,20 @@ export const succeedScan = (query: string): Token[] => {
}
return result.term
}
const patternScanner = zeroOrMore(
oneOf<Term>(
whitespace,
toPatternResult(quoted('/'), PatternKind.Regexp),
// We don't use scanPattern or literal here because we want to treat parenthesis as regular characters
toPatternResult(scanToken(/\S+/), PatternKind.Literal)
)
)
/**
* Scans the search query as a sequence of patterns only. This is used in situations where we don't want
* to interpret filters or keywords.
*/
export function scanSearchQueryAsPatterns(query: string): ScanResult<Token[]> {
return patternScanner(query, 0)
}

View File

@ -1,5 +1,5 @@
query FuzzyFinderQuery($query: String!) {
search(query: $query) {
search(query: $query, version: V3) {
results {
results {
...FuzzyFinderFileMatch
@ -24,11 +24,9 @@ fragment FuzzyFinderFileMatch on FileMatch {
}
repository {
name
stars
}
}
fragment FuzzyFinderRepository on Repository {
name
stars
}

View File

@ -1,5 +1,6 @@
<script lang="ts" context="module">
export enum FuzzyFinderTabType {
All = 'all',
Repos = 'repos',
Symbols = 'symbols',
Files = 'files',
@ -23,17 +24,11 @@
import SymbolKindIcon from '$lib/search/SymbolKindIcon.svelte'
import { displayRepoName } from '$lib/shared'
import TabsHeader, { type Tab } from '$lib/TabsHeader.svelte'
import { Input } from '$lib/wildcard'
import { Alert, Input } from '$lib/wildcard'
import Button from '$lib/wildcard/Button.svelte'
import { filesHotkey, reposHotkey, symbolsHotkey } from './keys'
import {
createRepositorySource,
type CompletionSource,
createFileSource,
type FuzzyFinderResult,
createSymbolSource,
} from './sources'
import { allHotkey, filesHotkey, reposHotkey, symbolsHotkey } from './keys'
import { type CompletionSource, createFuzzyFinderSource } from './sources'
export let open = false
export let scope = ''
@ -45,17 +40,77 @@
}
}
const client = getGraphQLClient()
const tabs: (Tab & { source: CompletionSource })[] = [
{
id: 'all',
title: 'All',
shortcut: allHotkey,
source: createFuzzyFinderSource({
client,
queryBuilder: value =>
`patterntype:keyword (type:repo OR type:path OR type:symbol) count:50 ${scope} ${value}`,
}),
},
{
id: 'repos',
title: 'Repos',
shortcut: reposHotkey,
source: createFuzzyFinderSource({
client,
queryBuilder: value => `patterntype:keyword type:repo count:50 ${value}`,
}),
},
{
id: 'symbols',
title: 'Symbols',
shortcut: symbolsHotkey,
source: createFuzzyFinderSource({
client,
queryBuilder: value => `patterntype:keyword type:symbol count:50 ${scope} ${value}`,
}),
},
{
id: 'files',
title: 'Files',
shortcut: filesHotkey,
source: createFuzzyFinderSource({
client,
queryBuilder: value => `patterntype:keyword type:path count:50 ${scope} ${value}`,
}),
},
]
let dialog: HTMLDialogElement | undefined
let listbox: HTMLElement | undefined
let input: HTMLInputElement | undefined
let query = ''
let selectedTab = tabs[0]
let selectedOption: number = 0
const client = getGraphQLClient()
const tabs: (Tab & { source: CompletionSource<FuzzyFinderResult> })[] = [
{ id: 'repos', title: 'Repos', shortcut: reposHotkey, source: createRepositorySource(client) },
{ id: 'symbols', title: 'Symbols', shortcut: symbolsHotkey, source: createSymbolSource(client, () => scope) },
{ id: 'files', title: 'Files', shortcut: filesHotkey, source: createFileSource(client, () => scope) },
]
$: useScope = scope && selectedTab.id !== 'repos'
$: source = selectedTab.source
$: if (open) {
source.next(query)
}
$: if (open) {
dialog?.showModal()
input?.select()
} else {
dialog?.close()
}
$: placeholder = (function () {
switch (selectedTab.id) {
case 'repos':
return 'Find repositories...'
case 'symbols':
return 'Find symbols...'
case 'files':
return 'Find files...'
default:
return 'Find anything...'
}
})()
function selectNext() {
let next: HTMLElement | null = null
@ -165,21 +220,6 @@
dialog?.close()
}
}
let selectedTab = tabs[0]
let selectedOption: number = 0
$: useScope = scope && selectedTab.id !== 'repos'
$: source = selectedTab.source
$: if (open) {
source.next(query)
}
$: if (open) {
dialog?.showModal()
input?.select()
} else {
dialog?.close()
}
</script>
<dialog bind:this={dialog} on:close>
@ -208,7 +248,7 @@
<Input
type="text"
bind:input
placeholder="Enter a fuzzy query"
{placeholder}
autofocus
value={query}
onInput={event => {
@ -227,58 +267,49 @@
{/if}
</div>
<ul role="listbox" bind:this={listbox} aria-label="Search results">
{#if $source.value}
{#each $source.value as item, index (item.item)}
{@const repo = item.item.repository.name}
{#if $source.pending}
<li class="message">Waiting for response...</li>
{:else if $source.error}
<li class="error"><Alert variant="danger">{$source.error.message}</Alert></li>
{:else if $source.value?.results}
{#each $source.value.results as item, index (item)}
{@const repo = item.repository.name}
{@const displayRepo = displayRepoName(repo)}
<li role="option" aria-selected={selectedOption === index} data-index={index}>
{#if item.item.type === 'repo'}
{#if item.type === 'repo'}
{@const matchOffset = repo.length - displayRepo.length}
<a href="/{item.item.repository.name}" on:click={handleClick}>
<span class="icon"><CodeHostIcon repository={item.item.repository.name} /></span>
<a href="/{item.repository.name}" on:click={handleClick}>
<span class="icon"><CodeHostIcon repository={item.repository.name} /></span>
<span class="label"
><EmphasizedLabel
label={displayRepo}
matches={item.positions}
offset={matchOffset}
/></span
><EmphasizedLabel label={displayRepo} offset={matchOffset} /></span
>
<span class="info">{repo}</span>
</a>
{:else if item.item.type == 'symbol'}
<a href={item.item.symbol.location.url} on:click={handleClick}>
<span class="icon"><SymbolKindIcon symbolKind={item.item.symbol.kind} /></span>
<span class="label"
><EmphasizedLabel
label={item.item.symbol.name}
matches={item.positions}
/></span
>
{:else if item.type == 'symbol'}
<a href={item.symbol.location.url} on:click={handleClick}>
<span class="icon"><SymbolKindIcon symbolKind={item.symbol.kind} /></span>
<span class="label"><EmphasizedLabel label={item.symbol.name} /></span>
<span class="info mono"
>{#if !useScope}{displayRepo} &middot; {/if}{item.item.file.path}</span
>{#if !useScope}{displayRepo} &middot; {/if}{item.file.path}</span
>
</a>
{:else if item.item.type == 'file'}
{@const fileName = item.item.file.name}
{@const folderName = dirname(item.item.file.path)}
<a href={item.item.file.url} on:click={handleClick}>
<span class="icon"><FileIcon file={item.item.file} inline /></span>
{:else if item.type == 'file'}
{@const fileName = item.file.name}
{@const folderName = dirname(item.file.path)}
<a href={item.file.url} on:click={handleClick}>
<span class="icon"><FileIcon file={item.file} inline /></span>
<span class="label"
><EmphasizedLabel
label={fileName}
matches={item.positions}
offset={folderName.length + 1}
/></span
><EmphasizedLabel label={fileName} offset={folderName.length + 1} /></span
>
<span class="info mono">
{#if !useScope}{displayRepo} &middot; {/if}
<EmphasizedLabel label={folderName} matches={item.positions} />
<EmphasizedLabel label={folderName} />
</span>
</a>
{/if}
</li>
{:else}
<li class="empty">No matches</li>
<li class="message">No matches</li>
{/each}
{/if}
</ul>
@ -381,8 +412,11 @@
}
}
.empty {
.message, .error {
padding: 1rem;
}
.message {
text-align: center;
color: var(--text-muted);
}

View File

@ -22,11 +22,24 @@
import { parseRepoRevision } from '$lib/shared'
import FuzzyFinder from './FuzzyFinder.svelte'
import { filesHotkey, reposHotkey, symbolsHotkey } from './keys'
import { allHotkey, filesHotkey, reposHotkey, symbolsHotkey } from './keys'
let finder: FuzzyFinder | undefined
let scope = ''
registerHotkey({
keys: allHotkey,
ignoreInputFields: false,
handler: event => {
event.stopPropagation()
fuzzyFinderState.set({
open: true,
selectedTabId: 'all',
})
return false
},
})
registerHotkey({
keys: reposHotkey,
ignoreInputFields: false,

View File

@ -1,5 +1,10 @@
import type { Keys } from '$lib/Hotkey'
export const allHotkey: Keys = {
key: 'ctrl+k',
mac: 'cmd+k',
}
export const reposHotkey: Keys = {
key: 'ctrl+i',
mac: 'cmd+i',

View File

@ -0,0 +1,24 @@
import { expect, describe, it } from 'vitest'
import { escapeQuery_TEST_ONLY as escapeQuery } from './sources'
describe('escapeQuery', () => {
it.each([
['repo:sourcegraph', '"repo:sourcegraph"'],
['file:main.go', '"file:main.go"'],
['r:sourcegraph f:main.go', '"r:sourcegraph" "f:main.go"'],
['OR AND NOT', '"OR" "AND" "NOT"'],
['( foo )', '"(" "foo" ")"'],
['(foo OR bar) AND baz', '"(foo" "OR" "bar)" "AND" "baz"'],
])('escapes special tokens: %s -> %s', (query, expected) => {
expect(escapeQuery(query)).toBe(expected)
})
it('preserves regex patterns', () => {
expect(escapeQuery('repo:^sourcegraph /f.o$/ bar')).toBe('"repo:^sourcegraph" /f.o$/ "bar"')
})
it('escapes quotes in patterns', () => {
expect(escapeQuery('foo"bar')).toBe('"foo\\"bar"')
})
})

View File

@ -1,12 +1,11 @@
import { Fzf, type FzfOptions, type FzfResultItem } from 'fzf'
import { Observable, Subject } from 'rxjs'
import { throttleTime, switchMap } from 'rxjs/operators'
import { Observable, Subject, from } from 'rxjs'
import { throttleTime, switchMap, startWith } from 'rxjs/operators'
import { readable, type Readable } from 'svelte/store'
import type { GraphQLClient } from '$lib/graphql'
import { mapOrThrow } from '$lib/graphql'
import { scanSearchQueryAsPatterns, stringHuman, PatternKind } from '$lib/shared'
import type { Loadable } from '$lib/utils'
import { CachedAsyncCompletionSource } from '$lib/web'
import { FuzzyFinderQuery, type FuzzyFinderFileMatch } from './FuzzyFinder.gql'
@ -30,181 +29,105 @@ interface RepositoryMatch {
export type FuzzyFinderResult = SymbolMatch | FileMatch | RepositoryMatch
export interface CompletionSource<T> extends Readable<Loadable<FzfResultItem<T>[]>> {
export interface CompletionSource extends Readable<Loadable<{ results: FuzzyFinderResult[] }>> {
next: (value: string) => void
}
export function createRepositorySource(client: GraphQLClient): CompletionSource<RepositoryMatch> {
const fzfOptions: FzfOptions<RepositoryMatch> = {
sort: true,
fuzzy: 'v2',
selector: item => item.repository.name,
forward: false,
limit: 50,
tiebreakers: [(a, b) => b.item.repository.stars - a.item.repository.stars, (a, b) => b.start - a.start],
}
const QUERY_THROTTLE_TIME = 200
const source = new CachedAsyncCompletionSource({
queryKey(value) {
return `type:repo count:50 repo:"${value}"`
},
async query(query) {
return client
.query(FuzzyFinderQuery, {
query,
})
.then(
mapOrThrow(response => {
const repos: [string, RepositoryMatch][] = []
for (const result of response.data?.search?.results.results ?? []) {
if (result.__typename === 'Repository') {
repos.push([result.name, { type: 'repo', repository: result }])
}
}
return repos
})
)
},
filter(entries, value) {
return new Fzf(entries, fzfOptions).find(value)
},
})
interface FuzzyFinderSourceOptions {
client: GraphQLClient
/**
* Generates the search query given the fuzzy finder input value.
*/
queryBuilder: (input: string) => string
}
/**
* Creates a completion source for the fuzzy finder.
*/
export function createFuzzyFinderSource({ client, queryBuilder }: FuzzyFinderSourceOptions): CompletionSource {
const subject = new Subject<string>()
const { subscribe } = fromObservable(
subject.pipe(
throttleTime(100, undefined, { leading: false, trailing: true }),
switchMap(value => toObservable(source, value))
),
{ pending: true, value: [], error: null }
)
return {
subscribe,
next: value => subject.next(value),
}
}
export function createFileSource(client: GraphQLClient, scope: () => string): CompletionSource<FileMatch> {
const fzfOptions: FzfOptions<FileMatch> = {
sort: true,
fuzzy: 'v2',
selector: item => item.file.path,
forward: false,
limit: 50,
tiebreakers: [(a, b) => b.item.repository.stars - a.item.repository.stars, (a, b) => b.start - a.start],
}
const source = new CachedAsyncCompletionSource({
dataCacheKey: scope,
queryKey(value, scope) {
return `type:path count:50 ${scope} file:"${value}"`
},
async query(query) {
return client
.query(FuzzyFinderQuery, {
query,
})
.then(
mapOrThrow(response => {
const repos: [string, FileMatch][] = []
for (const result of response.data?.search?.results.results ?? []) {
if (result.__typename === 'FileMatch') {
repos.push([
result.file.url,
{ type: 'file', file: result.file, repository: result.repository },
])
}
}
return repos
})
)
},
filter(entries, value) {
return new Fzf(entries, fzfOptions).find(value)
},
})
const subject = new Subject<string>()
const { subscribe } = fromObservable(
subject.pipe(
throttleTime(100, undefined, { leading: false, trailing: true }),
switchMap(value => toObservable(source, value))
),
{ pending: true, value: [], error: null }
)
return {
subscribe,
next: value => subject.next(value),
}
}
export function createSymbolSource(client: GraphQLClient, scope: () => string): CompletionSource<SymbolMatch> {
const fzfOptions: FzfOptions<SymbolMatch> = {
sort: true,
fuzzy: 'v2',
selector: item => item.symbol.name,
limit: 50,
tiebreakers: [(a, b) => b.item.repository.stars - a.item.repository.stars, (a, b) => b.start - a.start],
}
const source = new CachedAsyncCompletionSource({
dataCacheKey: scope,
queryKey(value, scope) {
return `type:symbol count:50 ${scope} "${value}"`
},
async query(query) {
return client
.query(FuzzyFinderQuery, {
query,
})
.then(
mapOrThrow(response => {
const results: [string, SymbolMatch][] = []
for (const result of response.data?.search?.results.results ?? []) {
if (result.__typename === 'FileMatch') {
for (const symbol of result.symbols) {
results.push([
symbol.location.url,
{ type: 'symbol', file: result.file, repository: result.repository, symbol },
])
throttleTime(QUERY_THROTTLE_TIME, undefined, { leading: false, trailing: true }),
switchMap(value =>
from(
client
.query(FuzzyFinderQuery, { query: queryBuilder(value) })
.then(
mapOrThrow(response => {
const results: FuzzyFinderResult[] = []
for (const result of response?.data?.search?.results.results ?? []) {
switch (result.__typename) {
case 'Repository':
results.push({ type: 'repo', repository: result })
break
case 'FileMatch':
if (result.symbols.length === 0) {
// This is a file match
results.push({
type: 'file',
file: result.file,
repository: result.repository,
})
} else {
// This is a symbol match
for (const symbol of result.symbols) {
results.push({
type: 'symbol',
file: result.file,
repository: result.repository,
symbol,
})
}
}
}
}
}
}
return results
})
)
},
filter(entries, value) {
return new Fzf(entries, fzfOptions).find(value)
},
})
const subject = new Subject<string>()
const { subscribe } = fromObservable(
subject.pipe(
throttleTime(100, undefined, { leading: false, trailing: true }),
switchMap(value => toObservable(source, value))
return { results }
})
)
.then(
value => ({ pending: false, value, error: null }),
error => ({ pending: false, value: { results: [] }, error })
)
).pipe(startWith({ pending: true, value: { results: [] }, error: null }))
)
),
{ pending: true, value: [], error: null }
{ pending: false, value: { results: [] }, error: null }
)
return {
subscribe,
next: value => subject.next(value),
next: value => subject.next(escapeQuery(value.trim())),
}
}
function toObservable<T, U>(source: CachedAsyncCompletionSource<T, U>, value: string): Observable<Loadable<U[]>> {
return new Observable(subscriber => {
const result = source.query(value, results => results)
subscriber.next({ pending: true, value: result.result, error: null })
result.next().then(result => {
subscriber.next({ pending: false, value: result.result, error: null })
subscriber.complete()
})
})
/**
* Converts sepecific token types to normal patterns. E.g. `repo:sourcegraph` should be escaped because
* we don't want it to be interpreted as a filter.
*
* @param query The query to escape.
* @returns The escaped query.
*/
function escapeQuery(query: string): string {
const result = scanSearchQueryAsPatterns(query)
if (result.type !== 'success') {
return query
}
return stringHuman(
result.term.map(token =>
token.type === 'pattern' && token.kind === PatternKind.Literal
? { ...token, value: `"${escapeQuotes(token.value)}"` }
: token
)
)
}
export const escapeQuery_TEST_ONLY = escapeQuery
function escapeQuotes(value: string): string {
return value.replaceAll(/"/g, '\\"')
}
function fromObservable<T>(observable: Observable<T>, initialValue: T): Readable<T> {

View File

@ -67,8 +67,9 @@ export {
type RelevantTokenResult,
EMPTY_RELEVANT_TOKEN_RESULT,
} from '@sourcegraph/shared/src/search/query/analyze'
export { scanSearchQuery } from '@sourcegraph/shared/src/search/query/scanner'
export { KeywordKind, type Token } from '@sourcegraph/shared/src/search/query/token'
export { scanSearchQuery, scanSearchQueryAsPatterns } from '@sourcegraph/shared/src/search/query/scanner'
export { stringHuman } from '@sourcegraph/shared/src/search/query/printer'
export { KeywordKind, PatternKind, type Token } from '@sourcegraph/shared/src/search/query/token'
export { FilterType } from '@sourcegraph/shared/src/search/query/filters'
export { getGlobalSearchContextFilter, findFilter, FilterKind } from '@sourcegraph/shared/src/search/query/query'
export { isFilterOfType } from '@sourcegraph/shared/src/search/query/utils'

View File

@ -36,7 +36,8 @@ test.describe('cloned repository', () => {
await expect(page.getByRole('heading', { name: 'sourcegraph/sourcegraph' })).toBeVisible()
})
test('has prepopulated search bar', async ({ page }) => {
// TODO: Better test to ensure that we are testing the search input
test.fixme('has prepopulated search bar', async ({ page }) => {
await expect(page.getByText('repo:^github\\.com/sourcegraph')).toBeVisible()
})
})
@ -120,7 +121,7 @@ test.describe('repo menu', () => {
test('click switch repo', async ({ page }) => {
await page.getByRole('heading', { name: 'sourcegraph/sourcegraph' }).click()
await page.getByRole('menuitem', { name: 'Switch repo' }).click()
await expect(page.getByPlaceholder('Enter a fuzzy query')).toBeVisible()
await expect(page.getByPlaceholder('Find repositories...')).toBeVisible()
})
test('settings url', async ({ page }) => {