Skip to content
Draft
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
3 changes: 2 additions & 1 deletion src/github/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,8 @@ const link = (url: string, token: string) =>
headers: {
...headers,
authorization: token ? `Bearer ${token}` : '',
Accept: 'application/vnd.github.merge-info-preview'
Accept: 'application/vnd.github.merge-info-preview',
'GraphQL-Features': 'graphql_pr_comment_positioning'
},
})).concat(
createHttpLink({
Expand Down
6 changes: 6 additions & 0 deletions src/github/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,12 @@ export interface ReviewThread {
}
}]
};
positioning?: {
__typename: string;
startLine?: number;
endLine?: number;
line?: number;
}
}

export interface TimelineEventsResponse {
Expand Down
32 changes: 27 additions & 5 deletions src/github/pullRequestModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,26 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
const pendingReviewId = await this.getPendingReviewId();

const { mutate, schema } = await this.githubRepository.ensure();

let linePositioning: any;
let multilinePositioning: any;
if (startLine === endLine && startLine !== undefined) {
linePositioning = {
line: startLine,
path: commentPath,
commitOid: this.head?.sha
};
} else if (startLine !== undefined && endLine !== undefined) {
multilinePositioning = {
startLine,
endLine,
startPath: commentPath,
endPath: commentPath,
startCommitOid: this.head?.sha,
endCommitOid: this.head?.sha
};
}

const { data } = await mutate<AddReviewThreadResponse>({
mutation: schema.AddReviewThread,
variables: {
Expand All @@ -753,7 +773,9 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
startLine: startLine === endLine ? undefined : startLine,
line: (endLine === undefined) ? 0 : endLine,
side,
subjectType: (startLine === undefined || endLine === undefined) ? SubjectType.FILE : SubjectType.LINE
subjectType: (startLine === undefined || endLine === undefined) ? SubjectType.FILE : SubjectType.LINE,
linePositioning,
multilinePositioning
}
}
}, { mutation: schema.LegacyAddReviewThread, deleteProps: ['subjectType'] });
Expand All @@ -772,7 +794,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
}

const thread = data.addPullRequestReviewThread.thread;
const newThread = parseGraphQLReviewThread(thread, this.githubRepository);
const newThread = parseGraphQLReviewThread(thread, this.githubRepository, this.head?.sha);
if (!this._reviewThreadsCache) {
this._reviewThreadsCache = [];
}
Expand Down Expand Up @@ -1453,7 +1475,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
}

private setReviewThreadCacheFromRaw(raw: ReviewThread[]): IReviewThread[] {
const reviewThreads: IReviewThread[] = raw.map(thread => parseGraphQLReviewThread(thread, this.githubRepository));
const reviewThreads: IReviewThread[] = raw.map(thread => parseGraphQLReviewThread(thread, this.githubRepository, this.head?.sha));
const oldReviewThreads = this._reviewThreadsCache ?? [];
this._reviewThreadsCache = reviewThreads;
this.diffThreads(oldReviewThreads, reviewThreads);
Expand Down Expand Up @@ -2105,7 +2127,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe

const index = this._reviewThreadsCache?.findIndex(thread => thread.id === threadId) ?? -1;
if (index > -1) {
const thread = parseGraphQLReviewThread(data.resolveReviewThread.thread, this.githubRepository);
const thread = parseGraphQLReviewThread(data.resolveReviewThread.thread, this.githubRepository, this.head?.sha);
this._reviewThreadsCache?.splice(index, 1, thread);
this._onDidChangeReviewThreads.fire({ added: [], changed: [thread], removed: [] });
}
Expand Down Expand Up @@ -2148,7 +2170,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe

const index = this._reviewThreadsCache?.findIndex(thread => thread.id === threadId) ?? -1;
if (index > -1) {
const thread = parseGraphQLReviewThread(data.unresolveReviewThread.thread, this.githubRepository);
const thread = parseGraphQLReviewThread(data.unresolveReviewThread.thread, this.githubRepository, this.head?.sha);
this._reviewThreadsCache?.splice(index, 1, thread);
this._onDidChangeReviewThreads.fire({ added: [], changed: [thread], removed: [] });
}
Expand Down
10 changes: 10 additions & 0 deletions src/github/queriesShared.gql
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,16 @@ query PullRequestComments($owner: String!, $name: String!, $number: Int!, $after
...ReviewComment
}
}
positioning {
__typename
... on MultilineComment {
startLine
endLine
}
... on LineComment {
line
}
}
}
pageInfo {
hasNextPage
Expand Down
32 changes: 27 additions & 5 deletions src/github/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,29 @@ export function convertGraphQLEventType(text: string) {
}
}

