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

Commit ce7531f

Browse files
fklingfkling
authored
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
1 parent 7a9d2b0 commit ce7531f

File tree

9 files changed

+247
-231
lines changed

9 files changed

+247
-231
lines changed

‎client/shared/src/search/query/scanner.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,3 +613,20 @@ export const succeedScan = (query: string): Token[] => {
613613
}
614614
return result.term
615615
}
616+
617+
const patternScanner = zeroOrMore(
618+
oneOf<Term>(
619+
whitespace,
620+
toPatternResult(quoted('/'), PatternKind.Regexp),
621+
// We don't use scanPattern or literal here because we want to treat parenthesis as regular characters
622+
toPatternResult(scanToken(/\S+/), PatternKind.Literal)
623+
)
624+
)
625+
626+
/**
627+
* Scans the search query as a sequence of patterns only. This is used in situations where we don't want
628+
* to interpret filters or keywords.
629+
*/
630+
export function scanSearchQueryAsPatterns(query: string): ScanResult<Token[]> {
631+
return patternScanner(query, 0)
632+
}

‎client/web-sveltekit/src/lib/fuzzyfinder/FuzzyFinder.gql

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
query FuzzyFinderQuery($query: String!) {
2-
search(query: $query) {
2+
search(query: $query, version: V3) {
33
results {
44
results {
55
...FuzzyFinderFileMatch
@@ -24,11 +24,9 @@ fragment FuzzyFinderFileMatch on FileMatch {
2424
}
2525
repository {
2626
name
27-
stars
2827
}
2928
}
3029

3130
fragment FuzzyFinderRepository on Repository {
3231
name
33-
stars
3432
}

‎client/web-sveltekit/src/lib/fuzzyfinder/FuzzyFinder.svelte

Lines changed: 99 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<script lang="ts" context="module">
22
export enum FuzzyFinderTabType {
3+
All = 'all',
34
Repos = 'repos',
45
Symbols = 'symbols',
56
Files = 'files',
@@ -23,17 +24,11 @@
2324
import SymbolKindIcon from '$lib/search/SymbolKindIcon.svelte'
2425
import { displayRepoName } from '$lib/shared'
2526
import TabsHeader, { type Tab } from '$lib/TabsHeader.svelte'
26-
import { Input } from '$lib/wildcard'
27+
import { Alert, Input } from '$lib/wildcard'
2728
import Button from '$lib/wildcard/Button.svelte'
2829
29-
import { filesHotkey, reposHotkey, symbolsHotkey } from './keys'
30-
import {
31-
createRepositorySource,
32-
type CompletionSource,
33-
createFileSource,
34-
type FuzzyFinderResult,
35-
createSymbolSource,
36-
} from './sources'
30+
import { allHotkey, filesHotkey, reposHotkey, symbolsHotkey } from './keys'
31+
import { type CompletionSource, createFuzzyFinderSource } from './sources'
3732
3833
export let open = false
3934
export let scope = ''
@@ -45,17 +40,77 @@
4540
}
4641
}
4742
43+
const client = getGraphQLClient()
44+
const tabs: (Tab & { source: CompletionSource })[] = [
45+
{
46+
id: 'all',
47+
title: 'All',
48+
shortcut: allHotkey,
49+
source: createFuzzyFinderSource({
50+
client,
51+
queryBuilder: value =>
52+
`patterntype:keyword (type:repo OR type:path OR type:symbol) count:50 ${scope} ${value}`,
53+
}),
54+
},
55+
{
56+
id: 'repos',
57+
title: 'Repos',
58+
shortcut: reposHotkey,
59+
source: createFuzzyFinderSource({
60+
client,
61+
queryBuilder: value => `patterntype:keyword type:repo count:50 ${value}`,
62+
}),
63+
},
64+
{
65+
id: 'symbols',
66+
title: 'Symbols',
67+
shortcut: symbolsHotkey,
68+
source: createFuzzyFinderSource({
69+
client,
70+
queryBuilder: value => `patterntype:keyword type:symbol count:50 ${scope} ${value}`,
71+
}),
72+
},
73+
{
74+
id: 'files',
75+
title: 'Files',
76+
shortcut: filesHotkey,
77+
source: createFuzzyFinderSource({
78+
client,
79+
queryBuilder: value => `patterntype:keyword type:path count:50 ${scope} ${value}`,
80+
}),
81+
},
82+
]
83+
4884
let dialog: HTMLDialogElement | undefined
4985
let listbox: HTMLElement | undefined
5086
let input: HTMLInputElement | undefined
5187
let query = ''
88+
let selectedTab = tabs[0]
89+
let selectedOption: number = 0
5290
53-
const client = getGraphQLClient()
54-
const tabs: (Tab & { source: CompletionSource<FuzzyFinderResult> })[] = [
55-
{ id: 'repos', title: 'Repos', shortcut: reposHotkey, source: createRepositorySource(client) },
56-
{ id: 'symbols', title: 'Symbols', shortcut: symbolsHotkey, source: createSymbolSource(client, () => scope) },
57-
{ id: 'files', title: 'Files', shortcut: filesHotkey, source: createFileSource(client, () => scope) },
58-
]
91+
$: useScope = scope && selectedTab.id !== 'repos'
92+
$: source = selectedTab.source
93+
$: if (open) {
94+
source.next(query)
95+
}
96+
$: if (open) {
97+
dialog?.showModal()
98+
input?.select()
99+
} else {
100+
dialog?.close()
101+
}
102+
$: placeholder = (function () {
103+
switch (selectedTab.id) {
104+
case 'repos':
105+
return 'Find repositories...'
106+
case 'symbols':
107+
return 'Find symbols...'
108+
case 'files':
109+
return 'Find files...'
110+
default:
111+
return 'Find anything...'
112+
}
113+
})()
59114
60115
function selectNext() {
61116
let next: HTMLElement | null = null
@@ -165,21 +220,6 @@
165220
dialog?.close()
166221
}
167222
}
168-
169-
let selectedTab = tabs[0]
170-
let selectedOption: number = 0
171-
172-
$: useScope = scope && selectedTab.id !== 'repos'
173-
$: source = selectedTab.source
174-
$: if (open) {
175-
source.next(query)
176-
}
177-
$: if (open) {
178-
dialog?.showModal()
179-
input?.select()
180-
} else {
181-
dialog?.close()
182-
}
183223
</script>
184224

185225
<dialog bind:this={dialog} on:close>
@@ -208,7 +248,7 @@
208248
<Input
209249
type="text"
210250
bind:input
211-
placeholder="Enter a fuzzy query"
251+
{placeholder}
212252
autofocus
213253
value={query}
214254
onInput={event => {
@@ -227,58 +267,49 @@
227267
{/if}
228268
</div>
229269
<ul role="listbox" bind:this={listbox} aria-label="Search results">
230-
{#if $source.value}
231-
{#each $source.value as item, index (item.item)}
232-
{@const repo = item.item.repository.name}
270+
{#if $source.pending}
271+
<li class="message">Waiting for response...</li>
272+
{:else if $source.error}
273+
<li class="error"><Alert variant="danger">{$source.error.message}</Alert></li>
274+
{:else if $source.value?.results}
275+
{#each $source.value.results as item, index (item)}
276+
{@const repo = item.repository.name}
233277
{@const displayRepo = displayRepoName(repo)}
234278
<li role="option" aria-selected={selectedOption === index} data-index={index}>
235-
{#if item.item.type === 'repo'}
279+
{#if item.type === 'repo'}
236280
{@const matchOffset = repo.length - displayRepo.length}
237-
<a href="/{item.item.repository.name}" on:click={handleClick}>
238-
<span class="icon"><CodeHostIcon repository={item.item.repository.name} /></span>
281+
<a href="/{item.repository.name}" on:click={handleClick}>
282+
<span class="icon"><CodeHostIcon repository={item.repository.name} /></span>
239283
<span class="label"
240-
><EmphasizedLabel
241-
label={displayRepo}
242-
matches={item.positions}
243-
offset={matchOffset}
244-
/></span
284+
><EmphasizedLabel label={displayRepo} offset={matchOffset} /></span
245285
>
246286
<span class="info">{repo}</span>
247287
</a>
248-
{:else if item.item.type == 'symbol'}
249-
<a href={item.item.symbol.location.url} on:click={handleClick}>
250-
<span class="icon"><SymbolKindIcon symbolKind={item.item.symbol.kind} /></span>
251-
<span class="label"
252-
><EmphasizedLabel
253-
label={item.item.symbol.name}
254-
matches={item.positions}
255-
/></span
256-
>
288+
{:else if item.type == 'symbol'}
289+
<a href={item.symbol.location.url} on:click={handleClick}>
290+
<span class="icon"><SymbolKindIcon symbolKind={item.symbol.kind} /></span>
291+
<span class="label"><EmphasizedLabel label={item.symbol.name} /></span>
257292
<span class="info mono"
258-
>{#if !useScope}{displayRepo} &middot; {/if}{item.item.file.path}</span
293+
>{#if !useScope}{displayRepo} &middot; {/if}{item.file.path}</span
259294
>
260295
</a>
261-
{:else if item.item.type == 'file'}
262-
{@const fileName = item.item.file.name}
263-
{@const folderName = dirname(item.item.file.path)}
264-
<a href={item.item.file.url} on:click={handleClick}>
265-
<span class="icon"><FileIcon file={item.item.file} inline /></span>
296+
{:else if item.type == 'file'}
297+
{@const fileName = item.file.name}
298+
{@const folderName = dirname(item.file.path)}
299+
<a href={item.file.url} on:click={handleClick}>
300+
<span class="icon"><FileIcon file={item.file} inline /></span>
266301
<span class="label"
267-
><EmphasizedLabel
268-
label={fileName}
269-
matches={item.positions}
270-
offset={folderName.length + 1}
271-
/></span
302+
><EmphasizedLabel label={fileName} offset={folderName.length + 1} /></span
272303
>
273304
<span class="info mono">
274305
{#if !useScope}{displayRepo} &middot; {/if}
275-
<EmphasizedLabel label={folderName} matches={item.positions} />
306+
<EmphasizedLabel label={folderName} />
276307
</span>
277308
</a>
278309
{/if}
279310
</li>
280311
{:else}
281-
<li class="empty">No matches</li>
312+
<li class="message">No matches</li>
282313
{/each}
283314
{/if}
284315
</ul>
@@ -381,8 +412,11 @@
381412
}
382413
}
383414
384-
.empty {
415+
.message, .error {
385416
padding: 1rem;
417+
}
418+
419+
.message {
386420
text-align: center;
387421
color: var(--text-muted);
388422
}

‎client/web-sveltekit/src/lib/fuzzyfinder/FuzzyFinderContainer.svelte

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,24 @@
2222
import { parseRepoRevision } from '$lib/shared'
2323
2424
import FuzzyFinder from './FuzzyFinder.svelte'
25-
import { filesHotkey, reposHotkey, symbolsHotkey } from './keys'
25+
import { allHotkey, filesHotkey, reposHotkey, symbolsHotkey } from './keys'
2626
2727
let finder: FuzzyFinder | undefined
2828
let scope = ''
2929
30+
registerHotkey({
31+
keys: allHotkey,
32+
ignoreInputFields: false,
33+
handler: event => {
34+
event.stopPropagation()
35+
fuzzyFinderState.set({
36+
open: true,
37+
selectedTabId: 'all',
38+
})
39+
return false
40+
},
41+
})
42+
3043
registerHotkey({
3144
keys: reposHotkey,
3245
ignoreInputFields: false,

‎client/web-sveltekit/src/lib/fuzzyfinder/keys.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import type { Keys } from '$lib/Hotkey'
22

3+
export const allHotkey: Keys = {
4+
key: 'ctrl+k',
5+
mac: 'cmd+k',
6+
}
7+
38
export const reposHotkey: Keys = {
49
key: 'ctrl+i',
510
mac: 'cmd+i',

‎client/web-sveltekit/src/lib/fuzzyfinder/sources.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { expect, describe, it } from 'vitest'
2+
3+
import { escapeQuery_TEST_ONLY as escapeQuery } from './sources'
4+
5+
describe('escapeQuery', () => {
6+
it.each([
7+
['repo:sourcegraph', '"repo:sourcegraph"'],
8+
['file:main.go', '"file:main.go"'],
9+
['r:sourcegraph f:main.go', '"r:sourcegraph" "f:main.go"'],
10+
['OR AND NOT', '"OR" "AND" "NOT"'],
11+
['( foo )', '"(" "foo" ")"'],
12+
['(foo OR bar) AND baz', '"(foo" "OR" "bar)" "AND" "baz"'],
13+
])('escapes special tokens: %s -> %s', (query, expected) => {
14+
expect(escapeQuery(query)).toBe(expected)
15+
})
16+
17+
it('preserves regex patterns', () => {
18+
expect(escapeQuery('repo:^sourcegraph /f.o$/ bar')).toBe('"repo:^sourcegraph" /f.o$/ "bar"')
19+
})
20+
21+
it('escapes quotes in patterns', () => {
22+
expect(escapeQuery('foo"bar')).toBe('"foo\\"bar"')
23+
})
24+
})

0 commit comments

Comments
 (0)