New Comments API #972
New Comments API #972
Conversation
package.json
Outdated
| @@ -397,7 +432,6 @@ | |||
| }, | |||
| { | |||
| "command": "pr.refreshPullRequest", | |||
| "when": "view == pr && viewItem =~ /pullrequest|description/", | |||
| "group": "pullrequest@2" | |||
There was a problem hiding this comment.
It's a hack as context values seem not working when debugging.
|
|
||
| export function getEmptyCommentThreadCommands(thread: vscode.CommentThread, inDraftMode: boolean, handler: CommentHandler, supportGraphQL: boolean): { acceptInputCommand: vscode.Command, additionalCommands: vscode.Command[] } { | ||
| let commands: vscode.Command[] = []; | ||
| let acceptInputCommand = { |
There was a problem hiding this comment.
I think the title for this command should also change based on whether we are in draft mode like getCommentThreadCommands below, either Add Comment or Add Review Comment
src/github/utils.ts
Outdated
| [] | ||
| ); | ||
|
|
||
| thread.comments.forEach(comment => { |
There was a problem hiding this comment.
could move this above and then call createCommentThread with vscodeThread.comments so there's no update event
There was a problem hiding this comment.
comment relies on thread as it's one argument for edit and delete command.
There was a problem hiding this comment.
Here I put comments in ctor and then update comments again with proper edit/delete commands, the reason is if we create comment thread with empty comments, there are chances that VS Code focuses in the comment editor.
|
|
||
| if (fileChange instanceof RemoteFileChangeNode) { | ||
| throw new Error('Comments not supported on remote file changes'); | ||
| thread.comments = [...thread.comments, convertToVSCodeComment(rawComment!, undefined)]; |
There was a problem hiding this comment.
The edit and delete commands are not being added here, we should have another method that will create comments when the thread already exists
| canDelete: comment.canDelete, | ||
| isDraft: !!comment.isDraft, | ||
| userIconPath: vscode.Uri.parse(comment.user!.avatarUrl), | ||
| label: !!comment.isDraft ? 'Pending' : undefined, |
There was a problem hiding this comment.
This seems to be an upstream issue, if I start a review, and then reload vscode, the comments are not initially shown with the 'Pending' label. I see that the label is correctly being set here. If I add another pending comment to the thread, the labels for the thread are rendered
| await this._prManager.deleteReviewComment(this.pullRequestModel, comment.commentId); | ||
| const index = fileChange.comments.findIndex(c => c.id.toString() === comment.commentId); | ||
| const index = thread.comments.findIndex(c => c.commentId === comment.commentId); | ||
| if (index > -1) { |
There was a problem hiding this comment.
If the comment is the last in the thread, we should also dispose the thread. Ditto in the reviewDocumentCommentProvider
|
@RMacfarlane thanks for your thorough review, I feel confident with our product quality as your review is always high quality ❤️ I addressed your comments and fixed a few upstream issues (not in Insiders yet so please wait till Monday). In addition to issues you ran into, I also fixed a few bugs with Review and comments update. Since we didn't merge this pr when I was still in office, I need your help to merge this pr. You may still run into bugs so it would be great if you have a better understanding how comment threads cache is managed in this PR
The challenge here is we now moved to The code architecture can be improved by moving the visible editors change listener to |
|
Thanks for the clear explanation! I've done some more testing and found some more issues that need to be fixed, seems like a lot of them are upstream.
|
…ng text collisions in vscode core
The refactoring of registering comment threas for Pull Request list view is done but review mode part is still in progress, following code needs to be improved
pr,reviewandfilescheme files