CommentProvider draft api proposals

[rebornix]

TLDR

Read Proposals

---

We have been discussing how we can make the commenting api generic enough but also have the flexibility of rendering. With this in mind, we proposed the first version of the commenting api, which includes two types of CommentProvider

interface DocumentCommentProvider {
	provideDocumentComments(document: TextDocument, token: CancellationToken): Promise<CommentInfo>;
	onDidChangeCommentThreads: Event<CommentThreadChangedEvent>;
}

interface WorkspaceCommentProvider {
	provideWorkspaceComments(token: CancellationToken): Promise<CommentThread[]>;
	onDidChangeCommentThreads: Event<CommentThreadChangedEvent>;
}

DocumentCommentProvider is invoked when a file is opened in an editor while WorkspaceCommentProvider is used to feed the Comments Panel. Both of these two providers will provide CommentThread to the core

interface CommentThread {
	threadId: string;
	resource: uri;
	range: range;
	comments: comment[];
	collapsibleState?: commentThreadCollapsibleState;
}

CommentThread is simply an abstraction of how comments are being rendered in the editor, we can see it as a flat list of Comment created at the same position in a particular document. It has nothing to do with how comments are being grouped into Discussion or Review or whatever, the underlying business logic is up to extensions.

Patterns

So far so good. Besides the API looks pretty close to our language feature apis, and it's easy to understand and adopt

export function registerCompletionItemProvider(selector: DocumentSelector, provider: CompletionItemProvider, ...triggerCharacters: string[]): Disposable;

export interface CompletionItemProvider {
	provideCompletionItems(document: TextDocument, position: Position, token: CancellationToken, context: CompletionContext): ProviderResult<CompletionItem[] | CompletionList>;
	resolveCompletionItem?(item: CompletionItem, token: CancellationToken): ProviderResult<CompletionItem>;
}

However, displaying comments in the comments panel and editor is just a start, we need to allow users to create new comments and reply to comments, and furthermore, users should be able to edit and delete comments.

We went back and forth on supporting comments creation and reply. The UI part is kind of intuitive, it consists of a textarea and a button

But how can extensions access the content in the textarea? What text should we render on the button, "Add Comment" or "Start Review"? The first idea came to our mind is using Command, it's a commonly used pattern in our extension API.

export class TreeItem {
...
	/**
	 * The [command](#Command) that should be executed when the tree item is selected.
	 */
	command?: Command;
...
}

export class CodeLens {
...
	/**
	 * The command this code lens represents.
	 */
	command?: Command;
...
}

export class CodeAction {
...
	/**
	 * The command this code lens represents.
	 */
	command?: Command;
...
}

We are wondering if we can have a command property on every CommentThread which implements Reply logic and a command for the CommentProvider which implements Create Comment. But it turns out we have more and more open questions.

One good example is, as an extension author, when you attempt to implement the Reply command, how do you know what kind of parameters VSCode will pass in, in what sequence? For comment reply, we only need the related comment thread, the text in the textarea. While for comment creation, we need document uri, the position or range of the newly created comment and content in textarea. Even though they are all Command but they are in differnet shapes. Instead of digging into this pattern too much, we introduced declractive APIs like below

interface DocumentCommentProvider {
...
	createNewCommentThread(document: TextDcument, range: Range, text: string, token: CancellationToken): Promise<CommentThread>;
	replyToCommentThread(document: TextDocument, range: Range, commentThread: CommentThread, text: string, token: CancellationToken): Promise<CommentThread>;
...
}

We are already using this group of APIs in GitHub Pull Request extension and enriched it with EditComment and DeleteComment later. But now we have four kinds of buttons/actions on the UI

• Create
• Reply
• Update
• Delete

and we provide expeclit API for them. However when we started design of commenting grouping/review/drafting concept, we found ourselves have to introduce new buttons or action items to the Comment Widget and it's a probably a good time to revisit our API design.

After digging into our existing APIs, we found SourceControl is in the same situation as ours, different source contorl providers have different wording, resource grouping and commands, they need to access the input box in the SCM viewlet. It would be interesting to see if we can similar design as SCM API.

Proposals

A

The first proposal is we still follow our existing APIs and introduce a set of new APIs to start/end draft mode

interface DocumentCommentProvider {
...
	startDraft(token: CancellationToken): Promise<void>;
	deleteDraft(token: CancellationToken): Promise<void>;
	finishDraft(token: CancellationToken): Promise<void>;
...
}

After users click Start Draft button on the UI, we invoke startDraft API and all comments being created will be marked as pending or draft. The flag or type is added to Comment

interface Comment {
	commentId: string;
	body: MarkdownString;
	userName: string;
	userIconPath?: Uri;
	gravatar?: string;
	canEdit?: boolean;
	canDelete?: boolean;

	/*
	 * is draft
	 */
	isDraft: boolean;
}

VSCode core switch the draft mode (display Finish Draft, or Delete Draft button), once it finds that there is a comment being marked as isDraft: true.

B

The second proposal is we follow the design of Source Control Provider. To enable commenting support, firstly extensions need to create a CommentProvider

export function createCommentProvider(id: string, label: string, rootUri?: Uri): CommentProvider;

With rootUri we can have sound support for multi root workspace. The CommentProvider would have following properties

interface CommentProvider {
	readonly id: string;
	readonly label: string;
	readonly rootUri: Uri | undefined;

	/**
	 * The active (focused) comment widget.
	 */
	readonly widget: CommentWidget;

