Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File level comments API proposal #177342

Merged
merged 1 commit into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/vs/editor/common/languages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1593,7 +1593,7 @@ export interface CommentThread<T = IRange> {
extensionId?: string;
threadId: string;
resource: string | null;
range: T;
range: T | undefined;
label: string | undefined;
contextValue: string | undefined;
comments: Comment[] | undefined;
Expand All @@ -1605,7 +1605,7 @@ export interface CommentThread<T = IRange> {
canReply: boolean;
input?: CommentInput;
onDidChangeInput: Event<CommentInput | undefined>;
onDidChangeRange: Event<T>;
onDidChangeRange: Event<T | undefined>;
onDidChangeLabel: Event<string | undefined>;
onDidChangeCollapsibleState: Event<CommentThreadCollapsibleState | undefined>;
onDidChangeState: Event<CommentThreadState | undefined>;
Expand Down
18 changes: 9 additions & 9 deletions src/vs/workbench/api/browser/mainThreadComments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import * as languages from 'vs/editor/common/languages';
import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
import { Registry } from 'vs/platform/registry/common/platform';
import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers';
import { ICommentInfo, ICommentService, INotebookCommentInfo } from 'vs/workbench/contrib/comments/browser/commentService';
import { ICommentController, ICommentInfo, ICommentService, INotebookCommentInfo } from 'vs/workbench/contrib/comments/browser/commentService';
import { CommentsPanel } from 'vs/workbench/contrib/comments/browser/commentsView';
import { CommentProviderFeatures, ExtHostCommentsShape, ExtHostContext, MainContext, MainThreadCommentsShape, CommentThreadChanges } from '../common/extHost.protocol';
import { COMMENTS_VIEW_ID, COMMENTS_VIEW_STORAGE_ID, COMMENTS_VIEW_TITLE } from 'vs/workbench/contrib/comments/browser/commentsTreeViewer';
Expand Down Expand Up @@ -80,12 +80,12 @@ export class MainThreadCommentThread<T> implements languages.CommentThread<T> {
private readonly _onDidChangeComments = new Emitter<readonly languages.Comment[] | undefined>();
get onDidChangeComments(): Event<readonly languages.Comment[] | undefined> { return this._onDidChangeComments.event; }

set range(range: T) {
set range(range: T | undefined) {
this._range = range;
this._onDidChangeRange.fire(this._range);
}

get range(): T {
get range(): T | undefined {
return this._range;
}

Expand All @@ -100,7 +100,7 @@ export class MainThreadCommentThread<T> implements languages.CommentThread<T> {
return this._canReply;
}

private readonly _onDidChangeRange = new Emitter<T>();
private readonly _onDidChangeRange = new Emitter<T | undefined>();
public onDidChangeRange = this._onDidChangeRange.event;

private _collapsibleState: languages.CommentThreadCollapsibleState | undefined;
Expand Down Expand Up @@ -138,7 +138,7 @@ export class MainThreadCommentThread<T> implements languages.CommentThread<T> {
}

isDocumentCommentThread(): this is languages.CommentThread<IRange> {
return Range.isIRange(this._range);
return this._range === undefined || Range.isIRange(this._range);
}

private _state: languages.CommentThreadState | undefined;
Expand All @@ -164,7 +164,7 @@ export class MainThreadCommentThread<T> implements languages.CommentThread<T> {
public extensionId: string,
public threadId: string,
public resource: string,
private _range: T,
private _range: T | undefined,
private _canReply: boolean,
private _isTemplate: boolean
) {
Expand Down Expand Up @@ -207,7 +207,7 @@ export class MainThreadCommentThread<T> implements languages.CommentThread<T> {
}
}

export class MainThreadCommentController {
export class MainThreadCommentController implements ICommentController {
get handle(): number {
return this._handle;
}
Expand Down Expand Up @@ -267,7 +267,7 @@ export class MainThreadCommentController {
commentThreadHandle: number,
threadId: string,
resource: UriComponents,
range: IRange | ICellRange,
range: IRange | ICellRange | undefined,
isTemplate: boolean
): languages.CommentThread<IRange | ICellRange> {
const thread = new MainThreadCommentThread(
Expand Down Expand Up @@ -547,7 +547,7 @@ export class MainThreadComments extends Disposable implements MainThreadComments
commentThreadHandle: number,
threadId: string,
resource: UriComponents,
range: IRange | ICellRange,
range: IRange | ICellRange | undefined,
extensionId: ExtensionIdentifier,
isTemplate: boolean
): languages.CommentThread<IRange | ICellRange> | undefined {
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/api/common/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export interface CommentChanges {
}

export type CommentThreadChanges<T = IRange> = Partial<{
range: T;
range: T | undefined;
label: string;
contextValue: string | null;
comments: CommentChanges[];
Expand All @@ -138,7 +138,7 @@ export interface MainThreadCommentsShape extends IDisposable {
$registerCommentController(handle: number, id: string, label: string): void;
$unregisterCommentController(handle: number): void;
$updateCommentControllerFeatures(handle: number, features: CommentProviderFeatures): void;
$createCommentThread(handle: number, commentThreadHandle: number, threadId: string, resource: UriComponents, range: IRange | ICellRange, extensionId: ExtensionIdentifier, isTemplate: boolean): languages.CommentThread<IRange | ICellRange> | undefined;
$createCommentThread(handle: number, commentThreadHandle: number, threadId: string, resource: UriComponents, range: IRange | ICellRange | undefined, extensionId: ExtensionIdentifier, isTemplate: boolean): languages.CommentThread<IRange | ICellRange> | undefined;
$updateCommentThread(handle: number, commentThreadHandle: number, threadId: string, resource: UriComponents, changes: CommentThreadChanges): void;
$deleteCommentThread(handle: number, commentThreadHandle: number): void;
$updateCommentingRanges(handle: number): void;
Expand Down
35 changes: 16 additions & 19 deletions src/vs/workbench/api/common/extHostComments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import * as types from 'vs/workbench/api/common/extHostTypes';
import type * as vscode from 'vscode';
import { ExtHostCommentsShape, IMainContext, MainContext, CommentThreadChanges, CommentChanges } from './extHost.protocol';
import { ExtHostCommands } from './extHostCommands';
import { checkProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions';

type ProviderHandle = number;

Expand Down Expand Up @@ -233,7 +234,7 @@ export function createExtHostComments(mainContext: IMainContext, commands: ExtHo
isTemplate: boolean;
}>;

class ExtHostCommentThread implements vscode.CommentThread {
class ExtHostCommentThread implements vscode.CommentThread2 {
private static _handlePool: number = 0;
readonly handle = ExtHostCommentThread._handlePool++;
public commentHandle: number = 0;
Expand Down Expand Up @@ -263,15 +264,15 @@ export function createExtHostComments(mainContext: IMainContext, commands: ExtHo
private readonly _onDidUpdateCommentThread = new Emitter<void>();
readonly onDidUpdateCommentThread = this._onDidUpdateCommentThread.event;

set range(range: vscode.Range) {
if (!range.isEqual(this._range)) {
set range(range: vscode.Range | undefined) {
if (((range === undefined) !== (this._range === undefined)) || (!range || !this._range || !range.isEqual(this._range))) {
this._range = range;
this.modifications.range = range;
this._onDidUpdateCommentThread.fire();
}
}

get range(): vscode.Range {
get range(): vscode.Range | undefined {
return this._range;
}

Expand Down Expand Up @@ -358,14 +359,14 @@ export function createExtHostComments(mainContext: IMainContext, commands: ExtHo

private _acceptInputDisposables = new MutableDisposable<DisposableStore>();

readonly value: vscode.CommentThread;
readonly value: vscode.CommentThread2;

constructor(
commentControllerId: string,
private _commentControllerHandle: number,
private _id: string | undefined,
private _uri: vscode.Uri,
private _range: vscode.Range,
private _range: vscode.Range | undefined,
private _comments: vscode.Comment[],
public readonly extensionDescription: IExtensionDescription,
private _isTemplate: boolean
Expand Down Expand Up @@ -409,7 +410,7 @@ export function createExtHostComments(mainContext: IMainContext, commands: ExtHo
this.value = {
get uri() { return that.uri; },
get range() { return that.range; },
set range(value: vscode.Range) { that.range = value; },
set range(value: vscode.Range | undefined) { that.range = value; },
get comments() { return that.comments; },
set comments(value: vscode.Comment[]) { that.comments = value; },
get collapsibleState() { return that.collapsibleState; },
Expand Down Expand Up @@ -582,11 +583,11 @@ export function createExtHostComments(mainContext: IMainContext, commands: ExtHo
set commentingRangeProvider(commentingRangeProvider: vscode.CommentingRangeProvider | undefined) { that.commentingRangeProvider = commentingRangeProvider; },
get reactionHandler(): ReactionHandler | undefined { return that.reactionHandler; },
set reactionHandler(handler: ReactionHandler | undefined) { that.reactionHandler = handler; },
createCommentThread(uri: vscode.Uri, range: vscode.Range, comments: vscode.Comment[]): vscode.CommentThread {
createCommentThread(uri: vscode.Uri, range: vscode.Range | undefined, comments: vscode.Comment[]): vscode.CommentThread | vscode.CommentThread2 {
return that.createCommentThread(uri, range, comments).value;
},
dispose: () => { that.dispose(); },
});
}) as any; // TODO @alexr00 remove this cast when the proposed API is stable

this._localDisposables = [];
this._localDisposables.push({
Expand All @@ -596,17 +597,13 @@ export function createExtHostComments(mainContext: IMainContext, commands: ExtHo
});
}

createCommentThread(resource: vscode.Uri, range: vscode.Range, comments: vscode.Comment[]): ExtHostCommentThread;
createCommentThread(arg0: vscode.Uri | string, arg1: vscode.Uri | vscode.Range, arg2: vscode.Range | vscode.Comment[], arg3?: vscode.Comment[]): vscode.CommentThread {
if (typeof arg0 === 'string') {
const commentThread = new ExtHostCommentThread(this.id, this.handle, arg0, arg1 as vscode.Uri, arg2 as vscode.Range, arg3 as vscode.Comment[], this._extension, false);
this._threads.set(commentThread.handle, commentThread);
return commentThread;
} else {
const commentThread = new ExtHostCommentThread(this.id, this.handle, undefined, arg0 as vscode.Uri, arg1 as vscode.Range, arg2 as vscode.Comment[], this._extension, false);
this._threads.set(commentThread.handle, commentThread);
return commentThread;
createCommentThread(resource: vscode.Uri, range: vscode.Range | undefined, comments: vscode.Comment[]): ExtHostCommentThread {
if (range === undefined) {
checkProposedApiEnabled(this._extension, 'fileComments');
}
const commentThread = new ExtHostCommentThread(this.id, this.handle, undefined, resource, range, comments, this._extension, false);
this._threads.set(commentThread.handle, commentThread);
return commentThread;
}

$createCommentThreadTemplate(uriComponents: UriComponents, range: IRange): ExtHostCommentThread {
Expand Down
11 changes: 8 additions & 3 deletions src/vs/workbench/contrib/comments/browser/commentThreadBody.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,14 @@ export class CommentThreadBody<T extends IRange | ICellRange = IRange> extends D

private _updateAriaLabel() {
if (this._commentThread.isDocumentCommentThread()) {
this._commentsElement.ariaLabel = nls.localize('commentThreadAria.withRange', "Comment thread with {0} comments on lines {1} through {2}. {3}.",
this._commentThread.comments?.length, this._commentThread.range.startLineNumber, this._commentThread.range.endLineNumber,
this._commentThread.label);
if (this._commentThread.range) {
this._commentsElement.ariaLabel = nls.localize('commentThreadAria.withRange', "Comment thread with {0} comments on lines {1} through {2}. {3}.",
this._commentThread.comments?.length, this._commentThread.range.startLineNumber, this._commentThread.range.endLineNumber,
this._commentThread.label);
} else {
this._commentsElement.ariaLabel = nls.localize('commentThreadAria.document', "Comment thread with {0} comments on the entire document. {1}.",
this._commentThread.comments?.length, this._commentThread.label);
}
} else {
this._commentsElement.ariaLabel = nls.localize('commentThreadAria', "Comment thread with {0} comments. {1}.",
this._commentThread.comments?.length, this._commentThread.label);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export class CommentThreadRangeDecorator extends Disposable {
const newDecoration: CommentThreadRangeDecoration[] = [];
if (thread) {
const range = thread.range;
if (!((range.startLineNumber === range.endLineNumber) && (range.startColumn === range.endColumn))) {
if (range && !((range.startLineNumber === range.endLineNumber) && (range.startColumn === range.endColumn))) {
if (thread.collapsibleState === CommentThreadCollapsibleState.Expanded) {
this.currentThreadCollapseStateListener = thread.onDidChangeCollapsibleState(state => {
if (state === CommentThreadCollapsibleState.Collapsed) {
Expand Down Expand Up @@ -108,7 +108,7 @@ export class CommentThreadRangeDecorator extends Disposable {
const range = thread.range;
// We only want to show a range decoration when there's the range spans either multiple lines
// or, when is spans multiple characters on the sample line
if ((range.startLineNumber === range.endLineNumber) && (range.startColumn === range.endColumn)) {
if (!range || (range.startLineNumber === range.endLineNumber) && (range.startColumn === range.endColumn)) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,18 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
const height = this.editor.getLayoutInfo().height;
const coords = this._commentThreadWidget.getCommentCoords(commentUniqueId);
if (coords) {
const commentThreadCoords = coords.thread;
const commentCoords = coords.comment;

this.editor.setScrollTop(this.editor.getTopForLineNumber(this._commentThread.range.startLineNumber) - height / 2 + commentCoords.top - commentThreadCoords.top);
let scrollTop: number = 1;
if (this._commentThread.range) {
const commentThreadCoords = coords.thread;
const commentCoords = coords.comment;
scrollTop = this.editor.getTopForLineNumber(this._commentThread.range.startLineNumber) - height / 2 + commentCoords.top - commentThreadCoords.top;
}
this.editor.setScrollTop(scrollTop);
return;
}
}

this.editor.revealRangeInCenter(this._commentThread.range);
this.editor.revealRangeInCenter(this._commentThread.range ?? new Range(1, 1, 1, 1));
if (focus) {
this._commentThreadWidget.focus();
}
Expand Down Expand Up @@ -228,12 +231,16 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
const newPosition = this.getPosition();

if (newPosition) {
let range: Range;
const originalRange = this._commentThread.range;
if (!originalRange) {
return;
}
let range: Range;

if (newPosition.lineNumber !== originalRange.endLineNumber) {
// The widget could have moved as a result of editor changes.
// We need to try to calculate the new, more correct, range for the comment.
const distance = newPosition.lineNumber - this._commentThread.range.endLineNumber;
const distance = newPosition.lineNumber - originalRange.endLineNumber;
range = new Range(originalRange.startLineNumber + distance, originalRange.startColumn, originalRange.endLineNumber + distance, originalRange.endColumn);
} else {
range = new Range(originalRange.startLineNumber, originalRange.startColumn, originalRange.endLineNumber, originalRange.endColumn);
Expand All @@ -251,7 +258,10 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
this._disposables.add(this._commentThreadWidget);
}

private arrowPosition(range: IRange): IPosition {
private arrowPosition(range: IRange | undefined): IPosition | undefined {
if (!range) {
return undefined;
}
// Arrow on top edge of zone widget will be at the start of the line if range is multi-line, else at midpoint of range (rounding rightwards)
return { lineNumber: range.endLineNumber, column: range.endLineNumber === range.startLineNumber ? (range.startColumn + range.endColumn + 1) / 2 : 1 };
}
Expand Down Expand Up @@ -295,7 +305,7 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
this._commentThreadWidget.updateCommentThread(commentThread);

// Move comment glyph widget and show position if the line has changed.
const lineNumber = this._commentThread.range.endLineNumber;
const lineNumber = this._commentThread.range?.endLineNumber ?? 1;
let shouldMoveWidget = false;
if (this._commentGlyph) {
this._commentGlyph.setThreadState(commentThread.state);
Expand Down Expand Up @@ -324,15 +334,17 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
this._commentThreadWidget.layout(widthInPixel);
}

display(range: IRange) {
this._commentGlyph = new CommentGlyphWidget(this.editor, range.endLineNumber);
this._commentGlyph.setThreadState(this._commentThread.state);
display(range: IRange | undefined) {
if (range) {
this._commentGlyph = new CommentGlyphWidget(this.editor, range?.endLineNumber ?? -1);
this._commentGlyph.setThreadState(this._commentThread.state);
}

this._commentThreadWidget.display(this.editor.getOption(EditorOption.lineHeight));
this._disposables.add(this._commentThreadWidget.onDidResize(dimension => {
this._refresh(dimension);
}));
if (this._commentThread.collapsibleState === languages.CommentThreadCollapsibleState.Expanded) {
if ((this._commentThread.collapsibleState === languages.CommentThreadCollapsibleState.Expanded) || (range === undefined)) {
this.show(this.arrowPosition(range), 2);
}

Expand All @@ -351,7 +363,7 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget

this._commentThreadDisposables.push(this._commentThread.onDidChangeRange(range => {
// Move comment glyph widget and show position if the line has changed.
const lineNumber = this._commentThread.range.startLineNumber;
const lineNumber = this._commentThread.range?.startLineNumber ?? 1;
let shouldMoveWidget = false;
if (this._commentGlyph) {
if (this._commentGlyph.getPosition().position!.lineNumber !== lineNumber) {
Expand Down Expand Up @@ -379,8 +391,9 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget

if (this._initialCollapsibleState === undefined) {
const onDidChangeInitialCollapsibleState = this._commentThread.onDidChangeInitialCollapsibleState(state => {
this._initialCollapsibleState = state;
this._commentThread.collapsibleState = state;
// File comments always start expanded
this._initialCollapsibleState = this._commentThread.range ? state : languages.CommentThreadCollapsibleState.Expanded;
this._commentThread.collapsibleState = this._initialCollapsibleState;
onDidChangeInitialCollapsibleState.dispose();
});
this._commentThreadDisposables.push(onDidChangeInitialCollapsibleState);
Expand Down Expand Up @@ -420,7 +433,7 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget

const currentPosition = this.getPosition();

if (this._viewZone && currentPosition && currentPosition.lineNumber !== this._viewZone.afterLineNumber) {
if (this._viewZone && currentPosition && currentPosition.lineNumber !== this._viewZone.afterLineNumber && this._viewZone.afterLineNumber !== 0) {
this._viewZone.afterLineNumber = currentPosition.lineNumber;
}

Expand All @@ -444,9 +457,9 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
this._commentThreadWidget.applyTheme(theme, fontInfo);
}

override show(rangeOrPos: IRange | IPosition, heightInLines: number): void {
override show(rangeOrPos: IRange | IPosition | undefined, heightInLines: number): void {
this._isExpanded = true;
super.show(rangeOrPos, heightInLines);
super.show(rangeOrPos ?? new Range(0, 0, 0, 0), heightInLines);
this._commentThread.collapsibleState = languages.CommentThreadCollapsibleState.Expanded;
this._refresh(this._commentThreadWidget.getDimensions());
}
Expand Down
Loading