Comments API: resolved/unresolved comments

[laurentlb]

The comments API doesn't have the concept of resolved/unresolved threads, although most code review tools distinguish them. The current API allows us to surface this to the users, see #122243

However, this seems pretty limited. I find it difficult to identify all my unresolved comments. It's very easy to miss some of them. If the API had a boolean for unresolved comments, we could surface them in multiple ways:

• In the Comments panel, surface the number of unresolved comments, similar to what is done with Problems:
• In the Comments panel, highlight the unresolved comments (or put them in bold), so that it's obvious to the user what comments they still have to go through.
• In the Comments panel, have a way to hide/filter the resolved comments.
• In the Editor, consider consider using a different background color for unresolved comments. I currently use a different label for unresolved comments, but this is very subtle.
• In the gutter, use a different symbol for unresolved comments (e.g. or a diamond of another color).
• In the Editor, provide a way to go to the next unresolved comment (an alternative to Go to Next Comment Thread).

[laurentlb]

Let's discuss the API change.

IMO, the information should be part of a CommentThread.

On many levels, Comments seem similar to Diagnostics. A diagnostic is a comment provided by a tool such as a compiler. A comment is typically provided by a human. Diagnostics have a severity field, which indicates how important is the problem. We need an equivalent for Comments, something that tells the user if an action is expected (e.g. they need to resolve it) or not. Once the user addresses the comment, the importance is reduced.

1. A simplistic solution is to add a boolean, e.g. isResolved: boolean.
2. Similar to collapsibleState, we can go for a two-state enum instead: resolvedState: resolved | unresolved
3. Or to be more general: importance: high | low.

We might consider having more fields here. In a code review, multiple users interact with the comments. The current VSCode user is not necessarily the person who needs to do something. For example, the user has just updated the comment and they wait for a response. So the third value could be used to mean "it's not my turn" or "acknowledged, I'll come back later". The benefit is that the user can use the list of "unresolved" issues as a TODO-list and publish the comments once it's empty.

4. resolvedState: resolved | pending | unresolved

[isidorn]

@laurentlb Thank you very much for your proposal.
@rebornix, @alexr00 and me have quickly discussed this and we believe this proposal makes good sense.
We plan to start looking into it next milestone. Though no promises yet when this will land, we will know more in December when we start our investigation.

[alexr00]

All of the proposals in #127473 (comment) have merit. In a perfect world I would choose one of options 2 or 4; however, we have users of the comments API who's comments don't have a resolved/unresolved state. Option 3 circumvents that, but then we have the issue of what to show in the UI: a GitHub PR comment isn't high or low priority.

Another proposal, similar to how comment reactions work:

vscode/src/vscode-dts/vscode.proposed.commentsResolvedState.d.ts

Lines 10 to 47 in 143578f

	export interface CommentState {
	/**
	* The human-readable label for the comment state. This may be shown in the UI (ex. "Mark comment as <label>").
	*/
	readonly label: string;
	/**
	* Icon for the state. This may be shown in the UI.
	*/
	readonly iconPath: string | Uri | ThemeIcon;
	/**
	* An optional color that may be used to indicate the comment's state.
	*/
	readonly color?: ThemeColor;
	// todo@alexr00 Do we also need a priority so we can count how many high priority comments there are in the UI?
	}
	export interface CommentController {
	/**
	* Optional state handler for adding or modifying the state of a comment. This can be used
	* for setting a comment's state to "resolved" or "unresolved" for example.
	*/
	stateHandler?: (comment: Comment, state: CommentState) => Thenable<void>;
	}
	export interface Comment {
	/**
	* Optional possible states of the {@link Comment}
	*/
	states?: CommentState[];
	/**
	* Optional current state of the {@link Comment}
	*/
	state?: CommentState;
	}

[laurentlb]

I see than @hermannloose left comments on the commit (6197a2d#diff-1dfb341fde2874ea77bfad0b6f93fe906baf50642c07905158906ec33116d888).

I like the approach of having an object for the state (instead of the proposed boolean/enum), it gives more flexibility and lets user create their own states. In my proposal, the information was attached to a thread, not to a single comment. Do we have use-cases where we want this information per comment?

We might also want to control the gutter icon:

readonly gutterIconPath?: string | Uri;

[alexr00]

Do we have use-cases where we want this information per comment?

GitHub comments are resolved per-thread, so no, no cases were we want it per comment. I'll move it to the thread.

[alexr00]

After much discussion, we have decided to proceed with the following:

vscode/src/vscode-dts/vscode.proposed.commentsResolvedState.d.ts

Lines 10 to 17 in 1712a23

	export enum CommentThreadState {
	Unresolved = 0,
	Resolved = 1
	}
	export interface CommentThread {
	state?: CommentThreadState;
	}

The main reasoning behind this is that resolved vs. unresolved comments should look the same to the user across a GitHub PR extension, an Azure Dev Ops PR extensions, and any other PR extension.

There original concern around using "resolved" and "unresolved" is that maybe not all comment provider have those states (maybe they have "viewed" and "unviewed" states for example). To alleviate this, we'll see if we can avoid using "resolved"/"unresolved" language in the UI. If additional states are needed later, we can always add more to the API.

[alexr00]

If anyone is eager for this API to be finalized then please comment and let me know how it's working for you. Otherwise, I will plan to finalize later in 2022.

[circlesmiler]

This would be very nice to have. We are using Github and especially larger PRs are hard to manage, if there are many comments (in Github as well as in VSCode)

[violetbrina]

So is there currently a way to filter resolved/unresolved comments in the sidebar? If not what can I modify currently to provide a visual distinction between resolved/unresolved?

[alexr00]

@violetbrina there's a color difference on the icon, but that's currently it. This is the feature request you're looking for #150958