From 916afeb0e2bd4e4589bf95cd474633ab4aa69bc0 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Tue, 8 Oct 2024 15:30:40 +0200 Subject: [PATCH 01/18] add commands to navigate through edits (#230791) --- .../contrib/chat/browser/chat.contribution.ts | 3 + .../contrib/chat/browser/chatEditorActions.ts | 68 +++++++++++++++ .../chat/browser/chatEditorController.ts | 85 ++++++++++++++++++- 3 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 src/vs/workbench/contrib/chat/browser/chatEditorActions.ts diff --git a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts index 2ccf14071767f7..0c1ceaa8b6bd8f 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts @@ -73,6 +73,8 @@ import { EditorContributionInstantiation, registerEditorContribution } from '../ import { ChatEditorController } from './chatEditorController.js'; import { LanguageModelToolsService } from './languageModelToolsService.js'; import { ChatEditorSaving } from './chatEditorSaving.js'; +import { registerChatEditorActions } from './chatEditorActions.js'; + // Register configuration const configurationRegistry = Registry.as(ConfigurationExtensions.Configuration); @@ -298,6 +300,7 @@ registerMoveActions(); registerNewChatActions(); registerChatContextActions(); registerChatDeveloperActions(); +registerChatEditorActions(); registerEditorFeature(ChatPasteProvidersFeature); registerEditorContribution(ChatEditorController.ID, ChatEditorController, EditorContributionInstantiation.Eventually); diff --git a/src/vs/workbench/contrib/chat/browser/chatEditorActions.ts b/src/vs/workbench/contrib/chat/browser/chatEditorActions.ts new file mode 100644 index 00000000000000..4ba86bc67aa7ac --- /dev/null +++ b/src/vs/workbench/contrib/chat/browser/chatEditorActions.ts @@ -0,0 +1,68 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +import { isCodeEditor } from '../../../../editor/browser/editorBrowser.js'; +import { localize2 } from '../../../../nls.js'; +import { ServicesAccessor } from '../../../../editor/browser/editorExtensions.js'; +import { Codicon } from '../../../../base/common/codicons.js'; +import { Action2, MenuId, registerAction2 } from '../../../../platform/actions/common/actions.js'; +import { KeybindingWeight } from '../../../../platform/keybinding/common/keybindingsRegistry.js'; +import { KeyCode, KeyMod } from '../../../../base/common/keyCodes.js'; +import { CHAT_CATEGORY } from './actions/chatActions.js'; +import { ChatEditorController, ctxHasEditorModification } from './chatEditorController.js'; +import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextkey.js'; +import { EditorContextKeys } from '../../../../editor/common/editorContextKeys.js'; +import { IEditorService } from '../../../services/editor/common/editorService.js'; + +abstract class NavigateAction extends Action2 { + + constructor(readonly next: boolean) { + super({ + id: next + ? 'chat.action.navigateNext' + : 'chat.action.navigatePrevious', + title: next + ? localize2('next', 'Go to Next Chat Edit') + : localize2('prev', 'Go to Previous Chat Edit'), + category: CHAT_CATEGORY, + icon: next ? Codicon.arrowDown : Codicon.arrowUp, + keybinding: { + primary: next + ? KeyMod.Alt | KeyCode.F5 + : KeyMod.Alt | KeyMod.Shift | KeyCode.F5, + weight: KeybindingWeight.EditorContrib, + when: ContextKeyExpr.and(ctxHasEditorModification, EditorContextKeys.focus), + }, + f1: true, + menu: { + id: MenuId.EditorTitle, + group: 'navigation', + order: next ? -100 : -101, + when: ctxHasEditorModification + } + }); + } + + override run(accessor: ServicesAccessor) { + + const editor = accessor.get(IEditorService).activeTextEditorControl; + + if (!isCodeEditor(editor)) { + return; + } + + if (this.next) { + ChatEditorController.get(editor)?.revealNext(); + } else { + ChatEditorController.get(editor)?.revealPrevious(); + } + } + +} + + +export function registerChatEditorActions() { + registerAction2(class extends NavigateAction { constructor() { super(true); } }); + registerAction2(class extends NavigateAction { constructor() { super(false); } }); +} diff --git a/src/vs/workbench/contrib/chat/browser/chatEditorController.ts b/src/vs/workbench/contrib/chat/browser/chatEditorController.ts index 4fd83d6e205e01..89326dea525211 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditorController.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditorController.ts @@ -3,35 +3,56 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { binarySearch, coalesceInPlace } from '../../../../base/common/arrays.js'; import { Disposable, DisposableStore, toDisposable } from '../../../../base/common/lifecycle.js'; import { Constants } from '../../../../base/common/uint.js'; import { ICodeEditor, IViewZone } from '../../../../editor/browser/editorBrowser.js'; import { LineSource, renderLines, RenderOptions } from '../../../../editor/browser/widget/diffEditor/components/diffEditorViewZones/renderLines.js'; import { diffAddDecoration, diffDeleteDecoration, diffWholeLineAddDecoration } from '../../../../editor/browser/widget/diffEditor/registrations.contribution.js'; import { EditorOption } from '../../../../editor/common/config/editorOptions.js'; +import { Range } from '../../../../editor/common/core/range.js'; import { IDocumentDiff } from '../../../../editor/common/diff/documentDiffProvider.js'; -import { IEditorContribution } from '../../../../editor/common/editorCommon.js'; +import { IEditorContribution, ScrollType } from '../../../../editor/common/editorCommon.js'; import { IModelDeltaDecoration, ITextModel } from '../../../../editor/common/model.js'; import { IEditorWorkerService } from '../../../../editor/common/services/editorWorker.js'; import { InlineDecoration, InlineDecorationType } from '../../../../editor/common/viewModel.js'; import { IChatEditingService, IChatEditingSession, IModifiedFileEntry, WorkingSetEntryState } from '../common/chatEditingService.js'; +import { localize } from '../../../../nls.js'; +import { IContextKey, IContextKeyService, RawContextKey } from '../../../../platform/contextkey/common/contextkey.js'; + +export const ctxHasEditorModification = new RawContextKey('chat.hasEditorModifications', undefined, localize('chat.hasEditorModifications', "The current editor contains chat modifications")); export class ChatEditorController extends Disposable implements IEditorContribution { public static readonly ID = 'editor.contrib.chatEditorController'; + private readonly _sessionStore = this._register(new DisposableStore()); private readonly _decorations = this._editor.createDecorationsCollection(); private _viewZones: string[] = []; + private readonly _ctxHasEditorModification: IContextKey; + + static get(editor: ICodeEditor): ChatEditorController | null { + const controller = editor.getContribution(ChatEditorController.ID); + return controller; + } constructor( private readonly _editor: ICodeEditor, @IChatEditingService private readonly _chatEditingService: IChatEditingService, - @IEditorWorkerService private readonly _editorWorkerService: IEditorWorkerService + @IEditorWorkerService private readonly _editorWorkerService: IEditorWorkerService, + @IContextKeyService contextKeyService: IContextKeyService, ) { super(); this._register(this._editor.onDidChangeModel(() => this._update())); this._register(this._chatEditingService.onDidChangeEditingSession(() => this._updateSessionDecorations())); this._register(toDisposable(() => this._clearRendering())); + + this._ctxHasEditorModification = ctxHasEditorModification.bindTo(contextKeyService); + } + + override dispose(): void { + this._clearRendering(); + super.dispose(); } private _update(): void { @@ -88,6 +109,7 @@ export class ChatEditorController extends Disposable implements IEditorContribut return; } + this._ctxHasEditorModification.set(true); this._updateWithDiff(model, entry, diff); }); } @@ -107,6 +129,7 @@ export class ChatEditorController extends Disposable implements IEditorContribut }); this._viewZones = []; this._decorations.clear(); + this._ctxHasEditorModification.reset(); } private _updateWithDiff(model: ITextModel, entry: IModifiedFileEntry, diff: IDocumentDiff | null): void { @@ -166,4 +189,62 @@ export class ChatEditorController extends Disposable implements IEditorContribut this._decorations.set(modifiedDecorations); }); } + + revealNext() { + this._reveal(true); + } + + revealPrevious() { + this._reveal(false); + } + + private _reveal(next: boolean) { + const position = this._editor.getPosition(); + if (!position) { + return; + } + + const decorations: (Range | undefined)[] = this._decorations + .getRanges() + .sort((a, b) => Range.compareRangesUsingStarts(a, b)); + + // TODO@jrieken this is slow and should be done smarter, e.g being able to read + // only whole range decorations because the goal is to go from change to change, skipping + // over word level changes + for (let i = 0; i < decorations.length; i++) { + const decoration = decorations[i]; + for (let j = 0; j < decorations.length; j++) { + if (i !== j && decoration && decorations[j]?.containsRange(decoration)) { + decorations[i] = undefined; + break; + } + } + } + + coalesceInPlace(decorations); + + if (decorations.length === 0) { + return; + } + + let idx = binarySearch(decorations, Range.fromPositions(position), Range.compareRangesUsingStarts); + if (idx < 0) { + idx = ~idx; + } + + let target: number; + if (decorations[idx]?.containsPosition(position)) { + target = idx + (next ? 1 : -1); + } else { + target = next ? idx : idx - 1; + } + + target = (target + decorations.length) % decorations.length; + + + const targetPosition = decorations[target].getStartPosition(); + this._editor.setPosition(targetPosition); + this._editor.revealPosition(targetPosition, ScrollType.Smooth); + this._editor.focus(); + } } From 482002d74163957cfea8a0e480c131ece1a75647 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 8 Oct 2024 15:55:50 +0200 Subject: [PATCH 02/18] add title as the description (#230801) --- .../workbench/services/keybinding/browser/keybindingService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/services/keybinding/browser/keybindingService.ts b/src/vs/workbench/services/keybinding/browser/keybindingService.ts index 671f9552963ca2..bb41a6d5705989 100644 --- a/src/vs/workbench/services/keybinding/browser/keybindingService.ts +++ b/src/vs/workbench/services/keybinding/browser/keybindingService.ts @@ -955,7 +955,7 @@ class KeybindingsJsonSchema { for (const [commandId, command] of allCommands) { const commandMetadata = command.metadata; - addKnownCommand(commandId, commandMetadata?.description); + addKnownCommand(commandId, commandMetadata?.description ?? MenuRegistry.getCommand(commandId)?.title); if (!commandMetadata || !commandMetadata.args || commandMetadata.args.length !== 1 || !commandMetadata.args[0].schema) { continue; From b28756f697470e049f4b11bb414a5dcb501f03ec Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Tue, 8 Oct 2024 15:59:54 +0200 Subject: [PATCH 03/18] when discarding changes, push an edit and undo stop (#230802) --- .../workbench/contrib/chat/browser/chatEditingService.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatEditingService.ts b/src/vs/workbench/contrib/chat/browser/chatEditingService.ts index eee5300f8729ec..05dae5ce02127c 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditingService.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditingService.ts @@ -13,6 +13,7 @@ import { derived, IObservable, ITransaction, observableValue, ValueWithChangeEve import { URI } from '../../../../base/common/uri.js'; import { isCodeEditor, isDiffEditor } from '../../../../editor/browser/editorBrowser.js'; import { IBulkEditService } from '../../../../editor/browser/services/bulkEditService.js'; +import { EditOperation } from '../../../../editor/common/core/editOperation.js'; import { TextEdit } from '../../../../editor/common/languages.js'; import { ILanguageService } from '../../../../editor/common/languages/language.js'; import { ITextModel } from '../../../../editor/common/model.js'; @@ -721,6 +722,7 @@ class ModifiedFileEntry extends Disposable implements IModifiedFileEntry { // already accepted or rejected return; } + this.docSnapshot.setValue(this.doc.createSnapshot()); this._stateObs.set(WorkingSetEntryState.Accepted, transaction); await this.collapse(transaction); @@ -732,7 +734,12 @@ class ModifiedFileEntry extends Disposable implements IModifiedFileEntry { // already accepted or rejected return; } - this.doc.setValue(this.docSnapshot.createSnapshot()); + + this.doc.pushStackElement(); + const edit = EditOperation.replace(this.doc.getFullModelRange(), this.docSnapshot.getValue()); + this.doc.pushEditOperations(null, [edit], () => null); + this.doc.pushStackElement(); + this._stateObs.set(WorkingSetEntryState.Rejected, transaction); await this.collapse(transaction); this._notifyAction('rejected'); From 4c2964969d08a5105671ac66f62b7e827a7ef287 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Tue, 8 Oct 2024 16:00:52 +0200 Subject: [PATCH 04/18] update milestones (#230803) --- .vscode/notebooks/api.github-issues | 2 +- .vscode/notebooks/my-work.github-issues | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.vscode/notebooks/api.github-issues b/.vscode/notebooks/api.github-issues index 6b6a41f5135484..35ea9042ef5c93 100644 --- a/.vscode/notebooks/api.github-issues +++ b/.vscode/notebooks/api.github-issues @@ -7,7 +7,7 @@ { "kind": 2, "language": "github-issues", - "value": "$REPO=repo:microsoft/vscode\n$MILESTONE=milestone:\"September 2024\"" + "value": "$REPO=repo:microsoft/vscode\n$MILESTONE=milestone:\"October 2024\"" }, { "kind": 1, diff --git a/.vscode/notebooks/my-work.github-issues b/.vscode/notebooks/my-work.github-issues index 5f54a2e5d91f8c..c58ceae0d01057 100644 --- a/.vscode/notebooks/my-work.github-issues +++ b/.vscode/notebooks/my-work.github-issues @@ -7,7 +7,7 @@ { "kind": 2, "language": "github-issues", - "value": "// list of repos we work in\n$REPOS=repo:microsoft/lsprotocol repo:microsoft/monaco-editor repo:microsoft/vscode repo:microsoft/vscode-anycode repo:microsoft/vscode-autopep8 repo:microsoft/vscode-black-formatter repo:microsoft/vscode-copilot repo:microsoft/vscode-copilot-release repo:microsoft/vscode-dev repo:microsoft/vscode-dev-chrome-launcher repo:microsoft/vscode-emmet-helper repo:microsoft/vscode-extension-telemetry repo:microsoft/vscode-flake8 repo:microsoft/vscode-github-issue-notebooks repo:microsoft/vscode-hexeditor repo:microsoft/vscode-internalbacklog repo:microsoft/vscode-isort repo:microsoft/vscode-js-debug repo:microsoft/vscode-jupyter repo:microsoft/vscode-jupyter-internal repo:microsoft/vscode-l10n repo:microsoft/vscode-livepreview repo:microsoft/vscode-markdown-languageservice repo:microsoft/vscode-markdown-tm-grammar repo:microsoft/vscode-mypy repo:microsoft/vscode-pull-request-github repo:microsoft/vscode-pylint repo:microsoft/vscode-python repo:microsoft/vscode-python-debugger repo:microsoft/vscode-python-tools-extension-template repo:microsoft/vscode-references-view repo:microsoft/vscode-remote-release repo:microsoft/vscode-remote-repositories-github repo:microsoft/vscode-remote-tunnels repo:microsoft/vscode-remotehub repo:microsoft/vscode-settings-sync-server repo:microsoft/vscode-unpkg repo:microsoft/vscode-vsce\n\n// current milestone name\n$MILESTONE=milestone:\"September 2024\"\n" + "value": "// list of repos we work in\n$REPOS=repo:microsoft/lsprotocol repo:microsoft/monaco-editor repo:microsoft/vscode repo:microsoft/vscode-anycode repo:microsoft/vscode-autopep8 repo:microsoft/vscode-black-formatter repo:microsoft/vscode-copilot repo:microsoft/vscode-copilot-release repo:microsoft/vscode-dev repo:microsoft/vscode-dev-chrome-launcher repo:microsoft/vscode-emmet-helper repo:microsoft/vscode-extension-telemetry repo:microsoft/vscode-flake8 repo:microsoft/vscode-github-issue-notebooks repo:microsoft/vscode-hexeditor repo:microsoft/vscode-internalbacklog repo:microsoft/vscode-isort repo:microsoft/vscode-js-debug repo:microsoft/vscode-jupyter repo:microsoft/vscode-jupyter-internal repo:microsoft/vscode-l10n repo:microsoft/vscode-livepreview repo:microsoft/vscode-markdown-languageservice repo:microsoft/vscode-markdown-tm-grammar repo:microsoft/vscode-mypy repo:microsoft/vscode-pull-request-github repo:microsoft/vscode-pylint repo:microsoft/vscode-python repo:microsoft/vscode-python-debugger repo:microsoft/vscode-python-tools-extension-template repo:microsoft/vscode-references-view repo:microsoft/vscode-remote-release repo:microsoft/vscode-remote-repositories-github repo:microsoft/vscode-remote-tunnels repo:microsoft/vscode-remotehub repo:microsoft/vscode-settings-sync-server repo:microsoft/vscode-unpkg repo:microsoft/vscode-vsce\n\n// current milestone name\n$MILESTONE=milestone:\"October 2024\"\n" }, { "kind": 1, From 63c2466a77a42f71c3c8b42e4f502edf28a040a1 Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Tue, 8 Oct 2024 09:08:26 -0700 Subject: [PATCH 05/18] feat: enable model picker for generating chat edits (#230815) --- .../contrib/chat/browser/actions/chatExecuteActions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatExecuteActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatExecuteActions.ts index 4d2fac6e025dbc..453d2c1d44290f 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatExecuteActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatExecuteActions.ts @@ -76,7 +76,7 @@ MenuRegistry.appendMenuItem(MenuId.ChatExecute, { }, order: 3, group: 'navigation', - when: ContextKeyExpr.and(CONTEXT_LANGUAGE_MODELS_ARE_USER_SELECTABLE, CONTEXT_PARTICIPANT_SUPPORTS_MODEL_PICKER, ContextKeyExpr.equals(CONTEXT_CHAT_LOCATION.key, 'panel')), + when: ContextKeyExpr.and(CONTEXT_LANGUAGE_MODELS_ARE_USER_SELECTABLE, CONTEXT_PARTICIPANT_SUPPORTS_MODEL_PICKER, ContextKeyExpr.or(ContextKeyExpr.equals(CONTEXT_CHAT_LOCATION.key, 'panel'), ContextKeyExpr.equals(CONTEXT_CHAT_LOCATION.key, ChatAgentLocation.EditingSession))), }); export class ChatSubmitSecondaryAgentAction extends Action2 { From 0b0a580fbb8cc1433bbb20f7ffbeca2b75b315cd Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Tue, 8 Oct 2024 09:58:27 -0700 Subject: [PATCH 06/18] feat: finalize chat participant detection api (#230819) --- .../common/extensionsApiProposals.ts | 3 --- .../browser/chatParticipantContributions.ts | 24 +++++++++---------- ...posed.contribChatParticipantDetection.d.ts | 7 ------ 3 files changed, 11 insertions(+), 23 deletions(-) delete mode 100644 src/vscode-dts/vscode.proposed.contribChatParticipantDetection.d.ts diff --git a/src/vs/platform/extensions/common/extensionsApiProposals.ts b/src/vs/platform/extensions/common/extensionsApiProposals.ts index 0a045ca4d28e94..ac432211095faf 100644 --- a/src/vs/platform/extensions/common/extensionsApiProposals.ts +++ b/src/vs/platform/extensions/common/extensionsApiProposals.ts @@ -76,9 +76,6 @@ const _allApiProposals = { contribAccessibilityHelpContent: { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.contribAccessibilityHelpContent.d.ts', }, - contribChatParticipantDetection: { - proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.contribChatParticipantDetection.d.ts', - }, contribCommentEditorActionsMenu: { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.contribCommentEditorActionsMenu.d.ts', }, diff --git a/src/vs/workbench/contrib/chat/browser/chatParticipantContributions.ts b/src/vs/workbench/contrib/chat/browser/chatParticipantContributions.ts index 8f9a2e9195b970..e5e0949dab318d 100644 --- a/src/vs/workbench/contrib/chat/browser/chatParticipantContributions.ts +++ b/src/vs/workbench/contrib/chat/browser/chatParticipantContributions.ts @@ -220,19 +220,17 @@ export class ChatExtensionPointHandler implements IWorkbenchContribution { examples: string[]; }[] = []; - if (isProposedApiEnabled(extension.description, 'contribChatParticipantDetection')) { - if (providerDescriptor.disambiguation?.length) { - participantsAndCommandsDisambiguation.push(...providerDescriptor.disambiguation.map((d) => ({ - ...d, category: d.category ?? d.categoryName - }))); - } - if (providerDescriptor.commands) { - for (const command of providerDescriptor.commands) { - if (command.disambiguation?.length) { - participantsAndCommandsDisambiguation.push(...command.disambiguation.map((d) => ({ - ...d, category: d.category ?? d.categoryName - }))); - } + if (providerDescriptor.disambiguation?.length) { + participantsAndCommandsDisambiguation.push(...providerDescriptor.disambiguation.map((d) => ({ + ...d, category: d.category ?? d.categoryName + }))); + } + if (providerDescriptor.commands) { + for (const command of providerDescriptor.commands) { + if (command.disambiguation?.length) { + participantsAndCommandsDisambiguation.push(...command.disambiguation.map((d) => ({ + ...d, category: d.category ?? d.categoryName + }))); } } } diff --git a/src/vscode-dts/vscode.proposed.contribChatParticipantDetection.d.ts b/src/vscode-dts/vscode.proposed.contribChatParticipantDetection.d.ts deleted file mode 100644 index 6de04ce23900be..00000000000000 --- a/src/vscode-dts/vscode.proposed.contribChatParticipantDetection.d.ts +++ /dev/null @@ -1,7 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -// empty placeholder declaration for the `chatParticipantDetection`-contribution point -// https://github.com/microsoft/vscode/issues/227699 From 181e4a299e5a0834df44ebeaf6d19a7c6e3f5e33 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Tue, 8 Oct 2024 10:40:42 -0700 Subject: [PATCH 07/18] Fix #230817. (#230820) --- .../workbench/contrib/notebook/browser/media/notebook.css | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/vs/workbench/contrib/notebook/browser/media/notebook.css b/src/vs/workbench/contrib/notebook/browser/media/notebook.css index 250ceaf80ef0c0..345210b836144f 100644 --- a/src/vs/workbench/contrib/notebook/browser/media/notebook.css +++ b/src/vs/workbench/contrib/notebook/browser/media/notebook.css @@ -646,3 +646,9 @@ .monaco-workbench .notebookOverlay .monaco-list .monaco-list-row.markdown-cell-row.nb-chatGenerationHighlight { background-color: var(--vscode-notebook-selectedCellBackground) !important; } + +/** Avoid Zone Widget hide cell editor focus indicator partially **/ +.monaco-workbench .notebookOverlay .cell-list-container .monaco-editor .zone-widget { + margin-left: 1px; + margin-right: 1px; +} From a43fbf4efe7c84e341abb69594223093e76e03f0 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Tue, 8 Oct 2024 19:55:26 +0200 Subject: [PATCH 08/18] files - store file paths on disk for save as elevated user flow (#230822) --- src/vs/code/node/cli.ts | 18 +++++- .../electron-main/nativeHostMainService.ts | 56 +++++++++++-------- .../electron-sandbox/elevatedFileService.ts | 16 +++++- 3 files changed, 62 insertions(+), 28 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 6ca979ca03fbd1..bb5f41975b2853 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -138,14 +138,26 @@ export async function main(argv: string[]): Promise { // Write File else if (args['file-write']) { - const source = args._[0]; - const target = args._[1]; + const argsFile = args._[0]; + if (!argsFile || !isAbsolute(argsFile) || !existsSync(argsFile) || !statSync(argsFile).isFile()) { + throw new Error('Using --file-write with invalid arguments.'); + } + + let source: string | undefined; + let target: string | undefined; + try { + const argsContents = JSON.parse(readFileSync(argsFile, 'utf8')); + source = argsContents.source; + target = argsContents.target; + } catch (error) { + throw new Error('Using --file-write with invalid arguments.'); + } // Windows: set the paths as allowed UNC paths given // they are explicitly provided by the user as arguments if (isWindows) { for (const path of [source, target]) { - if (isUNC(path)) { + if (typeof path === 'string' && isUNC(path)) { addUNCHostToAllowlist(URI.file(path).authority); } } diff --git a/src/vs/platform/native/electron-main/nativeHostMainService.ts b/src/vs/platform/native/electron-main/nativeHostMainService.ts index 26b5175291c315..dcee07881f866c 100644 --- a/src/vs/platform/native/electron-main/nativeHostMainService.ts +++ b/src/vs/platform/native/electron-main/nativeHostMainService.ts @@ -47,6 +47,7 @@ import { CancellationError } from '../../../base/common/errors.js'; import { IConfigurationService } from '../../configuration/common/configuration.js'; import { IProxyAuthService } from './auth.js'; import { AuthInfo, Credentials, IRequestService } from '../../request/common/request.js'; +import { randomPath } from '../../../base/common/extpath.js'; export interface INativeHostMainService extends AddFirstParameterToFunctions /* only methods, not events */, number | undefined /* window ID */> { } @@ -568,35 +569,44 @@ export class NativeHostMainService extends Disposable implements INativeHostMain async writeElevated(windowId: number | undefined, source: URI, target: URI, options?: { unlock?: boolean }): Promise { const sudoPrompt = await import('@vscode/sudo-prompt'); - return new Promise((resolve, reject) => { - const sudoCommand: string[] = [`"${this.cliPath}"`]; - if (options?.unlock) { - sudoCommand.push('--file-chmod'); - } + const argsFile = randomPath(this.environmentMainService.userDataPath, 'code-elevated'); + await Promises.writeFile(argsFile, JSON.stringify({ source: source.fsPath, target: target.fsPath })); - sudoCommand.push('--file-write', `"${source.fsPath}"`, `"${target.fsPath}"`); + try { + await new Promise((resolve, reject) => { + const sudoCommand: string[] = [`"${this.cliPath}"`]; + if (options?.unlock) { + sudoCommand.push('--file-chmod'); + } - const promptOptions = { - name: this.productService.nameLong.replace('-', ''), - icns: (isMacintosh && this.environmentMainService.isBuilt) ? join(dirname(this.environmentMainService.appRoot), `${this.productService.nameShort}.icns`) : undefined - }; + sudoCommand.push('--file-write', `"${argsFile}"`); - sudoPrompt.exec(sudoCommand.join(' '), promptOptions, (error?, stdout?, stderr?) => { - if (stdout) { - this.logService.trace(`[sudo-prompt] received stdout: ${stdout}`); - } + const promptOptions = { + name: this.productService.nameLong.replace('-', ''), + icns: (isMacintosh && this.environmentMainService.isBuilt) ? join(dirname(this.environmentMainService.appRoot), `${this.productService.nameShort}.icns`) : undefined + }; - if (stderr) { - this.logService.trace(`[sudo-prompt] received stderr: ${stderr}`); - } + this.logService.trace(`[sudo-prompt] running command: ${sudoCommand.join(' ')}`); - if (error) { - reject(error); - } else { - resolve(undefined); - } + sudoPrompt.exec(sudoCommand.join(' '), promptOptions, (error?, stdout?, stderr?) => { + if (stdout) { + this.logService.trace(`[sudo-prompt] received stdout: ${stdout}`); + } + + if (stderr) { + this.logService.error(`[sudo-prompt] received stderr: ${stderr}`); + } + + if (error) { + reject(error); + } else { + resolve(undefined); + } + }); }); - }); + } finally { + await fs.promises.unlink(argsFile); + } } async isRunningUnderARM64Translation(): Promise { diff --git a/src/vs/workbench/services/files/electron-sandbox/elevatedFileService.ts b/src/vs/workbench/services/files/electron-sandbox/elevatedFileService.ts index 09f67aba17c9f5..158a5ac0f4c6a8 100644 --- a/src/vs/workbench/services/files/electron-sandbox/elevatedFileService.ts +++ b/src/vs/workbench/services/files/electron-sandbox/elevatedFileService.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { localize } from '../../../../nls.js'; import { VSBuffer, VSBufferReadable, VSBufferReadableStream } from '../../../../base/common/buffer.js'; import { randomPath } from '../../../../base/common/extpath.js'; import { Schemas } from '../../../../base/common/network.js'; @@ -10,9 +11,11 @@ import { URI } from '../../../../base/common/uri.js'; import { IFileService, IFileStatWithMetadata, IWriteFileOptions } from '../../../../platform/files/common/files.js'; import { InstantiationType, registerSingleton } from '../../../../platform/instantiation/common/extensions.js'; import { INativeHostService } from '../../../../platform/native/common/native.js'; +import { IWorkspaceTrustRequestService } from '../../../../platform/workspace/common/workspaceTrust.js'; import { INativeWorkbenchEnvironmentService } from '../../environment/electron-sandbox/environmentService.js'; import { IElevatedFileService } from '../common/elevatedFileService.js'; - +import { isWindows } from '../../../../base/common/platform.js'; +import { ILabelService } from '../../../../platform/label/common/label.js'; export class NativeElevatedFileService implements IElevatedFileService { readonly _serviceBrand: undefined; @@ -20,7 +23,9 @@ export class NativeElevatedFileService implements IElevatedFileService { constructor( @INativeHostService private readonly nativeHostService: INativeHostService, @IFileService private readonly fileService: IFileService, - @INativeWorkbenchEnvironmentService private readonly environmentService: INativeWorkbenchEnvironmentService + @INativeWorkbenchEnvironmentService private readonly environmentService: INativeWorkbenchEnvironmentService, + @IWorkspaceTrustRequestService private readonly workspaceTrustRequestService: IWorkspaceTrustRequestService, + @ILabelService private readonly labelService: ILabelService ) { } isSupported(resource: URI): boolean { @@ -32,6 +37,13 @@ export class NativeElevatedFileService implements IElevatedFileService { } async writeFileElevated(resource: URI, value: VSBuffer | VSBufferReadable | VSBufferReadableStream, options?: IWriteFileOptions): Promise { + const trusted = await this.workspaceTrustRequestService.requestWorkspaceTrust({ + message: isWindows ? localize('fileNotTrustedMessageWindows', "You are about to save '{0}' as admin.", this.labelService.getUriLabel(resource)) : localize('fileNotTrustedMessagePosix', "You are about to save '{0}' as super user.", this.labelService.getUriLabel(resource)), + }); + if (!trusted) { + throw new Error(localize('fileNotTrusted', "Workspace is not trusted.")); + } + const source = URI.file(randomPath(this.environmentService.userDataPath, 'code-elevated')); try { // write into a tmp file first From 9df2e3bee3475cd4cc0b430c39679bc8b948aa78 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Tue, 8 Oct 2024 11:20:42 -0700 Subject: [PATCH 09/18] allow passing in images to open chat action (#230744) --- .../chat/browser/actions/chatActions.ts | 22 +++++++++++++++++++ .../contrib/chat/common/chatModel.ts | 3 +++ 2 files changed, 25 insertions(+) diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatActions.ts index 01abc19098ac83..e272d68126fa79 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatActions.ts @@ -10,6 +10,7 @@ import { fromNowByDay } from '../../../../../base/common/date.js'; import { KeyCode, KeyMod } from '../../../../../base/common/keyCodes.js'; import { DisposableStore } from '../../../../../base/common/lifecycle.js'; import { ThemeIcon } from '../../../../../base/common/themables.js'; +import { URI } from '../../../../../base/common/uri.js'; import { ICodeEditor } from '../../../../../editor/browser/editorBrowser.js'; import { EditorAction2, ServicesAccessor } from '../../../../../editor/browser/editorExtensions.js'; import { Position } from '../../../../../editor/common/core/position.js'; @@ -55,6 +56,17 @@ export interface IChatViewOpenOptions { * Any previous chat requests and responses that should be shown in the chat view. */ previousRequests?: IChatViewOpenRequestEntry[]; + + /** + * The image(s) to include in the request + */ + images?: IChatImageAttachment[]; +} + +export interface IChatImageAttachment { + id: string; + name: string; + value: URI | Uint8Array; } export interface IChatViewOpenRequestEntry { @@ -101,6 +113,16 @@ class OpenChatGlobalAction extends Action2 { chatService.addCompleteRequest(chatWidget.viewModel.sessionId, request, undefined, 0, { message: response }); } } + if (opts?.images) { + chatWidget.attachmentModel.clear(); + for (const image of opts.images) { + chatWidget.attachmentModel.addContext({ + ...image, + isDynamic: true, + isImage: true + }); + } + } if (opts?.query) { if (opts.isPartialQuery) { chatWidget.setInput(opts.query); diff --git a/src/vs/workbench/contrib/chat/common/chatModel.ts b/src/vs/workbench/contrib/chat/common/chatModel.ts index 6a0a16c42af1ca..47e82b72cc9656 100644 --- a/src/vs/workbench/contrib/chat/common/chatModel.ts +++ b/src/vs/workbench/contrib/chat/common/chatModel.ts @@ -36,6 +36,9 @@ export interface IChatRequestVariableEntry { mimeType?: string; // TODO are these just a 'kind'? + /** + * True if the variable has a value vs being a reference to a variable + */ isDynamic?: boolean; isFile?: boolean; isTool?: boolean; From 61ad7eae42a33ddb81857b9d5f0f3f4b8a7f7c31 Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Tue, 8 Oct 2024 12:21:41 -0700 Subject: [PATCH 10/18] fix: remove duplicate mic icon from chat editing execute menu (#230813) --- .../contrib/chat/electron-sandbox/actions/voiceChatActions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/electron-sandbox/actions/voiceChatActions.ts b/src/vs/workbench/contrib/chat/electron-sandbox/actions/voiceChatActions.ts index 08575f13eb173e..e89738779e8fe7 100644 --- a/src/vs/workbench/contrib/chat/electron-sandbox/actions/voiceChatActions.ts +++ b/src/vs/workbench/contrib/chat/electron-sandbox/actions/voiceChatActions.ts @@ -607,7 +607,7 @@ export class StartVoiceChatAction extends Action2 { }, { id: MenuId.ChatExecute, - when: ContextKeyExpr.and(CONTEXT_CHAT_LOCATION.isEqualTo(ChatAgentLocation.Panel).negate(), menuCondition), + when: ContextKeyExpr.and(CONTEXT_CHAT_LOCATION.isEqualTo(ChatAgentLocation.Panel).negate(), CONTEXT_CHAT_LOCATION.isEqualTo(ChatAgentLocation.EditingSession).negate(), menuCondition), group: 'navigation', order: 2 }, From aad8af12b079f732a297135b3a3b7fa20512499a Mon Sep 17 00:00:00 2001 From: Joyce Er Date: Tue, 8 Oct 2024 14:28:47 -0700 Subject: [PATCH 11/18] fix: add missing ? in chat editing save dialog message (#230837) --- src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts b/src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts index 1bdfd1d19c93cb..a1991fc9fea263 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts @@ -88,8 +88,8 @@ export class ChatEditorSaving extends Disposable implements IWorkbenchContributi const filelabel = labelService.getUriBasenameLabel(workingCopy.resource); const message = agentName - ? localize('message.1', "Do you want to accept the changes {0} made in {1}", agentName, filelabel) - : localize('message.2', "Do you want to accept the changes chat made in {1}", filelabel); + ? localize('message.1', "Do you want to accept the changes {0} made in {1}?", agentName, filelabel) + : localize('message.2', "Do you want to accept the changes chat made in {1}?", filelabel); const result = await this._dialogService.confirm({ message, From ef869b69586adcf11905aa37393d18aa1b706869 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 8 Oct 2024 16:49:51 -0700 Subject: [PATCH 12/18] Fix issues around tool check not rendering (#230841) * Fix rendering loop caused by using wrong word count * Simpler calculation for moreContentAvailable, and fix another issue with the tool check not rendering --- .../contrib/chat/browser/actions/codeBlockOperations.ts | 2 +- .../browser/chatContentParts/chatToolInvocationPart.ts | 5 ++++- .../workbench/contrib/chat/browser/chatListRenderer.ts | 9 ++++++++- .../contrib/chat/common/chatCodeMapperService.ts | 2 +- src/vs/workbench/contrib/chat/common/chatModel.ts | 9 ++++++--- src/vs/workbench/contrib/chat/common/chatViewModel.ts | 2 +- .../chat/browser/terminalChatController.ts | 2 +- 7 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/actions/codeBlockOperations.ts b/src/vs/workbench/contrib/chat/browser/actions/codeBlockOperations.ts index 161860a4efc641..a434980b9f4a54 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/codeBlockOperations.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/codeBlockOperations.ts @@ -390,7 +390,7 @@ function getChatConversation(context: ICodeBlockActionContext): (ConversationReq if (isResponseVM(context.element)) { return [{ type: 'response', - message: context.element.response.toMarkdown(), + message: context.element.response.getMarkdown(), references: getReferencesAsDocumentContext(context.element.contentReferences) }]; } else if (isRequestVM(context.element)) { diff --git a/src/vs/workbench/contrib/chat/browser/chatContentParts/chatToolInvocationPart.ts b/src/vs/workbench/contrib/chat/browser/chatContentParts/chatToolInvocationPart.ts index 2c460dd9a1cbfc..9a91746ce74130 100644 --- a/src/vs/workbench/contrib/chat/browser/chatContentParts/chatToolInvocationPart.ts +++ b/src/vs/workbench/contrib/chat/browser/chatContentParts/chatToolInvocationPart.ts @@ -86,7 +86,6 @@ class ChatToolInvocationSubPart extends Disposable { toolInvocation.confirmed.complete(button.data); })); toolInvocation.confirmed.p.then(() => this._onNeedsRerender.fire()); - toolInvocation.isCompleteDeferred.p.then(() => this._onNeedsRerender.fire()); } else { const message = toolInvocation.invocationMessage + '…'; const progressMessage: IChatProgressMessage = { @@ -100,5 +99,9 @@ class ChatToolInvocationSubPart extends Disposable { const progressPart = this._register(instantiationService.createInstance(ChatProgressContentPart, progressMessage, renderer, context, undefined, true, iconOverride)); this.domNode = progressPart.domNode; } + + if (toolInvocation.kind === 'toolInvocation' && !toolInvocation.isComplete) { + toolInvocation.isCompleteDeferred.p.then(() => this._onNeedsRerender.fire()); + } } } diff --git a/src/vs/workbench/contrib/chat/browser/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/chatListRenderer.ts index 247490792e4b97..b057c4ed0ae8c9 100644 --- a/src/vs/workbench/contrib/chat/browser/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/chatListRenderer.ts @@ -683,6 +683,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer part.kind === 'markdownContent')) { + moreContentAvailable = true; + } break; } } else { @@ -724,7 +731,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer 0 }; + return { content: partsToRender, moreContentAvailable }; } private getDataForProgressiveRender(element: IChatResponseViewModel) { diff --git a/src/vs/workbench/contrib/chat/common/chatCodeMapperService.ts b/src/vs/workbench/contrib/chat/common/chatCodeMapperService.ts index bf1ae9d8c194a3..097ca6d89157ca 100644 --- a/src/vs/workbench/contrib/chat/common/chatCodeMapperService.ts +++ b/src/vs/workbench/contrib/chat/common/chatCodeMapperService.ts @@ -140,7 +140,7 @@ export class CodeMapperService implements ICodeMapperService { }); conversation.push({ type: 'response', - message: response.response.toMarkdown(), + message: response.response.getMarkdown(), result: response.result, references: getReferencesAsDocumentContext(response.contentReferences) }); diff --git a/src/vs/workbench/contrib/chat/common/chatModel.ts b/src/vs/workbench/contrib/chat/common/chatModel.ts index 47e82b72cc9656..c7eaef0b5f1afe 100644 --- a/src/vs/workbench/contrib/chat/common/chatModel.ts +++ b/src/vs/workbench/contrib/chat/common/chatModel.ts @@ -121,7 +121,7 @@ export type IChatProgressRenderableResponseContent = Exclude; - toMarkdown(): string; + getMarkdown(): string; toString(): string; } @@ -250,7 +250,10 @@ export class Response extends Disposable implements IResponse { return this._responseRepr; } - toMarkdown(): string { + /** + * _Just_ the content of markdown parts in the response + */ + getMarkdown(): string { return this._markdownContent; } @@ -362,7 +365,7 @@ export class Response extends Disposable implements IResponse { } }) .filter(s => s.length > 0) - .join('\n\n'); + .join(''); if (!quiet) { this._onDidChangeValue.fire(); diff --git a/src/vs/workbench/contrib/chat/common/chatViewModel.ts b/src/vs/workbench/contrib/chat/common/chatViewModel.ts index 4faeae41c0bdd4..02cf44b3708a1b 100644 --- a/src/vs/workbench/contrib/chat/common/chatViewModel.ts +++ b/src/vs/workbench/contrib/chat/common/chatViewModel.ts @@ -523,7 +523,7 @@ export class ChatResponseViewModel extends Disposable implements IChatResponseVi // This should be true, if the model is changing if (this._contentUpdateTimings) { const now = Date.now(); - const wordCount = countWords(_model.response.toString()); + const wordCount = countWords(_model.response.getMarkdown()); // Apply a min time difference, or the rate is typically too high for first few words const timeDiff = Math.max(now - this._contentUpdateTimings.firstWordTime, 250); diff --git a/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatController.ts b/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatController.ts index 4af266b4bb6252..1c8a5228db606a 100644 --- a/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatController.ts +++ b/src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatController.ts @@ -251,7 +251,7 @@ export class TerminalChatController extends Disposable implements ITerminalContr })); } await responsePromise.p; - this._lastResponseContent = response?.response.toMarkdown(); + this._lastResponseContent = response?.response.getMarkdown(); return response; } catch { this._lastResponseContent = undefined; From 7254e846cdef127bdba590819791e4f4c0df3bc8 Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Wed, 9 Oct 2024 02:58:17 +0200 Subject: [PATCH 13/18] Don't wait for non-winning inline completion providers when requesting in automatic mode. (#230789) * Don't wait for non-winning inline completion providers when requesting in automatic mode. * Fixes leak * Fixes another leak --- src/vs/base/common/async.ts | 26 ++++ .../controller/inlineCompletionsController.ts | 4 + .../browser/model/inlineCompletionsSource.ts | 1 + .../browser/model/inlineEditsAdapter.ts | 3 + .../browser/model/provideInlineCompletions.ts | 132 ++++++++++++------ 5 files changed, 125 insertions(+), 41 deletions(-) diff --git a/src/vs/base/common/async.ts b/src/vs/base/common/async.ts index a270e41a213f78..98a68886677ebe 100644 --- a/src/vs/base/common/async.ts +++ b/src/vs/base/common/async.ts @@ -126,6 +126,32 @@ export function raceTimeout(promise: Promise, timeout: number, onTimeout?: ]); } +export function raceFilter(promises: Promise[], filter: (result: T) => boolean): Promise { + return new Promise((resolve, reject) => { + if (promises.length === 0) { + resolve(undefined); + return; + } + + let resolved = false; + let unresolvedCount = promises.length; + for (const promise of promises) { + promise.then(result => { + unresolvedCount--; + if (!resolved) { + if (filter(result)) { + resolved = true; + resolve(result); + } else if (unresolvedCount === 0) { + // Last one has to resolve the promise + resolve(undefined); + } + } + }).catch(reject); + } + }); +} + export function asPromise(callback: () => T | Thenable): Promise { return new Promise((resolve, reject) => { const item = callback(); diff --git a/src/vs/editor/contrib/inlineCompletions/browser/controller/inlineCompletionsController.ts b/src/vs/editor/contrib/inlineCompletions/browser/controller/inlineCompletionsController.ts index 53e1dca17478ff..fd5f6543ecb74f 100644 --- a/src/vs/editor/contrib/inlineCompletions/browser/controller/inlineCompletionsController.ts +++ b/src/vs/editor/contrib/inlineCompletions/browser/controller/inlineCompletionsController.ts @@ -136,6 +136,7 @@ export class InlineCompletionsController extends Disposable { private readonly _playAccessibilitySignal = observableSignal(this); private readonly _fontFamily = this._editorObs.getOption(EditorOption.inlineSuggest).map(val => val.fontFamily); + private readonly _hideInlineEditOnSelectionChange = this._editorObs.getOption(EditorOption.inlineSuggest).map(val => true); constructor( public readonly editor: ICodeEditor, @@ -178,6 +179,9 @@ export class InlineCompletionsController extends Disposable { this._register(runOnChange(this._editorObs.selections, (_value, _, changes) => { if (changes.some(e => e.reason === CursorChangeReason.Explicit || e.source === 'api')) { + if (!this._hideInlineEditOnSelectionChange.get() && this.model.get()?.stateWithInlineEdit.get()?.kind === 'inlineEdit') { + return; + } this.model.get()?.stop(); } })); diff --git a/src/vs/editor/contrib/inlineCompletions/browser/model/inlineCompletionsSource.ts b/src/vs/editor/contrib/inlineCompletions/browser/model/inlineCompletionsSource.ts index 7031a4fb698153..4d97d184d6b003 100644 --- a/src/vs/editor/contrib/inlineCompletions/browser/model/inlineCompletionsSource.ts +++ b/src/vs/editor/contrib/inlineCompletions/browser/model/inlineCompletionsSource.ts @@ -115,6 +115,7 @@ export class InlineCompletionsSource extends Disposable { } if (source.token.isCancellationRequested || this._store.isDisposed || this._textModel.getVersionId() !== request.versionId) { + updatedCompletions.dispose(); return false; } diff --git a/src/vs/editor/contrib/inlineCompletions/browser/model/inlineEditsAdapter.ts b/src/vs/editor/contrib/inlineCompletions/browser/model/inlineEditsAdapter.ts index c03fb7bfa610bb..c2016411b7a9a7 100644 --- a/src/vs/editor/contrib/inlineCompletions/browser/model/inlineEditsAdapter.ts +++ b/src/vs/editor/contrib/inlineCompletions/browser/model/inlineEditsAdapter.ts @@ -84,6 +84,9 @@ export class InlineEditsAdapter extends Disposable { e.provider.freeInlineEdit(e.result); } } + toString(): string { + return 'InlineEditsAdapter'; + } })); })); } diff --git a/src/vs/editor/contrib/inlineCompletions/browser/model/provideInlineCompletions.ts b/src/vs/editor/contrib/inlineCompletions/browser/model/provideInlineCompletions.ts index fc7e6b42ee1c83..70ff7f7557a54d 100644 --- a/src/vs/editor/contrib/inlineCompletions/browser/model/provideInlineCompletions.ts +++ b/src/vs/editor/contrib/inlineCompletions/browser/model/provideInlineCompletions.ts @@ -4,16 +4,16 @@ *--------------------------------------------------------------------------------------------*/ import { assertNever } from '../../../../../base/common/assert.js'; -import { DeferredPromise } from '../../../../../base/common/async.js'; -import { CancellationToken } from '../../../../../base/common/cancellation.js'; +import { DeferredPromise, raceFilter } from '../../../../../base/common/async.js'; +import { CancellationToken, CancellationTokenSource } from '../../../../../base/common/cancellation.js'; import { SetMap } from '../../../../../base/common/map.js'; import { onUnexpectedExternalError } from '../../../../../base/common/errors.js'; -import { IDisposable } from '../../../../../base/common/lifecycle.js'; +import { Disposable, IDisposable } from '../../../../../base/common/lifecycle.js'; import { ISingleEditOperation } from '../../../../common/core/editOperation.js'; import { Position } from '../../../../common/core/position.js'; import { Range } from '../../../../common/core/range.js'; import { LanguageFeatureRegistry } from '../../../../common/languageFeatureRegistry.js'; -import { Command, InlineCompletion, InlineCompletionContext, InlineCompletionProviderGroupId, InlineCompletions, InlineCompletionsProvider } from '../../../../common/languages.js'; +import { Command, InlineCompletion, InlineCompletionContext, InlineCompletionProviderGroupId, InlineCompletions, InlineCompletionsProvider, InlineCompletionTriggerKind } from '../../../../common/languages.js'; import { ILanguageConfigurationService } from '../../../../common/languages/languageConfigurationRegistry.js'; import { ITextModel } from '../../../../common/model.js'; import { fixBracketsInLine } from '../../../../common/model/bracketPairsTextModelPart/fixBrackets.js'; @@ -22,16 +22,19 @@ import { getReadonlyEmptyArray } from '../utils.js'; import { SnippetParser, Text } from '../../../snippet/browser/snippetParser.js'; import { LineEditWithAdditionalLines } from '../../../../common/tokenizationTextModelPart.js'; import { OffsetRange } from '../../../../common/core/offsetRange.js'; +import { isDefined } from '../../../../../base/common/types.js'; export async function provideInlineCompletions( registry: LanguageFeatureRegistry, positionOrRange: Position | Range, model: ITextModel, context: InlineCompletionContext, - token: CancellationToken = CancellationToken.None, + baseToken: CancellationToken = CancellationToken.None, languageConfigurationService?: ILanguageConfigurationService, ): Promise { - // Important: Don't use position after the await calls, as the model could have been changed in the meantime! + const tokenSource = new CancellationTokenSource(baseToken); + const token = tokenSource.token; + const defaultReplaceRange = positionOrRange instanceof Position ? getDefaultRange(positionOrRange, model) : positionOrRange; const providers = registry.all(model); @@ -54,11 +57,14 @@ export async function provideInlineCompletions( return result; } - type Result = Promise | null | undefined>; - const states = new Map>, Result>(); + type Result = Promise; + const states = new Map(); - const seen = new Set>>(); - function findPreferredProviderCircle(provider: InlineCompletionsProvider, stack: InlineCompletionsProvider[]): InlineCompletionsProvider[] | undefined { + const seen = new Set(); + function findPreferredProviderCircle( + provider: InlineCompletionsProvider, + stack: InlineCompletionsProvider[] + ): InlineCompletionsProvider[] | undefined { stack = [...stack, provider]; if (seen.has(provider)) { return stack; } @@ -75,65 +81,109 @@ export async function provideInlineCompletions( return undefined; } - function processProvider(provider: InlineCompletionsProvider): Result { + function queryProviderOrPreferredProvider(provider: InlineCompletionsProvider): Result { const state = states.get(provider); - if (state) { - return state; - } + if (state) { return state; } const circle = findPreferredProviderCircle(provider, []); if (circle) { - onUnexpectedExternalError(new Error(`Inline completions: cyclic yield-to dependency detected. Path: ${circle.map(s => s.toString ? s.toString() : ('' + s)).join(' -> ')}`)); + onUnexpectedExternalError(new Error(`Inline completions: cyclic yield-to dependency detected.` + + ` Path: ${circle.map(s => s.toString ? s.toString() : ('' + s)).join(' -> ')}`)); } - const deferredPromise = new DeferredPromise | null | undefined>(); + const deferredPromise = new DeferredPromise(); states.set(provider, deferredPromise.p); (async () => { if (!circle) { const preferred = getPreferredProviders(provider); for (const p of preferred) { - const result = await processProvider(p); - if (result && result.items.length > 0) { + const result = await queryProviderOrPreferredProvider(p); + if (result && result.inlineCompletions.items.length > 0) { // Skip provider return undefined; } } } - try { - if (positionOrRange instanceof Position) { - const completions = await provider.provideInlineCompletions(model, positionOrRange, context, token); - return completions; - } else { - const completions = await provider.provideInlineEditsForRange?.(model, positionOrRange, context, token); - return completions; - } - } catch (e) { - onUnexpectedExternalError(e); - return undefined; - } + return query(provider); })().then(c => deferredPromise.complete(c), e => deferredPromise.error(e)); return deferredPromise.p; } - const providerResults = await Promise.all(providers.map(async provider => ({ provider, completions: await processProvider(provider) }))); + async function query(provider: InlineCompletionsProvider): Promise { + let result: InlineCompletions | null | undefined; + try { + if (positionOrRange instanceof Position) { + result = await provider.provideInlineCompletions(model, positionOrRange, context, token); + } else { + result = await provider.provideInlineEditsForRange?.(model, positionOrRange, context, token); + } + } catch (e) { + onUnexpectedExternalError(e); + return undefined; + } + + if (!result) { return undefined; } + const list = new InlineCompletionList(result, provider); + + runWhenCancelled(token, () => list.removeRef()); + return list; + } + + const promises = providers.map(queryProviderOrPreferredProvider); + let inlineCompletionLists: (InlineCompletionList | undefined)[]; + if (context.triggerKind === InlineCompletionTriggerKind.Automatic) { + // in automatic mode, we only show the first result. + // When the user cycles through the completions, it will be an explicit request. + inlineCompletionLists = [await raceFilter(promises, result => !!result && result.inlineCompletions.items.length > 0)]; + } else { + inlineCompletionLists = await Promise.all(promises); + } + + if (token.isCancellationRequested) { + tokenSource.dispose(true); + // result has been disposed before we could call addRef! So we have to discard everything. + return new InlineCompletionProviderResult([], new Set(), []); + } + + const result = addRefAndCreateResult(inlineCompletionLists, defaultReplaceRange, model, languageConfigurationService); + tokenSource.dispose(true); // This disposes results that are not referenced. + return result; +} + +/** If the token does not leak, this will not leak either. */ +function runWhenCancelled(token: CancellationToken, callback: () => void): IDisposable { + if (token.isCancellationRequested) { + callback(); + return Disposable.None; + } else { + const listener = token.onCancellationRequested(() => { + listener.dispose(); + callback(); + }); + return { dispose: () => listener.dispose() }; + } +} + +function addRefAndCreateResult( + inlineCompletionLists: (InlineCompletionList | undefined)[], + defaultReplaceRange: Range, + model: ITextModel, + languageConfigurationService: ILanguageConfigurationService | undefined +): InlineCompletionProviderResult { + // for deduplication const itemsByHash = new Map(); - const lists: InlineCompletionList[] = []; - for (const result of providerResults) { - const completions = result.completions; - if (!completions) { - continue; - } - const list = new InlineCompletionList(completions, result.provider); - lists.push(list); - for (const item of completions.items) { + const lists = inlineCompletionLists.filter(isDefined); + for (const completions of lists) { + completions.addRef(); + for (const item of completions.inlineCompletions.items) { const inlineCompletionItem = InlineCompletionItem.from( item, - list, + completions, defaultReplaceRange, model, languageConfigurationService From 321e1e5b8a0af43a0ee7549713f606936a1ac9ac Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Tue, 8 Oct 2024 18:07:33 -0700 Subject: [PATCH 14/18] Pick up TS 5.6.3 (#230848) Should fix the ATA regression we're seeing reported: https://github.com/microsoft/TypeScript/issues/60156 --- extensions/package-lock.json | 8 ++++---- extensions/package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/extensions/package-lock.json b/extensions/package-lock.json index 82f695dc38f5db..aa07b93e52e89d 100644 --- a/extensions/package-lock.json +++ b/extensions/package-lock.json @@ -10,7 +10,7 @@ "hasInstallScript": true, "license": "MIT", "dependencies": { - "typescript": "5.6.2" + "typescript": "5.6.3" }, "devDependencies": { "@parcel/watcher": "2.1.0", @@ -606,9 +606,9 @@ } }, "node_modules/typescript": { - "version": "5.6.2", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.6.2.tgz", - "integrity": "sha512-NW8ByodCSNCwZeghjN3o+JX5OFH0Ojg6sadjEKY4huZ52TqbJTJnDo5+Tw98lSy63NZvi4n+ez5m2u5d4PkZyw==", + "version": "5.6.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.6.3.tgz", + "integrity": "sha512-hjcS1mhfuyi4WW8IWtjP7brDrG2cuDZukyrYrSauoXGNgx0S7zceP07adYkJycEr56BOUTNPzbInooiN3fn1qw==", "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" diff --git a/extensions/package.json b/extensions/package.json index 7f3fad949be9d2..a1a224947891b8 100644 --- a/extensions/package.json +++ b/extensions/package.json @@ -4,7 +4,7 @@ "license": "MIT", "description": "Dependencies shared by all extensions", "dependencies": { - "typescript": "5.6.2" + "typescript": "5.6.3" }, "scripts": { "postinstall": "node ./postinstall.mjs" From 6148e1487ab4f6d1045d5f0727a4c257b4b8fee9 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 8 Oct 2024 21:28:01 -0700 Subject: [PATCH 15/18] lmTools API polish (#230847) * Simplify lmTools part names * Remove unneeded isError * Remove text/plain requirement for tools * One more * Bump API version * Fix build --- .../common/extensionsApiProposals.ts | 2 +- .../workbench/api/common/extHost.api.impl.ts | 6 ++-- .../api/common/extHostLanguageModels.ts | 10 +++--- src/vs/workbench/api/common/extHostTypes.ts | 8 ++--- .../chat/browser/languageModelToolsService.ts | 5 --- .../tools/languageModelToolsContribution.ts | 4 +-- .../vscode.proposed.chatProvider.d.ts | 2 +- src/vscode-dts/vscode.proposed.lmTools.d.ts | 34 +++++++------------ 8 files changed, 29 insertions(+), 42 deletions(-) diff --git a/src/vs/platform/extensions/common/extensionsApiProposals.ts b/src/vs/platform/extensions/common/extensionsApiProposals.ts index ac432211095faf..7d844383e9139f 100644 --- a/src/vs/platform/extensions/common/extensionsApiProposals.ts +++ b/src/vs/platform/extensions/common/extensionsApiProposals.ts @@ -244,7 +244,7 @@ const _allApiProposals = { }, lmTools: { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.lmTools.d.ts', - version: 7 + version: 8 }, mappedEditsProvider: { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.mappedEditsProvider.d.ts', diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 83e055e89342de..d604b1d151db89 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -1776,9 +1776,9 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I ChatReferenceBinaryData: extHostTypes.ChatReferenceBinaryData, LanguageModelChatMessageRole: extHostTypes.LanguageModelChatMessageRole, LanguageModelChatMessage: extHostTypes.LanguageModelChatMessage, - LanguageModelChatMessageToolResultPart: extHostTypes.LanguageModelToolResultPart, - LanguageModelChatResponseTextPart: extHostTypes.LanguageModelTextPart, - LanguageModelChatResponseToolCallPart: extHostTypes.LanguageModelToolCallPart, + LanguageModelToolResultPart: extHostTypes.LanguageModelToolResultPart, + LanguageModelTextPart: extHostTypes.LanguageModelTextPart, + LanguageModelToolCallPart: extHostTypes.LanguageModelToolCallPart, LanguageModelError: extHostTypes.LanguageModelError, NewSymbolName: extHostTypes.NewSymbolName, NewSymbolNameTag: extHostTypes.NewSymbolNameTag, diff --git a/src/vs/workbench/api/common/extHostLanguageModels.ts b/src/vs/workbench/api/common/extHostLanguageModels.ts index 95a9f2d1a803e5..2e8bbc88575ddb 100644 --- a/src/vs/workbench/api/common/extHostLanguageModels.ts +++ b/src/vs/workbench/api/common/extHostLanguageModels.ts @@ -37,13 +37,13 @@ type LanguageModelData = { class LanguageModelResponseStream { - readonly stream = new AsyncIterableSource(); + readonly stream = new AsyncIterableSource(); constructor( readonly option: number, - stream?: AsyncIterableSource + stream?: AsyncIterableSource ) { - this.stream = stream ?? new AsyncIterableSource(); + this.stream = stream ?? new AsyncIterableSource(); } } @@ -52,7 +52,7 @@ class LanguageModelResponse { readonly apiObject: vscode.LanguageModelChatResponse; private readonly _responseStreams = new Map(); - private readonly _defaultStream = new AsyncIterableSource(); + private readonly _defaultStream = new AsyncIterableSource(); private _isDone: boolean = false; constructor() { @@ -100,7 +100,7 @@ class LanguageModelResponse { this._responseStreams.set(fragment.index, res); } - let out: vscode.LanguageModelChatResponseTextPart | vscode.LanguageModelChatResponseToolCallPart; + let out: vscode.LanguageModelTextPart | vscode.LanguageModelToolCallPart; if (fragment.part.type === 'text') { out = new extHostTypes.LanguageModelTextPart(fragment.part.value); } else { diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index 956c9897bcb3b9..f50208a09ad78a 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -4583,7 +4583,7 @@ export enum LanguageModelChatMessageRole { System = 3 } -export class LanguageModelToolResultPart implements vscode.LanguageModelChatMessageToolResultPart { +export class LanguageModelToolResultPart implements vscode.LanguageModelToolResultPart { toolCallId: string; content: string; @@ -4610,7 +4610,7 @@ export class LanguageModelChatMessage implements vscode.LanguageModelChatMessage role: vscode.LanguageModelChatMessageRole; content: string; - content2: (string | vscode.LanguageModelChatMessageToolResultPart | vscode.LanguageModelChatResponseToolCallPart)[]; + content2: (string | vscode.LanguageModelToolResultPart | vscode.LanguageModelToolCallPart)[]; name: string | undefined; constructor(role: vscode.LanguageModelChatMessageRole, content: string, name?: string) { @@ -4621,7 +4621,7 @@ export class LanguageModelChatMessage implements vscode.LanguageModelChatMessage } } -export class LanguageModelToolCallPart implements vscode.LanguageModelChatResponseToolCallPart { +export class LanguageModelToolCallPart implements vscode.LanguageModelToolCallPart { name: string; toolCallId: string; parameters: any; @@ -4633,7 +4633,7 @@ export class LanguageModelToolCallPart implements vscode.LanguageModelChatRespon } } -export class LanguageModelTextPart implements vscode.LanguageModelChatResponseTextPart { +export class LanguageModelTextPart implements vscode.LanguageModelTextPart { value: string; constructor(value: string) { diff --git a/src/vs/workbench/contrib/chat/browser/languageModelToolsService.ts b/src/vs/workbench/contrib/chat/browser/languageModelToolsService.ts index bf464385d97e08..226acd4f4a4f13 100644 --- a/src/vs/workbench/contrib/chat/browser/languageModelToolsService.ts +++ b/src/vs/workbench/contrib/chat/browser/languageModelToolsService.ts @@ -57,11 +57,6 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo throw new Error(`Tool "${toolData.id}" is already registered.`); } - // Ensure that text/plain is supported - if (!toolData.supportedContentTypes.includes('text/plain')) { - toolData.supportedContentTypes.push('text/plain'); - } - this._tools.set(toolData.id, { data: toolData }); this._onDidChangeToolsScheduler.schedule(); diff --git a/src/vs/workbench/contrib/chat/common/tools/languageModelToolsContribution.ts b/src/vs/workbench/contrib/chat/common/tools/languageModelToolsContribution.ts index 13a6356b5c0c62..2bb02dd9cede93 100644 --- a/src/vs/workbench/contrib/chat/common/tools/languageModelToolsContribution.ts +++ b/src/vs/workbench/contrib/chat/common/tools/languageModelToolsContribution.ts @@ -39,7 +39,7 @@ const languageModelToolsExtensionPoint = extensionsRegistry.ExtensionsRegistry.r } }, jsonSchema: { - description: localize('vscode.extension.contributes.tools', 'Contributes a tool that can be invoked by a language model.'), + description: localize('vscode.extension.contributes.tools', 'Contributes a tool that can be invoked by a language model in a chat session, or from a standalone command. Registered tools can be used by all extensions.'), type: 'array', items: { additionalProperties: false, @@ -103,7 +103,7 @@ const languageModelToolsExtensionPoint = extensionsRegistry.ExtensionsRegistry.r type: 'string' }, supportedContentTypes: { - markdownDescription: localize('contentTypes', "The list of content types that this tool can return. It's required that tools support `text/plain`, and that is assumed even if not specified here. Another example could be the contentType exported by the `@vscode/prompt-tsx` library."), + markdownDescription: localize('contentTypes', "The list of content types that this tool can return. It's recommended that all tools support `text/plain`, which would indicate any text-based content. Another example is the contentType exported by the `@vscode/prompt-tsx` library, which would let a tool return a `PromptElementJSON` which can be easily rendered in a prompt by an extension using `@vscode/prompt-tsx`."), type: 'array', items: { type: 'string' diff --git a/src/vscode-dts/vscode.proposed.chatProvider.d.ts b/src/vscode-dts/vscode.proposed.chatProvider.d.ts index be60001825727f..3ab4fb3d3dab79 100644 --- a/src/vscode-dts/vscode.proposed.chatProvider.d.ts +++ b/src/vscode-dts/vscode.proposed.chatProvider.d.ts @@ -12,7 +12,7 @@ declare module 'vscode' { export interface ChatResponseFragment2 { index: number; - part: LanguageModelChatResponseTextPart | LanguageModelChatResponseToolCallPart; + part: LanguageModelTextPart | LanguageModelToolCallPart; } // @API extension ship a d.ts files for their options diff --git a/src/vscode-dts/vscode.proposed.lmTools.d.ts b/src/vscode-dts/vscode.proposed.lmTools.d.ts index 86b39748dbf501..c5ecb486a82f61 100644 --- a/src/vscode-dts/vscode.proposed.lmTools.d.ts +++ b/src/vscode-dts/vscode.proposed.lmTools.d.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -// version: 7 +// version: 8 // https://github.com/microsoft/vscode/issues/213274 declare module 'vscode' { @@ -30,8 +30,7 @@ declare module 'vscode' { } // LM -> USER: function that should be used - // TODO@API NAME: LanguageModelChatMessageToolCallPart, LanguageModelToolCallPart - export class LanguageModelChatResponseToolCallPart { + export class LanguageModelToolCallPart { name: string; toolCallId: string; parameters: any; @@ -40,26 +39,23 @@ declare module 'vscode' { } // LM -> USER: text chunk - // TODO@API NAME: LanguageModelChatMessageTextPart, LanguageModelTextPart - export class LanguageModelChatResponseTextPart { + export class LanguageModelTextPart { value: string; constructor(value: string); } export interface LanguageModelChatResponse { - stream: AsyncIterable; + stream: AsyncIterable; } // USER -> LM: the result of a function call - // TODO@API NAME: LanguageModelChatMessageToolResultPart, LanguageModelToolResultPart - export class LanguageModelChatMessageToolResultPart { + export class LanguageModelToolResultPart { toolCallId: string; content: string; - isError: boolean; - constructor(toolCallId: string, content: string, isError?: boolean); + constructor(toolCallId: string, content: string); } export interface LanguageModelChatMessage { @@ -68,10 +64,10 @@ declare module 'vscode' { * Some parts would be message-type specific for some models and wouldn't go together, * but it's up to the chat provider to decide what to do about that. * Can drop parts that are not valid for the message type. - * LanguageModelChatMessageToolResultPart: only on User messages - * LanguageModelChatResponseToolCallPart: only on Assistant messages + * LanguageModelToolResultPart: only on User messages + * LanguageModelToolCallPart: only on Assistant messages */ - content2: (string | LanguageModelChatMessageToolResultPart | LanguageModelChatResponseToolCallPart)[]; + content2: (string | LanguageModelToolResultPart | LanguageModelToolCallPart)[]; } // Tool registration/invoking between extensions @@ -84,15 +80,11 @@ declare module 'vscode' { /** * The result can contain arbitrary representations of the content. A tool user can set * {@link LanguageModelToolInvocationOptions.requested} to request particular types, and a tool implementation should only - * compute the types that were requested. `text/plain` is required to be supported by all tools. Another example might be - * a `PromptElementJSON` from `@vscode/prompt-tsx`, using the `contentType` exported by that library. + * compute the types that were requested. `text/plain` is recommended to be supported by all tools, which would indicate + * any text-based content. Another example might be a `PromptElementJSON` from `@vscode/prompt-tsx`, using the + * `contentType` exported by that library. */ [contentType: string]: any; - - /** - * A string representation of the result. - */ - 'text/plain'?: string; } export namespace lm { @@ -139,7 +131,7 @@ declare module 'vscode' { /** * A tool user can request that particular content types be returned from the tool, depending on what the tool user - * supports. All tools are required to support `text/plain`. See {@link LanguageModelToolResult}. + * supports. All tools are recommended to support `text/plain`. See {@link LanguageModelToolResult}. */ requestedContentTypes: string[]; From a016ec9b66ffdd3ff0f831768b8e75be008a54e4 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 8 Oct 2024 21:28:49 -0700 Subject: [PATCH 16/18] try/catch participant registration (#230861) So that an invalid one doesn't break the whole process --- .../browser/chatParticipantContributions.ts | 65 ++++++++++--------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatParticipantContributions.ts b/src/vs/workbench/contrib/chat/browser/chatParticipantContributions.ts index e5e0949dab318d..c5135968d7d7ef 100644 --- a/src/vs/workbench/contrib/chat/browser/chatParticipantContributions.ts +++ b/src/vs/workbench/contrib/chat/browser/chatParticipantContributions.ts @@ -5,6 +5,7 @@ import { coalesce, isNonEmptyArray } from '../../../../base/common/arrays.js'; import { Codicon } from '../../../../base/common/codicons.js'; +import { toErrorMessage } from '../../../../base/common/errorMessage.js'; import { Event } from '../../../../base/common/event.js'; import { Disposable, DisposableMap, DisposableStore, IDisposable, toDisposable } from '../../../../base/common/lifecycle.js'; import * as strings from '../../../../base/common/strings.js'; @@ -235,37 +236,41 @@ export class ChatExtensionPointHandler implements IWorkbenchContribution { } } - const store = new DisposableStore(); - store.add(this._chatAgentService.registerAgent( - providerDescriptor.id, - { - extensionId: extension.description.identifier, - publisherDisplayName: extension.description.publisherDisplayName ?? extension.description.publisher, // May not be present in OSS - extensionPublisherId: extension.description.publisher, - extensionDisplayName: extension.description.displayName ?? extension.description.name, - id: providerDescriptor.id, - description: providerDescriptor.description, - supportsModelPicker: providerDescriptor.supportsModelPicker, - when: providerDescriptor.when, - metadata: { - isSticky: providerDescriptor.isSticky, - sampleRequest: providerDescriptor.sampleRequest, - }, - name: providerDescriptor.name, - fullName: providerDescriptor.fullName, - isDefault: providerDescriptor.isDefault, - locations: isNonEmptyArray(providerDescriptor.locations) ? - providerDescriptor.locations.map(ChatAgentLocation.fromRaw) : - [ChatAgentLocation.Panel], - slashCommands: providerDescriptor.commands ?? [], - disambiguation: coalesce(participantsAndCommandsDisambiguation.flat()), - supportsToolReferences: providerDescriptor.supportsToolReferences, - } satisfies IChatAgentData)); + try { + const store = new DisposableStore(); + store.add(this._chatAgentService.registerAgent( + providerDescriptor.id, + { + extensionId: extension.description.identifier, + publisherDisplayName: extension.description.publisherDisplayName ?? extension.description.publisher, // May not be present in OSS + extensionPublisherId: extension.description.publisher, + extensionDisplayName: extension.description.displayName ?? extension.description.name, + id: providerDescriptor.id, + description: providerDescriptor.description, + supportsModelPicker: providerDescriptor.supportsModelPicker, + when: providerDescriptor.when, + metadata: { + isSticky: providerDescriptor.isSticky, + sampleRequest: providerDescriptor.sampleRequest, + }, + name: providerDescriptor.name, + fullName: providerDescriptor.fullName, + isDefault: providerDescriptor.isDefault, + locations: isNonEmptyArray(providerDescriptor.locations) ? + providerDescriptor.locations.map(ChatAgentLocation.fromRaw) : + [ChatAgentLocation.Panel], + slashCommands: providerDescriptor.commands ?? [], + disambiguation: coalesce(participantsAndCommandsDisambiguation.flat()), + supportsToolReferences: providerDescriptor.supportsToolReferences, + } satisfies IChatAgentData)); - this._participantRegistrationDisposables.set( - getParticipantKey(extension.description.identifier, providerDescriptor.id), - store - ); + this._participantRegistrationDisposables.set( + getParticipantKey(extension.description.identifier, providerDescriptor.id), + store + ); + } catch (e) { + this.logService.error(`Failed to register participant ${providerDescriptor.id}: ${toErrorMessage(e, true)}`); + } } } From 8329e3a760c3e1bcf0cc71fb6a9ea2de1e9bdb66 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 9 Oct 2024 12:20:25 +0200 Subject: [PATCH 17/18] simplify save flow (#230883) --- .../contrib/chat/browser/chatEditorSaving.ts | 40 +++++++++---------- .../textfile/common/textFileEditorModel.ts | 6 +++ .../common/textFileSaveParticipant.ts | 13 +++++- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts b/src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts index a1991fc9fea263..0f1cc24f54e2dd 100644 --- a/src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts +++ b/src/vs/workbench/contrib/chat/browser/chatEditorSaving.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Queue } from '../../../../base/common/async.js'; +import { CancellationError } from '../../../../base/common/errors.js'; import { Disposable, DisposableMap, DisposableStore, MutableDisposable } from '../../../../base/common/lifecycle.js'; import { localize, localize2 } from '../../../../nls.js'; import { Action2, registerAction2 } from '../../../../platform/actions/common/actions.js'; @@ -20,7 +21,7 @@ import { IChatEditingService, IChatEditingSession, WorkingSetEntryState } from ' import { CHAT_CATEGORY } from './actions/chatActions.js'; -const _storageKey = 'workbench.chat.editorSaving'; +const _storageKey = 'workbench.chat.saveWithAiGeneratedChanges'; export class ChatEditorSaving extends Disposable implements IWorkbenchContribution { @@ -48,8 +49,8 @@ export class ChatEditorSaving extends Disposable implements IWorkbenchContributi store.clear(); - const alwaysAcceptOnSave = this._storageService.getBoolean(_storageKey, StorageScope.PROFILE, false); - if (alwaysAcceptOnSave) { + const alwaysSave = this._storageService.getBoolean(_storageKey, StorageScope.PROFILE, false); + if (alwaysSave) { return; } @@ -78,9 +79,8 @@ export class ChatEditorSaving extends Disposable implements IWorkbenchContributi await queue.queue(async () => { // this might have changed in the meantime and there is checked again and acted upon - const alwaysAcceptOnSave = this._storageService.getBoolean(_storageKey, StorageScope.PROFILE, false); - if (alwaysAcceptOnSave) { - await session.accept(workingCopy.resource); + const alwaysSave = this._storageService.getBoolean(_storageKey, StorageScope.PROFILE, false); + if (alwaysSave) { return; } @@ -88,30 +88,28 @@ export class ChatEditorSaving extends Disposable implements IWorkbenchContributi const filelabel = labelService.getUriBasenameLabel(workingCopy.resource); const message = agentName - ? localize('message.1', "Do you want to accept the changes {0} made in {1}?", agentName, filelabel) - : localize('message.2', "Do you want to accept the changes chat made in {1}?", filelabel); + ? localize('message.1', "Do you want to save the changes {0} made in {1}?", agentName, filelabel) + : localize('message.2', "Do you want to save the changes chat made in {0}?", filelabel); const result = await this._dialogService.confirm({ message, detail: localize('detail', "AI-generated changes may be incorect and should be reviewed before saving.", agentName), - primaryButton: localize('save', "Accept & Save"), - cancelButton: localize('discard', "Discard & Save"), + primaryButton: localize('save', "Save"), + cancelButton: localize('discard', "Cancel"), checkbox: { - label: localize('config', "Always accept edits when saving"), + label: localize('config', "Always save with AI-generated changes"), checked: false } }); - if (result.confirmed) { - await session.accept(workingCopy.resource); - - if (result.checkboxChecked) { - // remember choice - this._storageService.store(_storageKey, true, StorageScope.PROFILE, StorageTarget.USER); - } + if (!result.confirmed) { + // cancel the save + throw new CancellationError(); + } - } else { - await session.reject(workingCopy.resource); + if (result.checkboxChecked) { + // remember choice + this._storageService.store(_storageKey, true, StorageScope.PROFILE, StorageTarget.USER); } }); } @@ -159,7 +157,7 @@ registerAction2(class extends Action2 { constructor() { super({ id: 'workbench.action.resetChatEditorSaving', - title: localize2('resetChatEditorSaving', "Reset Choise for 'Always accept edits when saving'"), + title: localize2('resetChatEditorSaving', "Reset Choise for 'Always save with AI-generated changes'"), category: CHAT_CATEGORY, f1: true }); diff --git a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts index febcf153c8b04b..31b4827edfb0fa 100644 --- a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts +++ b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts @@ -34,6 +34,7 @@ import { PLAINTEXT_LANGUAGE_ID } from '../../../../editor/common/languages/modes import { IExtensionService } from '../../extensions/common/extensions.js'; import { IMarkdownString } from '../../../../base/common/htmlContent.js'; import { IProgress, IProgressService, IProgressStep, ProgressLocation } from '../../../../platform/progress/common/progress.js'; +import { isCancellationError } from '../../../../base/common/errors.js'; interface IBackupMetaData extends IWorkingCopyBackupMeta { mtime: number; @@ -870,6 +871,11 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil this.ignoreSaveFromSaveParticipants = true; try { await this.textFileService.files.runSaveParticipants(this, { reason: options.reason ?? SaveReason.EXPLICIT, savedFrom: options.from }, progress, saveCancellation.token); + } catch (err) { + if (isCancellationError(err) && !saveCancellation.token.isCancellationRequested) { + // participant wants to cancel this operation + saveCancellation.cancel(); + } } finally { this.ignoreSaveFromSaveParticipants = false; } diff --git a/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts b/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts index 80f5d3a27cbd42..521e78198e8aa8 100644 --- a/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts +++ b/src/vs/workbench/services/textfile/common/textFileSaveParticipant.ts @@ -12,7 +12,7 @@ import { IDisposable, Disposable, toDisposable } from '../../../../base/common/l import { LinkedList } from '../../../../base/common/linkedList.js'; import { localize } from '../../../../nls.js'; import { NotificationPriority } from '../../../../platform/notification/common/notification.js'; -import { isCancellationError } from '../../../../base/common/errors.js'; +import { CancellationError, isCancellationError } from '../../../../base/common/errors.js'; export class TextFileSaveParticipant extends Disposable { @@ -42,6 +42,8 @@ export class TextFileSaveParticipant extends Disposable { message: localize('saveParticipants1', "Running Code Actions and Formatters...") }); + let bubbleCancel = false; + // create an "inner" progress to allow to skip over long running save participants await this.progressService.withProgress({ priority: NotificationPriority.URGENT, @@ -60,6 +62,11 @@ export class TextFileSaveParticipant extends Disposable { } catch (err) { if (!isCancellationError(err)) { this.logService.error(err); + } else if (!cts.token.isCancellationRequested) { + // we see a cancellation error BUT the token didn't signal it + // this means the participant wants the save operation to be cancelled + cts.cancel(); + bubbleCancel = true; } } } @@ -71,6 +78,10 @@ export class TextFileSaveParticipant extends Disposable { model.textEditorModel?.pushStackElement(); cts.dispose(); + + if (bubbleCancel) { + throw new CancellationError(); + } } override dispose(): void { From bb09960d30806b18e432a835b430c21e107f6aad Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Wed, 9 Oct 2024 13:09:12 +0200 Subject: [PATCH 18/18] add descriptions for enums (#230898) --- src/vs/workbench/browser/parts/editor/editorCommands.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/browser/parts/editor/editorCommands.ts b/src/vs/workbench/browser/parts/editor/editorCommands.ts index 6dda6ad5348bb7..d0ceaa9a6c8df0 100644 --- a/src/vs/workbench/browser/parts/editor/editorCommands.ts +++ b/src/vs/workbench/browser/parts/editor/editorCommands.ts @@ -335,7 +335,11 @@ function registerEditorGroupsLayoutCommands(): void { 'orientation': { 'type': 'number', 'default': 0, - 'enum': [0, 1] + 'enum': [0, 1], + 'enumDescriptions': [ + localize('editorGroupLayout.horizontal', "Horizontal"), + localize('editorGroupLayout.vertical', "Vertical") + ], }, 'groups': { '$ref': '#/definitions/editorGroupsSchema',