export function parseGraphQLReviewThread(thread: GraphQL.ReviewThread, githubRepository: GitHubRepository): IReviewThread {
export function parseGraphQLReviewThread(thread: GraphQL.ReviewThread, githubRepository: GitHubRepository, headCommitOid?: string): IReviewThread {
let startLine: number = 0;
let endLine: number = 0;
if (thread.positioning) {
if (thread.positioning.__typename === 'MultilineComment') {
startLine = thread.positioning.startLine!;
endLine = thread.positioning.endLine!;
} else if (thread.positioning.__typename === 'LineComment') {
startLine = thread.positioning.line!;
endLine = thread.positioning.line!;
}
} else {
startLine = thread.startLine ?? thread.line;
endLine = thread.line;
}

// There's a bug with comments outside the diff where they always show as outdated. Try to work around that.
let isOutdated = thread.isOutdated;
if (isOutdated && headCommitOid && thread.comments.nodes.length > 0 &&
thread.comments.nodes.every(comment => comment.commit.oid === headCommitOid)) {
isOutdated = false;
}

return {
id: thread.id,
prReviewDatabaseId: thread.comments.edges && thread.comments.edges.length ?
Expand All @@ -520,13 +542,13 @@ export function parseGraphQLReviewThread(thread: GraphQL.ReviewThread, githubRep
viewerCanResolve: thread.viewerCanResolve,
viewerCanUnresolve: thread.viewerCanUnresolve,
path: thread.path,
startLine: thread.startLine ?? thread.line,
endLine: thread.line,
startLine,
endLine,
originalStartLine: thread.originalStartLine ?? thread.originalLine,
originalEndLine: thread.originalLine,
diffSide: thread.diffSide,
isOutdated: thread.isOutdated,
comments: thread.comments.nodes.map(comment => parseGraphQLComment(comment, thread.isResolved, thread.isOutdated, githubRepository)),
isOutdated,
comments: thread.comments.nodes.map(comment => parseGraphQLComment(comment, thread.isResolved, isOutdated, githubRepository)),
subjectType: thread.subjectType ?? SubjectType.LINE
};
}
Expand Down
12 changes: 11 additions & 1 deletion src/test/view/reviewCommentController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,13 @@ describe('ReviewCommentController', function () {
startLine: undefined,
line: 22,
side: 'RIGHT',
subjectType: 'LINE'
subjectType: 'LINE',
linePositioning: {
line: 22,
path: fileName,
commitOid: activePullRequest.head?.sha
},
multilinePositioning: undefined
}
}
},
Expand Down Expand Up @@ -322,6 +328,10 @@ describe('ReviewCommentController', function () {
author: {}
}
]
},
positioning: {
__typename: 'LineComment',
line: 22,
}
}
}
Expand Down
35 changes: 20 additions & 15 deletions src/view/reviewCommentController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
CommentReactionHandler,
createVSCodeCommentThreadForReviewThread,
getRepositoryForFile,
isEnterprise,
isFileInRepo,
setReplyAuthor,
threadRange,
Expand Down Expand Up @@ -522,25 +523,29 @@ export class ReviewCommentController extends CommentControllerBase implements Co
const ranges: vscode.Range[] = [];

if (matchedFile) {
const diffHunks = await matchedFile.changeModel.diffHunks();
if ((matchedFile.status === GitChangeType.RENAME) && (diffHunks.length === 0)) {
Logger.debug('No commenting ranges: File was renamed with no diffs.', ReviewCommentController.ID);
return { ranges: [], enableFileComments: true };
}
if (!isEnterprise(this._folderRepoManager.activePullRequest.remote.authProviderId)) {
ranges.push(new vscode.Range(0, 0, document.lineCount - 1, document.lineAt(document.lineCount - 1).text.length - 1));
} else {
const diffHunks = await matchedFile.changeModel.diffHunks();
if ((matchedFile.status === GitChangeType.RENAME) && (diffHunks.length === 0)) {
Logger.debug('No commenting ranges: File was renamed with no diffs.', ReviewCommentController.ID);
return { ranges: [], enableFileComments: true };
}

const contentDiff = await this.getContentDiff(document.uri, matchedFile.fileName);
const contentDiff = await this.getContentDiff(document.uri, matchedFile.fileName);

for (let i = 0; i < diffHunks.length; i++) {
const diffHunk = diffHunks[i];
const start = mapOldPositionToNew(contentDiff, diffHunk.newLineNumber, document.lineCount);
const end = mapOldPositionToNew(contentDiff, diffHunk.newLineNumber + diffHunk.newLength - 1, document.lineCount);
if (start > 0 && end > 0) {
ranges.push(new vscode.Range(start - 1, 0, end - 1, 0));
for (let i = 0; i < diffHunks.length; i++) {
const diffHunk = diffHunks[i];
const start = mapOldPositionToNew(contentDiff, diffHunk.newLineNumber, document.lineCount);
const end = mapOldPositionToNew(contentDiff, diffHunk.newLineNumber + diffHunk.newLength - 1, document.lineCount);
if (start > 0 && end > 0) {
ranges.push(new vscode.Range(start - 1, 0, end - 1, 0));
}
}
}

if (ranges.length === 0) {
Logger.debug('No commenting ranges: File has diffs, but they could not be mapped to current lines.', ReviewCommentController.ID);
if (ranges.length === 0) {
Logger.debug('No commenting ranges: File has diffs, but they could not be mapped to current lines.', ReviewCommentController.ID);
}
}
} else {
Logger.debug('No commenting ranges: File does not match any of the files in the review.', ReviewCommentController.ID);
Expand Down
7 changes: 6 additions & 1 deletion src/view/treeNodes/pullRequestNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { getIconForeground, getListErrorForeground, getListWarningForeground, ge
import { DirectoryTreeNode } from './directoryTreeNode';
import { InMemFileChangeNode, RemoteFileChangeNode } from './fileChangeNode';
import { TreeNode, TreeNodeParent } from './treeNode';
import { isEnterprise } from '../../github/utils';
import { NotificationsManager } from '../../notifications/notificationsManager';
import { PrsTreeModel } from '../prsTreeModel';

Expand Down Expand Up @@ -411,7 +412,11 @@ export class PRNode extends TreeNode implements vscode.CommentingRangeProvider2
return undefined;
}

return { ranges: getCommentingRanges(await fileChange.changeModel.diffHunks(), params.isBase, PRNode.ID), enableFileComments: true };
if (isEnterprise(this.pullRequestModel.remote.authProviderId)) {
return { ranges: getCommentingRanges(await fileChange.changeModel.diffHunks(), params.isBase, PRNode.ID), enableFileComments: true };
} else {
return { ranges: [(new vscode.Range(0, 0, document.lineCount - 1, document.lineAt(document.lineCount).text.length))], enableFileComments: true };
}
}

return undefined;
Expand Down