From 6143ef555237ff2e5cc49ae4a6698cffef4dccfb Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 18 Mar 2022 11:12:52 -0700 Subject: [PATCH] comment model for editor should not be aware of notebook. --- .../api/browser/mainThreadComments.ts | 62 ++++++++++++------- .../workbench/api/common/extHost.protocol.ts | 1 - .../comments/browser/commentService.ts | 18 +++++- .../browser/commentsEditorContribution.ts | 40 ++++++------ .../contrib/comments/common/commentModel.ts | 39 +++++------- 5 files changed, 91 insertions(+), 69 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadComments.ts b/src/vs/workbench/api/browser/mainThreadComments.ts index 76ea96af373..6dbcd7fdc33 100644 --- a/src/vs/workbench/api/browser/mainThreadComments.ts +++ b/src/vs/workbench/api/browser/mainThreadComments.ts @@ -257,11 +257,19 @@ export class MainThreadCommentController { this._threads.set(commentThreadHandle, thread); - this._commentService.updateComments(this._uniqueId, { - added: [thread], - removed: [], - changed: [] - }); + if (thread.isDocumentCommentThread()) { + this._commentService.updateComments(this._uniqueId, { + added: [thread], + removed: [], + changed: [] + }); + } else { + this._commentService.updateNotebookComments(this._uniqueId, { + added: [thread as MainThreadCommentThread], + removed: [], + changed: [] + }); + } return thread; } @@ -273,22 +281,39 @@ export class MainThreadCommentController { let thread = this.getKnownThread(commentThreadHandle); thread.batchUpdate(changes); - this._commentService.updateComments(this._uniqueId, { - added: [], - removed: [], - changed: [thread] - }); + if (thread.isDocumentCommentThread()) { + this._commentService.updateComments(this._uniqueId, { + added: [], + removed: [], + changed: [thread] + }); + } else { + this._commentService.updateNotebookComments(this._uniqueId, { + added: [], + removed: [], + changed: [thread as MainThreadCommentThread] + }); + } + } deleteCommentThread(commentThreadHandle: number) { let thread = this.getKnownThread(commentThreadHandle); this._threads.delete(commentThreadHandle); - this._commentService.updateComments(this._uniqueId, { - added: [], - removed: [thread], - changed: [] - }); + if (thread.isDocumentCommentThread()) { + this._commentService.updateComments(this._uniqueId, { + added: [], + removed: [thread], + changed: [] + }); + } else { + this._commentService.updateNotebookComments(this._uniqueId, { + added: [], + removed: [thread as MainThreadCommentThread], + changed: [] + }); + } thread.dispose(); } @@ -626,13 +651,6 @@ export class MainThreadComments extends Disposable implements MainThreadComments } return this._handlers.get(handle)!; } - - $onDidCommentThreadsChange(handle: number, event: languages.CommentThreadChangedEvent) { - // notify comment service - const providerId = this.getHandler(handle); - this._commentService.updateComments(providerId, event); - } - override dispose(): void { super.dispose(); this._workspaceProviders.forEach(value => dispose(value)); diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 74e9add6a55..748c0dbf60b 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -132,7 +132,6 @@ export interface MainThreadCommentsShape extends IDisposable { $updateCommentThread(handle: number, commentThreadHandle: number, threadId: string, resource: UriComponents, changes: CommentThreadChanges): void; $deleteCommentThread(handle: number, commentThreadHandle: number): void; $updateCommentingRanges(handle: number): void; - $onDidCommentThreadsChange(handle: number, event: languages.CommentThreadChangedEvent): void; } export interface MainThreadAuthenticationShape extends IDisposable { diff --git a/src/vs/workbench/contrib/comments/browser/commentService.ts b/src/vs/workbench/contrib/comments/browser/commentService.ts index eca82718296..a11e4b8fb15 100644 --- a/src/vs/workbench/contrib/comments/browser/commentService.ts +++ b/src/vs/workbench/contrib/comments/browser/commentService.ts @@ -38,6 +38,10 @@ export interface IWorkspaceCommentThreadsEvent { commentThreads: CommentThread[]; } +export interface INotebookCommentThreadChangedEvent extends CommentThreadChangedEvent { + owner: string; +} + export interface ICommentController { id: string; features: { @@ -61,6 +65,7 @@ export interface ICommentService { readonly onDidSetResourceCommentInfos: Event; readonly onDidSetAllCommentThreads: Event; readonly onDidUpdateCommentThreads: Event; + readonly onDidUpdateNotebookCommentThreads: Event; readonly onDidChangeActiveCommentThread: Event; readonly onDidUpdateCommentingRanges: Event<{ owner: string }>; readonly onDidChangeActiveCommentingRange: Event<{ range: Range; commentingRangesInfo: CommentingRanges }>; @@ -75,7 +80,8 @@ export interface ICommentService { createCommentThreadTemplate(owner: string, resource: URI, range: Range): void; updateCommentThreadTemplate(owner: string, threadHandle: number, range: Range): Promise; getCommentMenus(owner: string): CommentMenus; - updateComments(ownerId: string, event: CommentThreadChangedEvent): void; + updateComments(ownerId: string, event: CommentThreadChangedEvent): void; + updateNotebookComments(ownerId: string, event: CommentThreadChangedEvent): void; disposeCommentThread(ownerId: string, threadId: string): void; getDocumentComments(resource: URI): Promise<(ICommentInfo | null)[]>; getNotebookComments(resource: URI): Promise<(INotebookCommentInfo | null)[]>; @@ -104,6 +110,9 @@ export class CommentService extends Disposable implements ICommentService { private readonly _onDidUpdateCommentThreads: Emitter = this._register(new Emitter()); readonly onDidUpdateCommentThreads: Event = this._onDidUpdateCommentThreads.event; + private readonly _onDidUpdateNotebookCommentThreads: Emitter = this._register(new Emitter()); + readonly onDidUpdateNotebookCommentThreads: Event = this._onDidUpdateNotebookCommentThreads.event; + private readonly _onDidUpdateCommentingRanges: Emitter<{ owner: string }> = this._register(new Emitter<{ owner: string }>()); readonly onDidUpdateCommentingRanges: Event<{ owner: string }> = this._onDidUpdateCommentingRanges.event; @@ -195,11 +204,16 @@ export class CommentService extends Disposable implements ICommentService { return menu; } - updateComments(ownerId: string, event: CommentThreadChangedEvent): void { + updateComments(ownerId: string, event: CommentThreadChangedEvent): void { const evt: ICommentThreadChangedEvent = Object.assign({}, event, { owner: ownerId }); this._onDidUpdateCommentThreads.fire(evt); } + updateNotebookComments(ownerId: string, event: CommentThreadChangedEvent): void { + const evt: INotebookCommentThreadChangedEvent = Object.assign({}, event, { owner: ownerId }); + this._onDidUpdateNotebookCommentThreads.fire(evt); + } + updateCommentingRanges(ownerId: string) { this._onDidUpdateCommentingRanges.fire({ owner: ownerId }); } diff --git a/src/vs/workbench/contrib/comments/browser/commentsEditorContribution.ts b/src/vs/workbench/contrib/comments/browser/commentsEditorContribution.ts index 7c6e0a160ef..a172f418ba0 100644 --- a/src/vs/workbench/contrib/comments/browser/commentsEditorContribution.ts +++ b/src/vs/workbench/contrib/comments/browser/commentsEditorContribution.ts @@ -515,32 +515,28 @@ export class CommentController implements IEditorContribution { }); changed.forEach(thread => { - if (thread.isDocumentCommentThread()) { - let matchedZones = this._commentWidgets.filter(zoneWidget => zoneWidget.owner === e.owner && zoneWidget.commentThread.threadId === thread.threadId); - if (matchedZones.length) { - let matchedZone = matchedZones[0]; - matchedZone.update(thread); - } + let matchedZones = this._commentWidgets.filter(zoneWidget => zoneWidget.owner === e.owner && zoneWidget.commentThread.threadId === thread.threadId); + if (matchedZones.length) { + let matchedZone = matchedZones[0]; + matchedZone.update(thread); } }); added.forEach(thread => { - if (thread.isDocumentCommentThread()) { - let matchedZones = this._commentWidgets.filter(zoneWidget => zoneWidget.owner === e.owner && zoneWidget.commentThread.threadId === thread.threadId); - if (matchedZones.length) { - return; - } - - let matchedNewCommentThreadZones = this._commentWidgets.filter(zoneWidget => zoneWidget.owner === e.owner && (zoneWidget.commentThread as any).commentThreadHandle === -1 && Range.equalsRange(zoneWidget.commentThread.range, thread.range)); - - if (matchedNewCommentThreadZones.length) { - matchedNewCommentThreadZones[0].update(thread); - return; - } - - const pendingCommentText = this._pendingCommentCache[e.owner] && this._pendingCommentCache[e.owner][thread.threadId!]; - this.displayCommentThread(e.owner, thread, pendingCommentText); - this._commentInfos.filter(info => info.owner === e.owner)[0].threads.push(thread); + let matchedZones = this._commentWidgets.filter(zoneWidget => zoneWidget.owner === e.owner && zoneWidget.commentThread.threadId === thread.threadId); + if (matchedZones.length) { + return; } + + let matchedNewCommentThreadZones = this._commentWidgets.filter(zoneWidget => zoneWidget.owner === e.owner && (zoneWidget.commentThread as any).commentThreadHandle === -1 && Range.equalsRange(zoneWidget.commentThread.range, thread.range)); + + if (matchedNewCommentThreadZones.length) { + matchedNewCommentThreadZones[0].update(thread); + return; + } + + const pendingCommentText = this._pendingCommentCache[e.owner] && this._pendingCommentCache[e.owner][thread.threadId!]; + this.displayCommentThread(e.owner, thread, pendingCommentText); + this._commentInfos.filter(info => info.owner === e.owner)[0].threads.push(thread); }); })); diff --git a/src/vs/workbench/contrib/comments/common/commentModel.ts b/src/vs/workbench/contrib/comments/common/commentModel.ts index b414fd9c8a4..d8f97430b4b 100644 --- a/src/vs/workbench/contrib/comments/common/commentModel.ts +++ b/src/vs/workbench/contrib/comments/common/commentModel.ts @@ -8,9 +8,8 @@ import { IRange } from 'vs/editor/common/core/range'; import { Comment, CommentThread, CommentThreadChangedEvent } from 'vs/editor/common/languages'; import { groupBy } from 'vs/base/common/arrays'; import { localize } from 'vs/nls'; -import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange'; -export interface ICommentThreadChangedEvent extends CommentThreadChangedEvent { +export interface ICommentThreadChangedEvent extends CommentThreadChangedEvent { owner: string; } @@ -105,32 +104,28 @@ export class CommentsModel { }); changed.forEach(thread => { - if (thread.isDocumentCommentThread()) { - // Find resource that has the comment thread - const matchingResourceIndex = threadsForOwner.findIndex((resourceData) => resourceData.id === thread.resource); - const matchingResourceData = threadsForOwner[matchingResourceIndex]; + // Find resource that has the comment thread + const matchingResourceIndex = threadsForOwner.findIndex((resourceData) => resourceData.id === thread.resource); + const matchingResourceData = threadsForOwner[matchingResourceIndex]; - // Find comment node on resource that is that thread and replace it - const index = matchingResourceData.commentThreads.findIndex((commentThread) => commentThread.threadId === thread.threadId); - if (index >= 0) { - matchingResourceData.commentThreads[index] = ResourceWithCommentThreads.createCommentNode(owner, URI.parse(matchingResourceData.id), thread); - } else if (thread.comments && thread.comments.length) { - matchingResourceData.commentThreads.push(ResourceWithCommentThreads.createCommentNode(owner, URI.parse(matchingResourceData.id), thread)); - } + // Find comment node on resource that is that thread and replace it + const index = matchingResourceData.commentThreads.findIndex((commentThread) => commentThread.threadId === thread.threadId); + if (index >= 0) { + matchingResourceData.commentThreads[index] = ResourceWithCommentThreads.createCommentNode(owner, URI.parse(matchingResourceData.id), thread); + } else if (thread.comments && thread.comments.length) { + matchingResourceData.commentThreads.push(ResourceWithCommentThreads.createCommentNode(owner, URI.parse(matchingResourceData.id), thread)); } }); added.forEach(thread => { - if (thread.isDocumentCommentThread()) { - const existingResource = threadsForOwner.filter(resourceWithThreads => resourceWithThreads.resource.toString() === thread.resource); - if (existingResource.length) { - const resource = existingResource[0]; - if (thread.comments && thread.comments.length) { - resource.commentThreads.push(ResourceWithCommentThreads.createCommentNode(owner, resource.resource, thread)); - } - } else { - threadsForOwner.push(new ResourceWithCommentThreads(owner, URI.parse(thread.resource!), [thread])); + const existingResource = threadsForOwner.filter(resourceWithThreads => resourceWithThreads.resource.toString() === thread.resource); + if (existingResource.length) { + const resource = existingResource[0]; + if (thread.comments && thread.comments.length) { + resource.commentThreads.push(ResourceWithCommentThreads.createCommentNode(owner, resource.resource, thread)); } + } else { + threadsForOwner.push(new ResourceWithCommentThreads(owner, URI.parse(thread.resource!), [thread])); } });