	/**
	 * Optional comment template string.
	 */
	commentTemplate?: string;

	/**
	 * Accept input commands.
	 *
	 * The first command will be invoked when the user accepts the value
	 * in the comment widget textarea.
	 */
	acceptInputCommands: Command[];


	/*
	 * Command for updating a comment.
	 */
	updateCommand?: Command;

	/**
	 * Create a new comment thread
	 */
	createCommentThread(id: string, resource: Uri, range: Range): CommentThread;


	/**
	 * Create a new commenting range
	 */
	createCommentingRange(resource: Uri, range: Range): CommentingRange;

	/**
	 * Dispose this comment provider.
	 */
	dispose(): void;
}

We will render a dropdown list in the comment widget for all acceptInputCommands. For example, an extension can register Create Comment and Start Conversation commands, and when the command is being invoked, we will pass in CommentProvider, and then the extension can read the commentWidget property to access the active/focused comment widget.

interface CommentWidget {
	/*
	 * Comment thread in this Comment Widget
	 */
	commentThread: ICommentThread;

	/*
	 * Focused Comment
	 * This comment must be part of CommentWidget.commentThread
	 */
	comment?: Commnet;

	/*
	 * Textarea content in the comment widget.
	 * There is only one active input box in a comment widget.
	 */
	input: string;
}

The active comment widget knows about the current comment thread, the focused comment and input of the textarea in the widget.

To reply to a comment, the commentThead is a valid one which has an id, a range, resource uri, etc. However, if you are going to create a new comment thread, commentThread.id is null as it doesn't exist yet.

Now the core doesn't need to control the draft/review/conversation mode as the state can be fully managed by the extension. For example, user choose Start Conversation command from the dropdown list, and then the extension can update CommentProvider.acceptInputCommands and provide three other commands

• Add to conversation
• Delete conversation
• Submit conversation

The dropdown list on Comment Widget will be updated accordingly.

[rebornix]

Based on our discussion, we will continue with proposal A first for now, which introduces minimal changes to the existing API and can allow us to implement this feature early.

The workflow of fetching comments and rendering comment widget/panel will change a little bit as below

Firstly, the core will call provideDocumentComments(document: TextDocument, token: CancellationToken): Promise<CommentInfo>; when opening a file. The return result CommentInfo will now indicate whether it contains drafts or not (the draft comment doesn't necessary need to be in this document though)

interface CommentInfo {
	threads: CommentThread[];
	commentingRanges?: Range[];

	/**
	 * If it's in draft mode or not
	 */
	inDraftMode?: boolean;
}

Then the core will validate whether the DocumentCommentProvider has following draft related methods/properties or not

interface DocumentCommentProvider {
	...

	startDraft?(token: CancellationToken): Promise<void>;
	deleteDraft?(token: CancellationToken): Promise<void>;
	finishDraft?(token: CancellationToken): Promise<void>;

	startDraftLabel?: string;
	deleteDraftLabel?: string;
	finishDraftLabel?: string;
	
	...
}

If these properties exist, we will render start/delete/finish drafts buttons on the Comment Widget.

[agurod42]

Hi! @rebornix,

First of all, thank you for this api draft.

I'm working on an extension that make use of this api (To be release when this api gets into code) and I have a use case which I'm not being able to solve with the current api methods.

What I'm trying to do is to fetch -in the background- comments from GitHub and show them using the VSCode Comments API. The problem is that if I have already opened the file in the current active text editor the provideDocumentComments doesn't get called. Currently the only way to invoke that method (and so "refresh" the comments) is reopening the file.

I was wondering if this is a desired API limitation, or maybe I'm missing something?

[rebornix]

@agurodriguez should ThreadChangeEvent help in this case https://github.com/Microsoft/vscode-pull-request-github/blob/master/src/typings/vscode.proposed.d.ts#L849 ? Say a file is opened (and visible) before we get the comments back from service, we can check all visible editors and then trigger a thread update event and tell the editor to update.

[agurod42]

Thank you for your response @rebornix, what you suggested worked perfectly for me.

While I was working in this change, another question came to my mind:

Let say I want to have a comment outlet always visible in the line in which the cursor currently is. To do that I'm registering a CommentProvider which returns the following structure in it's provideDocumentComments method:

if (vscode.window.activeTextEditor) {
	const file = ReviewMachine.files[document.uri.path];
	const line = vscode.window.activeTextEditor.selection.active.line;
	if (file && file.blame[line] && file.blame[line].sha !== '000000') {
		commentingRanges.push(document.lineAt(line).range);
	}
}

const commentInfo: vscode.CommentInfo = {
	commentingRanges: commentingRanges,
	threads: []
};

return Promise.resolve(commentInfo);

The problem is: when I move the cursor, in order to move the outlet with it, I need to "refresh" the commentingRanges variable. To do that I need to get the provideDocumentComments called. And to do that I need to fire the event you suggested. The problem is: If I trigger the event without added, changed or removed threads, the provideDocumentComments doesn't get called (Which I think it's the expected behavior and indeed it's the way it should behave).

What I'm doing in order to work it out, is -whenever the text selection change- to dispose and re-create the listener constructed when the CommentProvider is registered. Which I think it's not a good solution. I believe the correct course of action would be having a way to "force" the invocation of the provideDocumentComments method regardless of the existence of changes in comment threads.

What do you think about it?

[rebornix]

Close this one in favor of #68020 . @agurodriguez we can continue our discussion there.