From be985fbd8855ffd2afc30e89dd71dbca0ee1de94 Mon Sep 17 00:00:00 2001 From: Quinn Slack Date: Mon, 18 Mar 2024 08:16:50 -0700 Subject: [PATCH] speed up & respect ignores in finding workspace files for file @-mentions (#3433) --- vscode/CHANGELOG.md | 1 + .../chat/chat-view/SimpleChatPanelProvider.ts | 87 +++++++------------ vscode/src/chat/context/chatContext.ts | 37 ++++++++ .../src/editor/utils/editor-context.test.ts | 9 -- vscode/src/editor/utils/editor-context.ts | 29 ++----- vscode/src/editor/utils/findWorkspaceFiles.ts | 78 +++++++++++++++++ vscode/src/editor/utils/index.ts | 9 -- vscode/test/e2e/chat-atFile.test.ts | 3 +- 8 files changed, 157 insertions(+), 96 deletions(-) create mode 100644 vscode/src/chat/context/chatContext.ts create mode 100644 vscode/src/editor/utils/findWorkspaceFiles.ts diff --git a/vscode/CHANGELOG.md b/vscode/CHANGELOG.md index 8dd286ad4613..96e4c67d448d 100644 --- a/vscode/CHANGELOG.md +++ b/vscode/CHANGELOG.md @@ -19,6 +19,7 @@ This is a log of all notable changes to Cody for VS Code. [Unreleased] changes a - Document: Fixed an issue where the generated documentation would be incorrectly inserted for Python. Cody will now follow PEP 257 – Docstring Conventions. [pull/3275](https://github.com/sourcegraph/cody/pull/3275) - Edit: Fixed incorrect decorations being shown for edits that only insert new code. [pull/3424](https://github.com/sourcegraph/cody/pull/3424) +- When `@`-mentioning files in chat and edits, the list of fuzzy-matching files is shown much faster (which is especially noticeable in large workspaces). ### Changed diff --git a/vscode/src/chat/chat-view/SimpleChatPanelProvider.ts b/vscode/src/chat/chat-view/SimpleChatPanelProvider.ts index 22c98d1663a3..902297705773 100644 --- a/vscode/src/chat/chat-view/SimpleChatPanelProvider.ts +++ b/vscode/src/chat/chat-view/SimpleChatPanelProvider.ts @@ -28,12 +28,7 @@ import { import type { View } from '../../../webviews/NavBar' import { getFullConfig } from '../../configuration' import { type RemoteSearch, RepoInclusion } from '../../context/remote-search' -import { - fillInContextItemContent, - getFileContextFiles, - getOpenTabsContextFile, - getSymbolContextFiles, -} from '../../editor/utils/editor-context' +import { fillInContextItemContent } from '../../editor/utils/editor-context' import type { VSCodeEditor } from '../../editor/vscode-editor' import { ContextStatusAggregator } from '../../local-context/enhanced-context-status' import type { LocalEmbeddingsController } from '../../local-context/local-embeddings' @@ -66,6 +61,7 @@ import { chatModel } from '../../models' import { getContextWindowForModel } from '../../models/utilts' import { recordExposedExperimentsToSpan } from '../../services/open-telemetry/utils' import type { MessageErrorType } from '../MessageProvider' +import { getChatContextItemsForMention } from '../context/chatContext' import type { ChatSubmitType, ConfigurationSubsetForWebview, @@ -581,63 +577,44 @@ export class SimpleChatPanelProvider implements vscode.Disposable, ChatSession { } private async handleGetUserContextFilesCandidates(query: string): Promise { - const source = 'chat' - if (!query.length) { - telemetryService.log('CodyVSCodeExtension:at-mention:executed', { source }) - telemetryRecorder.recordEvent('cody.at-mention', 'executed', { privateMetadata: { source } }) - - const tabs = await getOpenTabsContextFile() - void this.postMessage({ - type: 'userContextFiles', - userContextFiles: tabs, - }) - return - } - - // Log when query only has 1 char to avoid logging the same query repeatedly - if (query.length === 1) { - const type = query.startsWith('#') ? 'symbol' : 'file' - telemetryService.log(`CodyVSCodeExtension:at-mention:${type}:executed`, { source }) - telemetryRecorder.recordEvent(`cody.at-mention.${type}`, 'executed', { - privateMetadata: { source }, - }) - } - // Cancel previously in-flight query. const cancellation = new vscode.CancellationTokenSource() this.contextFilesQueryCancellation?.cancel() this.contextFilesQueryCancellation = cancellation + const source = 'chat' + const scopedTelemetryRecorder: Parameters[2] = { + empty: () => { + telemetryService.log('CodyVSCodeExtension:at-mention:executed', { source }) + telemetryRecorder.recordEvent('cody.at-mention', 'executed', { + privateMetadata: { source }, + }) + }, + withType: type => { + telemetryService.log(`CodyVSCodeExtension:at-mention:${type}:executed`, { source }) + telemetryRecorder.recordEvent(`cody.at-mention.${type}`, 'executed', { + privateMetadata: { source }, + }) + }, + } + try { - const MAX_RESULTS = 20 - if (query.startsWith('#')) { - // It would be nice if the VS Code symbols API supports - // cancellation, but it doesn't - const symbolResults = await getSymbolContextFiles(query.slice(1), MAX_RESULTS) - // Check if cancellation was requested while getFileContextFiles - // was executing, which means a new request has already begun - // (i.e. prevent race conditions where slow old requests get - // processed after later faster requests) - if (!cancellation.token.isCancellationRequested) { - await this.postMessage({ - type: 'userContextFiles', - userContextFiles: symbolResults, - }) - } - } else { - const fileResults = await getFileContextFiles(query, MAX_RESULTS, cancellation.token) - // Check if cancellation was requested while getFileContextFiles - // was executing, which means a new request has already begun - // (i.e. prevent race conditions where slow old requests get - // processed after later faster requests) - if (!cancellation.token.isCancellationRequested) { - await this.postMessage({ - type: 'userContextFiles', - userContextFiles: fileResults, - }) - } + const items = await getChatContextItemsForMention( + query, + cancellation.token, + scopedTelemetryRecorder + ) + if (cancellation.token.isCancellationRequested) { + return } + void this.postMessage({ + type: 'userContextFiles', + userContextFiles: items, + }) } catch (error) { + if (cancellation.token.isCancellationRequested) { + return + } this.postError(new Error(`Error retrieving context files: ${error}`)) } } diff --git a/vscode/src/chat/context/chatContext.ts b/vscode/src/chat/context/chatContext.ts new file mode 100644 index 000000000000..16ced211fdf9 --- /dev/null +++ b/vscode/src/chat/context/chatContext.ts @@ -0,0 +1,37 @@ +import type { ContextItem } from '@sourcegraph/cody-shared' +import type * as vscode from 'vscode' +import { + getFileContextFiles, + getOpenTabsContextFile, + getSymbolContextFiles, +} from '../../editor/utils/editor-context' + +export async function getChatContextItemsForMention( + query: string, + cancellationToken: vscode.CancellationToken, + telemetryRecorder?: { + empty: () => void + withType: (type: 'symbol' | 'file') => void + } +): Promise { + // Logging: log when the at-mention starts, and then log when we know the type (after the 1st + // character is typed). Don't log otherwise because we would be logging prefixes of the same + // query repeatedly, which is not needed. + const queryType = query.startsWith('#') ? 'symbol' : 'file' + if (query === '') { + telemetryRecorder?.empty() + } else if (query.length === 1) { + telemetryRecorder?.withType(queryType) + } + + if (query.length === 0) { + return getOpenTabsContextFile() + } + + const MAX_RESULTS = 20 + if (query.startsWith('#')) { + // It would be nice if the VS Code symbols API supports cancellation, but it doesn't + return getSymbolContextFiles(query.slice(1), MAX_RESULTS) + } + return getFileContextFiles(query, MAX_RESULTS, cancellationToken) +} diff --git a/vscode/src/editor/utils/editor-context.test.ts b/vscode/src/editor/utils/editor-context.test.ts index c5999eb26b00..b6e739a0a76a 100644 --- a/vscode/src/editor/utils/editor-context.test.ts +++ b/vscode/src/editor/utils/editor-context.test.ts @@ -125,15 +125,6 @@ describe('getFileContextFiles', () => { expect(vscode.workspace.findFiles).toBeCalledTimes(1) }) - - it('cancels previous requests', async () => { - vscode.workspace.findFiles = vi.fn().mockResolvedValueOnce([]) - const cancellation = new vscode.CancellationTokenSource() - await getFileContextFiles('search', 5, cancellation.token) - await getFileContextFiles('search', 5, new vscode.CancellationTokenSource().token) - expect(cancellation.token.isCancellationRequested) - expect(vscode.workspace.findFiles).toBeCalledTimes(2) - }) }) describe('filterLargeFiles', () => { diff --git a/vscode/src/editor/utils/editor-context.ts b/vscode/src/editor/utils/editor-context.ts index 8ba2957fe87c..58c3940ba033 100644 --- a/vscode/src/editor/utils/editor-context.ts +++ b/vscode/src/editor/utils/editor-context.ts @@ -24,29 +24,16 @@ import { type ContextItemWithContent, } from '@sourcegraph/cody-shared/src/codebase-context/messages' import { CHARS_PER_TOKEN } from '@sourcegraph/cody-shared/src/prompt/constants' -import { getOpenTabsUris, getWorkspaceSymbols } from '.' +import { getOpenTabsUris } from '.' import { toVSCodeRange } from '../../common/range' - -const findWorkspaceFiles = async ( - cancellationToken: vscode.CancellationToken -): Promise => { - // TODO(toolmantim): Add support for the search.exclude option, e.g. - // Object.keys(vscode.workspace.getConfiguration().get('search.exclude', - // {})) - const fileExcludesPattern = - '**/{*.env,.git/,.class,out/,dist/,build/,snap,node_modules/,__pycache__/}**' - // TODO(toolmantim): Check this performs with remote workspaces (do we need a UI spinner etc?) - return vscode.workspace.findFiles('', fileExcludesPattern, undefined, cancellationToken) -} +import { findWorkspaceFiles } from './findWorkspaceFiles' // Some matches we don't want to ignore because they might be valid code (for example `bin/` in Dart) // but could also be junk (`bin/` in .NET). If a file path contains a segment matching any of these // items it will be ranked low unless the users query contains the exact segment. const lowScoringPathSegments = ['bin'] -// This is expensive for large repos (e.g. Chromium), so we only do it max once -// every 10 seconds. It also handily supports a cancellation callback to use -// with the cancellation token to discard old requests. +// This is expensive for large repos (e.g. Chromium), so we only do it max once every 10 seconds. const throttledFindFiles = throttle(findWorkspaceFiles, 10000) /** @@ -63,12 +50,8 @@ export async function getFileContextFiles( if (!query.trim()) { return [] } - token.onCancellationRequested(() => { - throttledFindFiles.cancel() - }) const uris = await throttledFindFiles(token) - if (!uris) { return [] } @@ -133,7 +116,11 @@ export async function getSymbolContextFiles( return [] } - const queryResults = await getWorkspaceSymbols(query) // doesn't support cancellation tokens :( + // doesn't support cancellation tokens :( + const queryResults = await vscode.commands.executeCommand( + 'vscode.executeWorkspaceSymbolProvider', + query + ) const relevantQueryResults = queryResults?.filter( symbol => diff --git a/vscode/src/editor/utils/findWorkspaceFiles.ts b/vscode/src/editor/utils/findWorkspaceFiles.ts new file mode 100644 index 000000000000..df8ce0f0a246 --- /dev/null +++ b/vscode/src/editor/utils/findWorkspaceFiles.ts @@ -0,0 +1,78 @@ +import * as vscode from 'vscode' + +/** + * Find all files in all workspace folders, respecting the user's `files.exclude`, `search.exclude`, + * and other exclude settings. The intent is to match the files shown by VS Code's built-in `Go to + * File...` command. + */ +export async function findWorkspaceFiles( + cancellationToken: vscode.CancellationToken +): Promise { + return ( + await Promise.all( + (vscode.workspace.workspaceFolders ?? [null]).map(async workspaceFolder => + vscode.workspace.findFiles( + workspaceFolder ? new vscode.RelativePattern(workspaceFolder, '**') : '', + await getExcludePattern(workspaceFolder), + undefined, + cancellationToken + ) + ) + ) + ).flat() +} + +type IgnoreRecord = Record + +async function getExcludePattern(workspaceFolder: vscode.WorkspaceFolder | null): Promise { + const config = vscode.workspace.getConfiguration('', workspaceFolder) + const filesExclude = config.get('files.exclude', {}) + const searchExclude = config.get('search.exclude', {}) + + const useIgnoreFiles = config.get('search.useIgnoreFiles') + const gitignoreExclude = + useIgnoreFiles && workspaceFolder + ? await readIgnoreFile(vscode.Uri.joinPath(workspaceFolder.uri, '.gitignore')) + : {} + const ignoreExclude = + useIgnoreFiles && workspaceFolder + ? await readIgnoreFile(vscode.Uri.joinPath(workspaceFolder.uri, '.ignore')) + : {} + + const mergedExclude: IgnoreRecord = { + ...filesExclude, + ...searchExclude, + ...gitignoreExclude, + ...ignoreExclude, + } + const excludePatterns = Object.keys(mergedExclude).filter(key => mergedExclude[key] === true) + return `{${excludePatterns.join(',')}}` +} + +async function readIgnoreFile(uri: vscode.Uri): Promise { + const ignore: IgnoreRecord = {} + try { + const data = await vscode.workspace.fs.readFile(uri) + for (let line of Buffer.from(data).toString('utf-8').split('\n')) { + if (line.startsWith('!')) { + continue + } + + // Strip comment and trailing whitespace. + line = line.replace(/\s*(#.*)?$/, '') + + if (line === '') { + continue + } + + if (line.endsWith('/')) { + line = line.slice(0, -1) + } + if (!line.startsWith('/') && !line.startsWith('**/')) { + line = `**/${line}` + } + ignore[line] = true + } + } catch {} + return ignore +} diff --git a/vscode/src/editor/utils/index.ts b/vscode/src/editor/utils/index.ts index 958058c74d6d..4c292858c1ad 100644 --- a/vscode/src/editor/utils/index.ts +++ b/vscode/src/editor/utils/index.ts @@ -32,15 +32,6 @@ export async function getSmartSelection( return getSelectionAroundLine(document, target) } -/** - * Searches for workspace symbols matching the given query string. - * @param query - The search query string. - * @returns A promise resolving to the array of SymbolInformation objects representing the matched workspace symbols. - */ -export async function getWorkspaceSymbols(query = ''): Promise { - return vscode.commands.executeCommand('vscode.executeWorkspaceSymbolProvider', query) -} - /** * Returns an array of URI's for all unique open editor tabs. * diff --git a/vscode/test/e2e/chat-atFile.test.ts b/vscode/test/e2e/chat-atFile.test.ts index ef6c35bd59be..9924df229e5b 100644 --- a/vscode/test/e2e/chat-atFile.test.ts +++ b/vscode/test/e2e/chat-atFile.test.ts @@ -102,8 +102,7 @@ test.extend({ chatPanelFrame.getByRole('option', { name: withPlatformSlashes('visualize.go') }) ).toBeVisible() await chatInput.press('ArrowDown') // second item (visualize.go) - await chatInput.press('ArrowDown') // third item (.vscode/settings.json) - await chatInput.press('ArrowDown') // wraps back to first item + await chatInput.press('ArrowDown') // wraps back to first item (var.go) await chatInput.press('ArrowDown') // second item again await chatInput.press('Tab') await expect(chatInput).toHaveText(