From 95f79a91e46503e5ee849a3069bf143e200825dc Mon Sep 17 00:00:00 2001 From: Ulugbek Abdullaev Date: Wed, 20 Sep 2023 11:01:36 +0200 Subject: [PATCH 1/7] refactor(chatModel): * create a separate type to represent a response part * rename existing `ResponsePart` to `InternalResponsePart` * fix? handle if response part's resolved value is a markdown string, which is allowed in the `IResponse` interface --- .../contrib/chat/common/chatModel.ts | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/chat/common/chatModel.ts b/src/vs/workbench/contrib/chat/common/chatModel.ts index 26c82c87dd2b0..11eefe47deabd 100644 --- a/src/vs/workbench/contrib/chat/common/chatModel.ts +++ b/src/vs/workbench/contrib/chat/common/chatModel.ts @@ -23,10 +23,21 @@ export interface IChatRequestModel { readonly response: IChatResponseModel | undefined; } +export type ResponsePart = + | string + | IMarkdownString + | { treeData: IChatResponseProgressFileTreeData } + | { + placeholder: string; + resolvedContent?: Promise< + string | IMarkdownString | { treeData: IChatResponseProgressFileTreeData } + >; + }; + export interface IResponse { readonly value: (IMarkdownString | IPlaceholderMarkdownString | IChatResponseProgressFileTreeData)[]; onDidChangeValue: Event; - updateContent(responsePart: string | IMarkdownString | { treeData: IChatResponseProgressFileTreeData } | { placeholder: string; resolvedContent?: Promise }, quiet?: boolean): void; + updateContent(responsePart: ResponsePart, quiet?: boolean): void; asString(): string; } @@ -93,7 +104,10 @@ export interface IPlaceholderMarkdownString extends IMarkdownString { isPlaceholder: boolean; } -type ResponsePart = { string: IMarkdownString; isPlaceholder?: boolean } | { treeData: IChatResponseProgressFileTreeData; isPlaceholder?: undefined }; +type InternalResponsePart = + | { string: IMarkdownString; isPlaceholder?: boolean } + | { treeData: IChatResponseProgressFileTreeData; isPlaceholder?: undefined }; + export class Response implements IResponse { private _onDidChangeValue = new Emitter(); public get onDidChangeValue() { @@ -101,7 +115,7 @@ export class Response implements IResponse { } // responseParts internally tracks all the response parts, including strings which are currently resolving, so that they can be updated when they do resolve - private _responseParts: ResponsePart[]; + private _responseParts: InternalResponsePart[]; // responseData externally presents the response parts with consolidated contiguous strings (including strings which were previously resolving) private _responseData: (IMarkdownString | IPlaceholderMarkdownString | IChatResponseProgressFileTreeData)[]; // responseRepr externally presents the response parts with consolidated contiguous strings (excluding tree data) @@ -126,7 +140,7 @@ export class Response implements IResponse { return this._responseRepr; } - updateContent(responsePart: string | IMarkdownString | { treeData: IChatResponseProgressFileTreeData } | { placeholder: string; resolvedContent?: Promise }, quiet?: boolean): void { + updateContent(responsePart: ResponsePart, quiet?: boolean): void { if (typeof responsePart === 'string' || isMarkdownString(responsePart)) { const responsePartLength = this._responseParts.length - 1; const lastResponsePart = this._responseParts[responsePartLength]; @@ -154,6 +168,9 @@ export class Response implements IResponse { if (typeof content === 'string') { this._responseParts[responsePosition] = { string: new MarkdownString(content), isPlaceholder: true }; this._updateRepr(quiet); + } else if ('value' in content) { + this._responseParts[responsePosition] = { string: content, isPlaceholder: true }; + this._updateRepr(quiet); } else if (content.treeData) { this._responseParts[responsePosition] = { treeData: content.treeData }; this._updateRepr(quiet); @@ -257,7 +274,7 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel this._id = 'response_' + ChatResponseModel.nextId++; } - updateContent(responsePart: string | IMarkdownString | { treeData: IChatResponseProgressFileTreeData } | { placeholder: string; resolvedContent?: Promise }, quiet?: boolean) { + updateContent(responsePart: ResponsePart, quiet?: boolean) { this._response.updateContent(responsePart, quiet); } From 49dadc018dbf8f5e743d5810315500d88bcc67fa Mon Sep 17 00:00:00 2001 From: Ulugbek Abdullaev Date: Thu, 21 Sep 2023 11:58:53 +0200 Subject: [PATCH 2/7] mapped edits: implement support for getting doc ranges from InteractiveSessionProvider's --- .../workbench/api/browser/mainThreadChat.ts | 12 ++++++++++++ .../workbench/api/common/extHost.protocol.ts | 13 ++++++++++++- src/vs/workbench/api/common/extHostChat.ts | 8 ++++++++ .../contrib/chat/common/chatModel.ts | 15 +++++++++++++-- .../contrib/chat/common/chatService.ts | 17 ++++++++++++++++- .../contrib/chat/common/chatServiceImpl.ts | 4 +++- .../vscode.proposed.interactive.d.ts | 19 ++++++++++++++++++- 7 files changed, 82 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadChat.ts b/src/vs/workbench/api/browser/mainThreadChat.ts index 6b45f16279627..a6aa6ee451c5c 100644 --- a/src/vs/workbench/api/browser/mainThreadChat.ts +++ b/src/vs/workbench/api/browser/mainThreadChat.ts @@ -159,6 +159,18 @@ export class MainThreadChat extends Disposable implements MainThreadChatShape { return; } + if ('documents' in progress) { + const usedContext = { + documents: progress.documents.map(({ uri, version, ranges }) => ({ + uri: URI.revive(uri), + version, + ranges, + })), + }; + this._activeRequestProgressCallbacks.get(id)?.(usedContext); // FIXME@ulugbekna: is this a correct thing to do? + return; + } + this._activeRequestProgressCallbacks.get(id)?.(progress); } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index ea5ccab44fc79..289f9a81eafbb 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -1223,7 +1223,18 @@ export interface IChatResponseProgressFileTreeData { children?: IChatResponseProgressFileTreeData[]; } -export type IChatResponseProgressDto = { content: string | IMarkdownString } | { requestId: string } | { placeholder: string } | { treeData: IChatResponseProgressFileTreeData }; +export type IDocumentContextDto = { + uri: UriComponents; + version: number; + ranges: IRange[]; +}; + +export type IChatResponseProgressDto = + | { content: string | IMarkdownString } + | { requestId: string } + | { placeholder: string } + | { treeData: IChatResponseProgressFileTreeData } + | { documents: IDocumentContextDto[] }; export interface MainThreadChatShape extends IDisposable { $registerChatProvider(handle: number, id: string): Promise; diff --git a/src/vs/workbench/api/common/extHostChat.ts b/src/vs/workbench/api/common/extHostChat.ts index 9aa2c625cc9ac..265994b5f96b1 100644 --- a/src/vs/workbench/api/common/extHostChat.ts +++ b/src/vs/workbench/api/common/extHostChat.ts @@ -236,6 +236,14 @@ export class ExtHostChat implements ExtHostChatShape { this._proxy.$acceptResponseProgress(handle, sessionId, { content: typeof progress.content === 'string' ? progress.content : typeConvert.MarkdownString.from(progress.content) }); + } else if ('documents' in progress) { + this._proxy.$acceptResponseProgress(handle, sessionId, { + documents: progress.documents.map(d => ({ + uri: d.uri, + version: d.version, + ranges: d.ranges.map(r => typeConvert.Range.from(r)) + })) + }); } else { this._proxy.$acceptResponseProgress(handle, sessionId, progress); } diff --git a/src/vs/workbench/contrib/chat/common/chatModel.ts b/src/vs/workbench/contrib/chat/common/chatModel.ts index 11eefe47deabd..69ecd13929968 100644 --- a/src/vs/workbench/contrib/chat/common/chatModel.ts +++ b/src/vs/workbench/contrib/chat/common/chatModel.ts @@ -11,7 +11,7 @@ import { URI, UriComponents } from 'vs/base/common/uri'; import { generateUuid } from 'vs/base/common/uuid'; import { ILogService } from 'vs/platform/log/common/log'; import { IChatAgentData, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents'; -import { IChat, IChatFollowup, IChatProgress, IChatReplyFollowup, IChatResponse, IChatResponseErrorDetails, IChatResponseProgressFileTreeData, InteractiveSessionVoteDirection } from 'vs/workbench/contrib/chat/common/chatService'; +import { IChat, IChatFollowup, IChatProgress, IChatReplyFollowup, IChatResponse, IChatResponseErrorDetails, IChatResponseProgressFileTreeData, IUsedContext, InteractiveSessionVoteDirection } from 'vs/workbench/contrib/chat/common/chatService'; export interface IChatRequestModel { readonly id: string; @@ -32,11 +32,13 @@ export type ResponsePart = resolvedContent?: Promise< string | IMarkdownString | { treeData: IChatResponseProgressFileTreeData } >; - }; + } + | IUsedContext; export interface IResponse { readonly value: (IMarkdownString | IPlaceholderMarkdownString | IChatResponseProgressFileTreeData)[]; onDidChangeValue: Event; + usedContext: IUsedContext | undefined; updateContent(responsePart: ResponsePart, quiet?: boolean): void; asString(): string; } @@ -114,6 +116,11 @@ export class Response implements IResponse { return this._onDidChangeValue.event; } + private _usedContext: IUsedContext | undefined; + public get usedContext(): IUsedContext | undefined { + return this._usedContext; + } + // responseParts internally tracks all the response parts, including strings which are currently resolving, so that they can be updated when they do resolve private _responseParts: InternalResponsePart[]; // responseData externally presents the response parts with consolidated contiguous strings (including strings which were previously resolving) @@ -179,6 +186,8 @@ export class Response implements IResponse { } else if (isCompleteInteractiveProgressTreeData(responsePart)) { this._responseParts.push(responsePart); this._updateRepr(quiet); + } else if ('documents' in responsePart) { + this._usedContext = responsePart; } } @@ -590,6 +599,8 @@ export class ChatModel extends Disposable implements IChatModel { request.response.updateContent(progress.content, quiet); } else if ('placeholder' in progress || isCompleteInteractiveProgressTreeData(progress)) { request.response.updateContent(progress, quiet); + } else if ('documents' in progress) { + request.response.updateContent(progress); } else { request.setProviderRequestId(progress.requestId); request.response.setProviderResponseId(progress.requestId); diff --git a/src/vs/workbench/contrib/chat/common/chatService.ts b/src/vs/workbench/contrib/chat/common/chatService.ts index b97c1a7c995f6..5c71168c49332 100644 --- a/src/vs/workbench/contrib/chat/common/chatService.ts +++ b/src/vs/workbench/contrib/chat/common/chatService.ts @@ -8,6 +8,7 @@ import { Event } from 'vs/base/common/event'; import { IMarkdownString } from 'vs/base/common/htmlContent'; import { IDisposable } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; +import { IRange } from 'vs/editor/common/core/range'; import { ProviderResult } from 'vs/editor/common/languages'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { IChatModel, ChatModel, ISerializableChatData } from 'vs/workbench/contrib/chat/common/chatModel'; @@ -51,8 +52,22 @@ export interface IChatResponseProgressFileTreeData { children?: IChatResponseProgressFileTreeData[]; } +export type IDocumentContext = { + uri: URI; + version: number; + ranges: IRange[]; +}; + +export type IUsedContext = { + documents: IDocumentContext[]; +}; + export type IChatProgress = - { content: string | IMarkdownString } | { requestId: string } | { treeData: IChatResponseProgressFileTreeData } | { placeholder: string; resolvedContent: Promise }; + | { content: string | IMarkdownString } + | { requestId: string } + | { treeData: IChatResponseProgressFileTreeData } + | { placeholder: string; resolvedContent: Promise } + | IUsedContext; export interface IPersistedChatState { } export interface IChatProvider { diff --git a/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts b/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts index 75bed122f24c1..f5fe279835f5c 100644 --- a/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/chatServiceImpl.ts @@ -440,7 +440,6 @@ export class ChatService extends Disposable implements IChatService { const resolvedCommand = typeof message === 'string' && message.startsWith('/') ? await this.handleSlashCommand(model.sessionId, message) : message; - let gotProgress = false; const requestType = typeof message === 'string' ? (message.startsWith('/') ? 'slashCommand' : 'string') : @@ -453,6 +452,7 @@ export class ChatService extends Disposable implements IChatService { } gotProgress = true; + if ('content' in progress) { this.trace('sendRequest', `Provider returned progress for session ${model.sessionId}, ${typeof progress.content === 'string' ? progress.content.length : progress.content.value.length} chars`); } else if ('placeholder' in progress) { @@ -460,6 +460,8 @@ export class ChatService extends Disposable implements IChatService { } else if (isCompleteInteractiveProgressTreeData(progress)) { // This isn't exposed in API this.trace('sendRequest', `Provider returned tree data for session ${model.sessionId}, ${progress.treeData.label}`); + } else if ('documents' in progress) { + this.trace('sendRequest', `Provider returned documents for session ${model.sessionId}:\n ${JSON.stringify(progress.documents, null, '\t')}`); } else { this.trace('sendRequest', `Provider returned id for session ${model.sessionId}, ${progress.requestId}`); } diff --git a/src/vscode-dts/vscode.proposed.interactive.d.ts b/src/vscode-dts/vscode.proposed.interactive.d.ts index 174991827c746..5c176ddfad8dc 100644 --- a/src/vscode-dts/vscode.proposed.interactive.d.ts +++ b/src/vscode-dts/vscode.proposed.interactive.d.ts @@ -138,7 +138,24 @@ declare module 'vscode' { treeData: FileTreeData; } - export type InteractiveProgress = InteractiveProgressContent | InteractiveProgressId | InteractiveProgressTask | InteractiveProgressFileTree; + // FIXME@ulugbekna: my reservation with this type is that + // we already have a type called `TextDocumentContext` above passed to inline chat providers + export interface DocumentContext { + uri: Uri; + version: number; + ranges: Range[]; // @ulugbekna: we're making this an array of ranges rather than array of `DocumentContext`s, which should be less costly + } + + export interface InteractiveProgressUsedContext { + documents: DocumentContext[]; + } + + export type InteractiveProgress = + | InteractiveProgressContent + | InteractiveProgressId + | InteractiveProgressTask + | InteractiveProgressFileTree + | InteractiveProgressUsedContext; export interface InteractiveResponseCommand { commandId: string; From 4849a90efe395e564377e50dbbc6a7ddc300789f Mon Sep 17 00:00:00 2001 From: Ulugbek Abdullaev Date: Thu, 21 Sep 2023 11:59:36 +0200 Subject: [PATCH 3/7] mapped edits: change proposed API to take a context of array of arrays of documents --- src/vs/editor/common/languages.ts | 11 +++--- src/vs/monaco.d.ts | 9 ++--- .../workbench/api/common/extHost.protocol.ts | 8 ++--- .../api/common/extHostLanguageFeatures.ts | 11 ++++-- .../api/common/extHostTypeConverters.ts | 12 ++++--- .../browser/actions/chatCodeblockActions.ts | 35 +++++++++++++++---- .../vscode.proposed.mappedEditsProvider.d.ts | 14 +++----- 7 files changed, 63 insertions(+), 37 deletions(-) diff --git a/src/vs/editor/common/languages.ts b/src/vs/editor/common/languages.ts index 62c27c2d020f8..4f029b07dc6ae 100644 --- a/src/vs/editor/common/languages.ts +++ b/src/vs/editor/common/languages.ts @@ -16,7 +16,7 @@ import { URI, UriComponents } from 'vs/base/common/uri'; import { EditOperation, ISingleEditOperation } from 'vs/editor/common/core/editOperation'; import { IPosition, Position } from 'vs/editor/common/core/position'; import { IRange, Range } from 'vs/editor/common/core/range'; -import { ISelection, Selection } from 'vs/editor/common/core/selection'; +import { Selection } from 'vs/editor/common/core/selection'; import { LanguageId } from 'vs/editor/common/encodedTokenAttributes'; import * as model from 'vs/editor/common/model'; import { TokenizationRegistry as TokenizationRegistryImpl } from 'vs/editor/common/tokenizationRegistry'; @@ -2043,14 +2043,15 @@ export interface DocumentOnDropEditProvider { provideDocumentOnDropEdits(model: model.ITextModel, position: IPosition, dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): ProviderResult; } -export interface RelatedContextItem { +export interface DocumentContextItem { readonly uri: URI; - readonly range: IRange; + readonly version: number; + readonly ranges: IRange[]; } export interface MappedEditsContext { - selections: ISelection[]; - related: RelatedContextItem[]; + /** The outer array is sorted by priority - from highest to lowest. The inner arrays contain elements of the same priority. */ + documents: DocumentContextItem[][]; } export interface MappedEditsProvider { diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index ab0e838f80848..531d8448e84a7 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -7713,14 +7713,15 @@ declare namespace monaco.languages { provideDocumentRangeSemanticTokens(model: editor.ITextModel, range: Range, token: CancellationToken): ProviderResult; } - export interface RelatedContextItem { + export interface DocumentContextItem { readonly uri: Uri; - readonly range: IRange; + readonly version: number; + readonly ranges: IRange[]; } export interface MappedEditsContext { - selections: ISelection[]; - related: RelatedContextItem[]; + /** The outer array is sorted by priority - from highest to lowest. The inner arrays contain elements of the same priority. */ + documents: DocumentContextItem[][]; } export interface MappedEditsProvider { diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 289f9a81eafbb..53224b35c02d3 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -365,14 +365,14 @@ export interface IShareableItemDto { selection?: IRange; } -export interface IRelatedContextItemDto { +export interface IDocumentContextItemDto { readonly uri: UriComponents; - readonly range: IRange; + readonly version: number; + readonly ranges: IRange[]; } export interface IMappedEditsContextDto { - selections: ISelection[]; - related: IRelatedContextItemDto[]; + documents: IDocumentContextItemDto[][]; } export interface ISignatureHelpProviderMetadataDto { diff --git a/src/vs/workbench/api/common/extHostLanguageFeatures.ts b/src/vs/workbench/api/common/extHostLanguageFeatures.ts index bead4ad761166..3eca836582a0e 100644 --- a/src/vs/workbench/api/common/extHostLanguageFeatures.ts +++ b/src/vs/workbench/api/common/extHostLanguageFeatures.ts @@ -1831,8 +1831,15 @@ class MappedEditsAdapter { const doc = this._documents.getDocument(uri); const ctx = { - selections: context.selections.map(s => typeConvert.Selection.to(s)), - related: context.related.map(r => ({ uri: URI.revive(r.uri), range: typeConvert.Range.to(r.range) })), + documents: context.documents.map((docSubArray) => + docSubArray.map((r) => { + return { + uri: URI.revive(r.uri), + version: r.version, + ranges: r.ranges.map((range) => typeConvert.Range.to(range)), + }; + }) + ), }; const mappedEdits = await this._provider.provideMappedEdits(doc, codeBlocks, ctx, token); diff --git a/src/vs/workbench/api/common/extHostTypeConverters.ts b/src/vs/workbench/api/common/extHostTypeConverters.ts index 06f595fcb7b0b..b22d52ebead9b 100644 --- a/src/vs/workbench/api/common/extHostTypeConverters.ts +++ b/src/vs/workbench/api/common/extHostTypeConverters.ts @@ -1579,11 +1579,13 @@ export namespace MappedEditsContext { export function from(extContext: vscode.MappedEditsContext): languages.MappedEditsContext { return { - selections: extContext.selections.map(s => Selection.from(s)), - related: extContext.related.map(r => ({ - uri: URI.from(r.uri), - range: Range.from(r.range) - })) + documents: extContext.documents.map((subArray) => + subArray.map((r) => ({ + uri: URI.from(r.uri), + version: r.version, + ranges: r.ranges.map((r) => Range.from(r)), + })) + ), }; } } diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts index 8439a5d14fd6a..3c469bc3b3a8c 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts @@ -11,7 +11,7 @@ import { ServicesAccessor } from 'vs/editor/browser/editorExtensions'; import { IBulkEditService, ResourceTextEdit } from 'vs/editor/browser/services/bulkEditService'; import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService'; import { Range } from 'vs/editor/common/core/range'; -import { RelatedContextItem, WorkspaceEdit } from 'vs/editor/common/languages'; +import { DocumentContextItem, WorkspaceEdit } from 'vs/editor/common/languages'; import { ILanguageService } from 'vs/editor/common/languages/language'; import { ITextModel } from 'vs/editor/common/model'; import { ILanguageFeaturesService } from 'vs/editor/common/services/languageFeatures'; @@ -260,17 +260,38 @@ export function registerChatCodeBlockActions() { if (mappedEditsProviders.length > 0) { const mostRelevantProvider = mappedEditsProviders[0]; - const selections = codeEditor.getSelections() ?? []; - const mappedEditsContext = { - selections, - related: [] as RelatedContextItem[], // TODO@ulugbekna: we do have not yet decided what to populate this with - }; + // 0th sub-array - editor selections array if there are any selections + // 1st sub-array - array with documents used to get the chat reply + const docRefs: DocumentContextItem[][] = []; + + const model = codeEditor.getModel(); + if (model) { + const currentDocUri = model.uri; + const currentDocVersion = model.getVersionId(); + const selections = codeEditor.getSelections(); + if (selections && selections.length > 0) { + + docRefs.push([ + { + uri: currentDocUri, + version: currentDocVersion, + ranges: selections, + } + ]); + } + } + + const usedDocuments = chatCodeBlockActionContext.element.response.usedContext?.documents; + if (usedDocuments) { + docRefs.push(usedDocuments); + } + const cancellationTokenSource = new CancellationTokenSource(); workspaceEdit = await mostRelevantProvider.provideMappedEdits( activeModel, [chatCodeBlockActionContext.code], - mappedEditsContext, + { documents: docRefs }, cancellationTokenSource.token); } diff --git a/src/vscode-dts/vscode.proposed.mappedEditsProvider.d.ts b/src/vscode-dts/vscode.proposed.mappedEditsProvider.d.ts index 9d05e467fd90a..653f6a2d50d1f 100644 --- a/src/vscode-dts/vscode.proposed.mappedEditsProvider.d.ts +++ b/src/vscode-dts/vscode.proposed.mappedEditsProvider.d.ts @@ -5,20 +5,14 @@ declare module 'vscode' { - export interface RelatedContextItem { + export interface DocumentContextItem { readonly uri: Uri; - readonly range: Range; + readonly version: number; + readonly ranges: Range[]; } export interface MappedEditsContext { - selections: Selection[]; - - /** - * If there's no context, the array should be empty. It's also empty until we figure out how to compute this or retrieve from an extension (eg, copilot chat) - * - * TODO: it was suggested initially to be sorted from highest priority to lowest. How would it look like? - */ - related: RelatedContextItem[]; + documents: DocumentContextItem[][]; } /** From 63080adeb69f09667dbf64fb9c28767e9ad55f97 Mon Sep 17 00:00:00 2001 From: Ulugbek Abdullaev Date: Thu, 21 Sep 2023 16:29:44 +0200 Subject: [PATCH 4/7] mapped edits: restore backward compat --- .../api/common/extHostLanguageFeatures.ts | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/api/common/extHostLanguageFeatures.ts b/src/vs/workbench/api/common/extHostLanguageFeatures.ts index 3eca836582a0e..ddf2519bfd5b9 100644 --- a/src/vs/workbench/api/common/extHostLanguageFeatures.ts +++ b/src/vs/workbench/api/common/extHostLanguageFeatures.ts @@ -1830,16 +1830,19 @@ class MappedEditsAdapter { const uri = URI.revive(resource); const doc = this._documents.getDocument(uri); + const usedContext = context.documents.map((docSubArray) => + docSubArray.map((r) => { + return { + uri: URI.revive(r.uri), + version: r.version, + ranges: r.ranges.map((range) => typeConvert.Range.to(range)), + }; + }) + ); + const ctx = { - documents: context.documents.map((docSubArray) => - docSubArray.map((r) => { - return { - uri: URI.revive(r.uri), - version: r.version, - ranges: r.ranges.map((range) => typeConvert.Range.to(range)), - }; - }) - ), + documents: usedContext, + selections: usedContext[0]?.[0]?.ranges ?? [] // @ulugbekna: this is a hack for backward compatibility }; const mappedEdits = await this._provider.provideMappedEdits(doc, codeBlocks, ctx, token); From 384e191d8e3ef20570e2e209ba69dd7986afdd51 Mon Sep 17 00:00:00 2001 From: Ulugbek Abdullaev Date: Thu, 21 Sep 2023 19:13:49 +0200 Subject: [PATCH 5/7] mapped edits: remove comments --- src/vscode-dts/vscode.proposed.interactive.d.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/vscode-dts/vscode.proposed.interactive.d.ts b/src/vscode-dts/vscode.proposed.interactive.d.ts index 5c176ddfad8dc..b7185615ced9a 100644 --- a/src/vscode-dts/vscode.proposed.interactive.d.ts +++ b/src/vscode-dts/vscode.proposed.interactive.d.ts @@ -138,12 +138,10 @@ declare module 'vscode' { treeData: FileTreeData; } - // FIXME@ulugbekna: my reservation with this type is that - // we already have a type called `TextDocumentContext` above passed to inline chat providers export interface DocumentContext { uri: Uri; version: number; - ranges: Range[]; // @ulugbekna: we're making this an array of ranges rather than array of `DocumentContext`s, which should be less costly + ranges: Range[]; } export interface InteractiveProgressUsedContext { From 5547de51dbffcd423541d3328f9fdf5eabe4a65f Mon Sep 17 00:00:00 2001 From: Ulugbek Abdullaev Date: Fri, 22 Sep 2023 11:54:40 +0200 Subject: [PATCH 6/7] mapped edits: update code around the command `_executeMappedEditsProvider` --- .../singlefolder-tests/mappedEdits.test.ts | 45 ++++++++++--------- .../workbench/api/common/extHostCommands.ts | 8 ++-- .../api/common/extHostTypeConverters.ts | 22 +++++---- 3 files changed, 42 insertions(+), 33 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/mappedEdits.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/mappedEdits.test.ts index f4fc349f115e9..72413a2773fb3 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/mappedEdits.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/mappedEdits.test.ts @@ -18,13 +18,12 @@ suite('mapped edits provider', () => { const r1 = vscode.chat.registerMappedEditsProvider(tsDocFilter, { provideMappedEdits: (_doc: vscode.TextDocument, codeBlocks: string[], context: vscode.MappedEditsContext, _token: vscode.CancellationToken) => { - assert(context.selections.length === 1); - assert(context.related.length === 1); - assert('uri' in context.related[0] && 'range' in context.related[0]); + assert((context as any).selections.length === 1); // context.selections is for backward compat + assert(context.documents.length === 1); const edit = new vscode.WorkspaceEdit(); const text = codeBlocks.join('\n//----\n'); - edit.replace(uri, context.selections[0], text); + edit.replace(uri, context.documents[0][0].ranges[0], text); return edit; } }); @@ -37,12 +36,16 @@ suite('mapped edits provider', () => { `function foo() {\n\treturn 1;\n}`, ], { - selections: [new vscode.Selection(0, 0, 1, 0)], - related: [ - { - uri, - range: new vscode.Range(new vscode.Position(0, 0), new vscode.Position(1, 0)) - } + documents: [ + [ + { + uri, + version: 1, + ranges: [ + new vscode.Range(new vscode.Position(0, 0), new vscode.Position(1, 0)) + ] + } + ] ] } ); @@ -60,13 +63,9 @@ suite('mapped edits provider', () => { const r1 = vscode.chat.registerMappedEditsProvider(tsDocFilter, { provideMappedEdits: (_doc: vscode.TextDocument, codeBlocks: string[], context: vscode.MappedEditsContext, _token: vscode.CancellationToken) => { - assert(context.selections.length === 1); - assert(context.related.length === 1); - assert('uri' in context.related[0] && 'range' in context.related[0]); - const edit = new vscode.WorkspaceEdit(); const text = codeBlocks.join('\n//----\n'); - edit.replace(uri, context.selections[0], text); + edit.replace(uri, context.documents[0][0].ranges[0], text); return edit; } }); @@ -80,12 +79,16 @@ suite('mapped edits provider', () => { `function foo() {\n\treturn 1;\n}`, ], { - selections: [new vscode.Selection(0, 0, 1, 0)], - related: [ - { - uri, - range: new vscode.Range(new vscode.Position(0, 0), new vscode.Position(1, 0)) - } + documents: [ + [ + { + uri, + version: 1, + ranges: [ + new vscode.Range(new vscode.Position(0, 0), new vscode.Position(1, 0)) + ] + } + ] ] } ); diff --git a/src/vs/workbench/api/common/extHostCommands.ts b/src/vs/workbench/api/common/extHostCommands.ts index fd3ba6c52b922..beb10dcddac1a 100644 --- a/src/vs/workbench/api/common/extHostCommands.ts +++ b/src/vs/workbench/api/common/extHostCommands.ts @@ -123,7 +123,7 @@ export class ExtHostCommands implements ExtHostCommandsShape { const internalArgs = apiCommand.args.map((arg, i) => { if (!arg.validate(apiArgs[i])) { - throw new Error(`Invalid argument '${arg.name}' when running '${apiCommand.id}', received: ${apiArgs[i]}`); + throw new Error(`Invalid argument '${arg.name}' when running '${apiCommand.id}', received: ${typeof apiArgs[i] === 'object' ? JSON.stringify(apiArgs[i], null, '\t') : apiArgs[i]} `); } return arg.convert(apiArgs[i]); }); @@ -238,7 +238,7 @@ export class ExtHostCommands implements ExtHostCommandsShape { try { validateConstraint(args[i], description.args[i].constraint); } catch (err) { - throw new Error(`Running the contributed command: '${id}' failed. Illegal argument '${description.args[i].name}' - ${description.args[i].description}`); + throw new Error(`Running the contributed command: '${id}' failed.Illegal argument '${description.args[i].name}' - ${description.args[i].description} `); } } } @@ -342,7 +342,7 @@ export const IExtHostCommands = createDecorator('IExtHostComma export class CommandsConverter implements extHostTypeConverter.Command.ICommandsConverter { - readonly delegatingCommandId: string = `__vsc${Date.now().toString(36)}`; + readonly delegatingCommandId: string = `__vsc${Date.now().toString(36)} `; private readonly _cache = new Map(); private _cachIdPool = 0; @@ -387,7 +387,7 @@ export class CommandsConverter implements extHostTypeConverter.Command.ICommands // we have a contributed command with arguments. that // means we don't want to send the arguments around - const id = `${command.command}/${++this._cachIdPool}`; + const id = `${command.command} /${++this._cachIdPool}`; this._cache.set(id, command); disposables.add(toDisposable(() => { this._cache.delete(id); diff --git a/src/vs/workbench/api/common/extHostTypeConverters.ts b/src/vs/workbench/api/common/extHostTypeConverters.ts index b22d52ebead9b..9055980c1e4a7 100644 --- a/src/vs/workbench/api/common/extHostTypeConverters.ts +++ b/src/vs/workbench/api/common/extHostTypeConverters.ts @@ -1567,14 +1567,20 @@ export namespace LanguageSelector { export namespace MappedEditsContext { export function is(v: unknown): v is vscode.MappedEditsContext { - return (!!v && - typeof v === 'object' && - 'selections' in v && - Array.isArray(v.selections) && - v.selections.every(s => s instanceof types.Selection) && - 'related' in v && - Array.isArray(v.related) && - v.related.every(e => e && typeof e === 'object' && URI.isUri(e.uri) && e.range instanceof types.Range)); + return ( + !!v && typeof v === 'object' && + 'documents' in v && + Array.isArray(v.documents) && + v.documents.every(subArr => + Array.isArray(subArr) && + subArr.every(docRef => + docRef && typeof docRef === 'object' && + 'uri' in docRef && URI.isUri(docRef.uri) && + 'version' in docRef && typeof docRef.version === 'number' && + 'ranges' in docRef && Array.isArray(docRef.ranges) && docRef.ranges.every((r: unknown) => r instanceof types.Range) + ) + ) + ); } export function from(extContext: vscode.MappedEditsContext): languages.MappedEditsContext { From 6702a6627e5042f1889ef356019cf947ff2c8197 Mon Sep 17 00:00:00 2001 From: Ulugbek Abdullaev Date: Fri, 22 Sep 2023 14:57:04 +0200 Subject: [PATCH 7/7] refactor(mapped edits): clean up selection context creation --- .../contrib/chat/browser/actions/chatCodeblockActions.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts index 3c469bc3b3a8c..ddbc257facf12 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatCodeblockActions.ts @@ -264,13 +264,12 @@ export function registerChatCodeBlockActions() { // 1st sub-array - array with documents used to get the chat reply const docRefs: DocumentContextItem[][] = []; - const model = codeEditor.getModel(); - if (model) { + if (codeEditor.hasModel()) { + const model = codeEditor.getModel(); const currentDocUri = model.uri; const currentDocVersion = model.getVersionId(); const selections = codeEditor.getSelections(); - if (selections && selections.length > 0) { - + if (selections.length > 0) { docRefs.push([ { uri: currentDocUri,