the position mapping between diff hunk and file is correct now

extensions/git-extended/src/common/comment.ts

@@ -5,13 +5,13 @@
 
 import * as path from 'path';
 import { Comment } from './models/comment';
-import { DIFF_HUNK_INFO } from './diff';
+import { DIFF_HUNK_HEADER } from './diff';
 import { FileChangeTreeItem } from './treeItems';
 
 export function parseComments(comments: any[]): Comment[] {
 	for (let i = 0; i < comments.length; i++) {
 		let diff_hunk = comments[i].diff_hunk;
-		let hunk_info = DIFF_HUNK_INFO.exec(diff_hunk);
+		let hunk_info = DIFF_HUNK_HEADER.exec(diff_hunk);
 		let oriStartLine = Number(hunk_info[1]);
 		let oriLen = Number(hunk_info[3]) | 0;
 		let startLine = Number(hunk_info[5]);

extensions/git-extended/src/common/diff.ts

@@ -8,39 +8,226 @@ import { getFileContent, writeTmpFile } from './file';
 import { GitChangeType, RichFileChange } from './models/file';
 import { Repository } from './models/repository';
 import { Comment } from './models/comment';
+import { DiffHunk } from './models/diffHunk';
+import { getDiffChangeType, DiffLine, DiffChangeType } from './models/diffLine';
 
 export const MODIFY_DIFF_INFO = /diff --git a\/(\S+) b\/(\S+).*\n*index.*\n*-{3}.*\n*\+{3}.*\n*((.*\n*)+)/;
 export const NEW_FILE_INFO = /diff --git a\/(\S+) b\/(\S+).*\n*new file mode .*\nindex.*\n*-{3}.*\n*\+{3}.*\n*((.*\n*)+)/;
 export const DELETE_FILE_INFO = /diff --git a\/(\S+) b\/(\S+).*\n*deleted file mode .*\nindex.*\n*-{3}.*\n*\+{3}.*\n*((.*\n*)+)/;
-export const DIFF_HUNK_INFO = /@@ \-(\d+)(,(\d+))?( \+(\d+)(,(\d+)?))? @@/;
+export const DIFF_HUNK_HEADER = /@@ \-(\d+)(,(\d+))?( \+(\d+)(,(\d+)?))? @@/;
 
+export function countCarriageReturns(text: string): number {
+	let count = 0;
+	let index = 0;
+	while ((index = text.indexOf('\r', index)) !== -1) {
+		index++;
+		count++;
+	}
+
+	return count;
+}
+
+function* LineReader(text: string): IterableIterator<string> {
+	let index = 0;
+
+	while (index !== -1 && index < text.length) {
+		let startIndex = index;
+		index = text.indexOf('\n', index);
+		let endIndex = index !== -1 ? index : text.length;
+		let length = endIndex - startIndex;
+
+		if (index !== -1) {
+			if (index > 0 && text[index - 1] === '\r') {
+				length--;
+			}
+
+			index++;
+		}
+
+		yield text.substr(startIndex, length);
+	}
+}
+
+export function* parseDiffHunk(diffHunkPatch: string): IterableIterator<DiffHunk> {
+	let lineReader = LineReader(diffHunkPatch);
+
+	let itr = lineReader.next();
+	let diffHunk: DiffHunk = null;
+	let diffLine = -1;
+	let oldLine = -1;
+	let newLine = -1;
+
+	while (!itr.done) {
+		let line = itr.value;
+		if (DIFF_HUNK_HEADER.test(line)) {
+			if (diffHunk) {
+				yield diffHunk;
+				diffHunk = null;
+			}
+
+			if (diffLine === -1) {
+				diffLine = 0;
+			}
+
+			let matches = DIFF_HUNK_HEADER.exec(line);
+			let oriStartLine = oldLine = Number(matches[1]);
+			let oriLen = Number(matches[3]) | 0;
+			let newStartLine = newLine = Number(matches[5]);
+			let newLen = Number(matches[7]) | 0;
+
+			diffHunk = new DiffHunk(oriStartLine, oriLen, newStartLine, newLen, diffLine);
+		} else if (diffHunk !== null) {
+			let type = getDiffChangeType(line[0]);
+
+			if (type !== DiffChangeType.Control) {
+				diffHunk.Lines.push(new DiffLine(type, type !== DiffChangeType.Add ? oldLine : -1,
+					type !== DiffChangeType.Delete ? newLine : -1,
+					diffLine,
+					line
+				));
+
+				var lineCount = 1;
+				lineCount += countCarriageReturns(line);
+
+				switch (type) {
+					case DiffChangeType.None:
+						oldLine += lineCount;
+						newLine += lineCount;
+						break;
+					case DiffChangeType.Delete:
+						oldLine += lineCount;
+						break;
+					case DiffChangeType.Add:
+						newLine += lineCount;
+						break;
+				}
+			}
+		}
+		if (diffLine !== -1) {
+			++diffLine;
+		}
+		itr = lineReader.next();
+	}
+
+	if (diffHunk) {
+		yield diffHunk;
+	}
+}
+
+export function getDiffLine(prPatch: string, diffLineNumber: number): DiffLine {
+	let prDiffReader = parseDiffHunk(prPatch);
+	let prDiffIter = prDiffReader.next();
+
+	while (!prDiffIter.done) {
+		let diffHunk = prDiffIter.value;
+		for (let i = 0; i < diffHunk.Lines.length; i++) {
+			if (diffHunk.Lines[i].diffLineNumber === diffLineNumber) {
+				return diffHunk.Lines[i];
+			}
+		}
+
+		prDiffIter = prDiffReader.next();
+	}
+
+	return null;
+}
+
+export function mapPositionHeadToDiffHunk(prPatch: string, localDiff: string, line: number): number {
+	let delta = 0;
+
+	let localDiffReader = parseDiffHunk(localDiff);
+	let localDiffIter = localDiffReader.next();
+	let lineInPRDiff = line;
+
+	while (!localDiffIter.done) {
+		let diffHunk = localDiffIter.value;
+		if (diffHunk.newLineNumber + diffHunk.newLength - 1 < line) {
+			delta += diffHunk.oldLength - diffHunk.newLength;
+		} else {
+			lineInPRDiff = line + delta;
+			break;
+		}
+
+		localDiffIter = localDiffReader.next();
+	}
+
+	let prDiffReader = parseDiffHunk(prPatch);
+	let prDiffIter = prDiffReader.next();
+
+	let positionInDiffHunk = -1;
+
+	while (!prDiffIter.done) {
+		let diffHunk = prDiffIter.value;
+		if (diffHunk.newLineNumber <= lineInPRDiff && diffHunk.newLineNumber + diffHunk.newLength - 1 >= lineInPRDiff) {
+			positionInDiffHunk = lineInPRDiff - diffHunk.newLineNumber + diffHunk.diffLine + 1;
+			break;
+		}
+
+		prDiffIter = prDiffReader.next();
+	}
+
+	return positionInDiffHunk;
+}
+
+export function mapOldPositionToNew(patch: string, line: number): number {
+	let diffReader = parseDiffHunk(patch);
+	let diffIter = diffReader.next();
+
+	let delta = 0;
+	while (!diffIter.done) {
+		let diffHunk = diffIter.value;
+
+		if (diffHunk.oldLineNumber > line) {
+			continue;
+		} else if (diffHunk.oldLineNumber + diffHunk.oldLength - 1 < line) {
+			delta = diffHunk.newLength - diffHunk.oldLength;
+		} else {
+			return line + delta;
+		}
+
+		diffIter = diffReader.next();
+	}
+
+	return line + delta;
+}
+export function mapDiffHunkToHeadRange(diffHunk: DiffHunk, localDiff: string): [number, number] {
+	let startLineNumber = diffHunk.newLineNumber;
+	let endLineNumber = diffHunk.newLineNumber + diffHunk.newLength - 1;
+
+	return [mapOldPositionToNew(localDiff, startLineNumber), mapOldPositionToNew(localDiff, endLineNumber)];
+}
 
 async function parseModifiedHunkComplete(originalContent, patch, a, b) {
 	let left = originalContent.split(/\r|\n|\r\n/);
-	let diffHunks = patch.split('\n');
-	diffHunks.pop(); // there is one additional line break at the end of the diff ??
+	let diffHunkReader = parseDiffHunk(patch);
+	let diffHunkIter = diffHunkReader.next();
 
 	let right = [];
 	let lastCommonLine = 0;
-	for (let i = 0; i < diffHunks.length; i++) {
-		let line = diffHunks[i];
-		if (DIFF_HUNK_INFO.test(line)) {
-			let changeInfo = DIFF_HUNK_INFO.exec(line);
-			let oriStartLine = Number(changeInfo[1]);
-			let oriEndLine = Number(changeInfo[3]) | 0;
+	while (!diffHunkIter.done) {
+		let diffHunk = diffHunkIter.value;
+
+		let oriStartLine = diffHunk.oldLineNumber;
 
-			for (let j = lastCommonLine + 1; j < oriStartLine; j++) {
-				right.push(left[j - 1]);
+		for (let j = lastCommonLine + 1; j < oriStartLine; j++) {
+			right.push(left[j - 1]);
+		}
+
+		for (let j = 0; j < diffHunk.Lines.length; j++) {
+			let diffLine = diffHunk.Lines[j];
+			if (diffLine.type === DiffChangeType.Delete) {
+				lastCommonLine++;
+			} else if (diffLine.type === DiffChangeType.Add) {
+				lastCommonLine++;
+				right.push(diffLine.content.substr(1));
+			} else {
+				lastCommonLine++;
+				let codeInFirstLine = diffLine.content.substr(1);
+				right.push(codeInFirstLine);
 			}
-			lastCommonLine = oriStartLine + oriEndLine - 1;
-		} else if (/^\-/.test(line)) {
-			// do nothing
-		} else if (/^\+/.test(line)) {
-			right.push(line.substr(1));
-		} else {
-			let codeInFirstLine = line.substr(1);
-			right.push(codeInFirstLine);
 		}
+
+		diffHunkIter = diffHunkReader.next();
 	}
 
 	if (lastCommonLine < left.length) {
@@ -140,44 +327,14 @@ export async function parseDiff(reviews: any[], repository: Repository, parentCo
 	return richFileChanges;
 }
 
-export function mapCommentsToHead(patches: string, comments: Comment[]) {
-	let regex = new RegExp(DIFF_HUNK_INFO, 'g');
-	let matches = regex.exec(patches);
-
-	let rangeMapping = [];
-	const diffHunkContext = 3;
-	while (matches) {
-		let oriStartLine = Number(matches[1]);
-		let oriLen = Number(matches[3]) | 0;
-		let newStartLine = Number(matches[5]);
-		let newLen = Number(matches[7]) | 0;
-
-		rangeMapping.push({
-			oriStart: oriStartLine + diffHunkContext,
-			oriLen: oriLen - diffHunkContext * 2,
-			newStart: newStartLine + diffHunkContext,
-			newLen: newLen - diffHunkContext * 2
-		});
-		matches = regex.exec(patches);
-	}
-
+export function mapCommentsToHead(prPatch: string, localDiff: string, comments: Comment[]) {
 	for (let i = 0; i < comments.length; i++) {
 		let comment = comments[i];
-		const startPosition = comment.position === null ? comment.original_position : comment.position - 1;
-		let commentPosition = comment.diff_hunk_range.start + startPosition;
-		let delta = 0;
-		for (let j = 0; j < rangeMapping.length; j++) {
-			let map = rangeMapping[j];
-			if (map.oriStart + map.oriLen - 1 < commentPosition) {
-				delta += map.newLen - map.oriLen;
-			} else if (map.oriStart > commentPosition) {
-				continue;
-			} else {
-				break;
-			}
-		}
 
-		comment.currentPosition = commentPosition + delta;
+		let diffLine = getDiffLine(prPatch, comment.position);
+		let positionInPr = diffLine.newLineNumber;
+		let newPosition = mapOldPositionToNew(localDiff, positionInPr);
+		comment.absolutePosition = newPosition;
 	}
 
 	return comments;

extensions/git-extended/src/common/models/comment.ts

@@ -32,6 +32,6 @@ export interface Comment {
 	created_at: string;
 	updated_at: string;
 	html_url: string;
-	currentPosition?: number;
+	absolutePosition?: number;
 }
 

extensions/git-extended/src/common/models/diffHunk.ts

@@ -10,7 +10,9 @@ export class DiffHunk {
 
 	constructor(
 		public oldLineNumber: number,
+		public oldLength: number,
 		public newLineNumber: number,
+		public newLength: number,
 		public diffLine: number
 	) { }
 }
\ No newline at end of file

extensions/git-extended/src/common/models/file.ts

@@ -50,6 +50,7 @@ export class RichFileChange {
 		public readonly originalFilePath: string,
 		public readonly status: GitChangeType,
 		public readonly fileName: string,
+		// public readonly diffHunks: DiffHunk[],
 		public readonly patch: string
 	) { }
 }
\ No newline at end of file

extensions/git-extended/src/prView/prProvider.ts

@@ -5,7 +5,7 @@
 
 import * as path from 'path';
 import * as vscode from 'vscode';
-import { parseDiff, DIFF_HUNK_INFO } from '../common/diff';
+import { parseDiff, parseDiffHunk, getDiffLine, mapPositionHeadToDiffHunk } from '../common/diff';
 import { Repository } from '../common//models/repository';
 import { Comment } from '../common/models/comment';
 import * as _ from 'lodash';
@@ -112,7 +112,7 @@ export class PRProvider implements vscode.TreeDataProvider<PRGroupTreeItem | Pul
 			});
 
 			vscode.commands.registerCommand('diff-' + element.prItem.number + '-post', async (uri: vscode.Uri, range: vscode.Range, thread: vscode.CommentThread, text: string) => {
-				if (thread) {
+				if (thread && thread.threadId) {
 					try {
 						let ret = await element.createCommentReply(text, thread.threadId);
 						return {
@@ -127,29 +127,21 @@ export class PRProvider implements vscode.TreeDataProvider<PRGroupTreeItem | Pul
 					let params = JSON.parse(uri.query);
 
 					let fileChange = richContentChanges.find(change => change.fileName === params.fileName);
-					let regex = new RegExp(DIFF_HUNK_INFO, 'g');
-					let matches = regex.exec(fileChange.patch);
 
-					let position;
-					while (matches) {
-						let newStartLine = Number(matches[5]);
-						let newLen = Number(matches[7]) | 0;
-
-						if (range.start.line >= newStartLine && range.start.line <= newStartLine + newLen - 1) {
-							position = range.start.line - newStartLine + 1;
-							break;
-						}
-
-						matches = regex.exec(fileChange.patch);
+					if (!fileChange) {
+						return null;
 					}
 
-					if (!position) {
+					let position = mapPositionHeadToDiffHunk(fileChange.patch, '', range.start.line);
+
+					if (position < 0) {
 						return;
 					}
 
 					// there is no thread Id, which means it's a new thread
 					let ret = await element.createComment(text, params.fileName, position);
 					return {
+						commentId: ret.data.id,
 						body: new vscode.MarkdownString(ret.data.body),
 						userName: ret.data.user.login,
 						gravatar: ret.data.user.avatar_url
@@ -171,22 +163,16 @@ export class PRProvider implements vscode.TreeDataProvider<PRGroupTreeItem | Pul
 						if (!fileChange) {
 							return null;
 						}
-						let regex = new RegExp(DIFF_HUNK_INFO, 'g');
-						let matches = regex.exec(fileChange.patch);
 
 						let commentingRanges: vscode.Range[] = [];
-						while (matches) {
-							let newStartLine = Number(matches[5]);
-							let newLen = Number(matches[7]) | 0;
-
-							commentingRanges.push(new vscode.Range(
-								newStartLine - 1,
-								0,
-								newStartLine + newLen - 1 - 1,
-								document.lineAt(newStartLine + newLen - 1 - 1).text.length
-							));
-
-							matches = regex.exec(fileChange.patch);
+
+						let diffHunkReader = parseDiffHunk(fileChange.patch);
+						let diffHunkIter = diffHunkReader.next();
+
+						while (!diffHunkIter.done) {
+							let diffHunk = diffHunkIter.value;
+							commentingRanges.push(new vscode.Range(diffHunk.newLineNumber, 1, diffHunk.newLineNumber + diffHunk.newLength - 1, 1));
+							diffHunkIter = diffHunkReader.next();
 						}
 
 						let matchingComments = commentsCache.get(document.uri.toString());
@@ -206,10 +192,13 @@ export class PRProvider implements vscode.TreeDataProvider<PRGroupTreeItem | Pul
 							let comments = sections[i];
 
 							const comment = comments[0];
+							let diffLine = getDiffLine(fileChange.patch, comment.position === null ? comment.original_position : comment.position);
+							let commentAbsolutePosition = 1;
+							if (diffLine) {
+								commentAbsolutePosition = diffLine.newLineNumber;
+							}
 							// If the position is null, the comment is on a line that has been changed. Fall back to using original position.
-							const commentPosition = comment.position === null ? comment.original_position : comment.position - 1;
-							const commentAbsolutePosition = comment.diff_hunk_range.start + commentPosition;
-							const pos = new vscode.Position(comment.currentPosition ? comment.currentPosition - 1 - 1 : commentAbsolutePosition - /* after line */ 1 - /* it's zero based*/ 1, 0);
+							const pos = new vscode.Position(commentAbsolutePosition - 1, 0);
 							const range = new vscode.Range(pos, pos);
 
 							threads.push({

extensions/git-extended/src/review/reviewMode.ts

@@ -7,7 +7,7 @@ import * as path from 'path';
 import * as vscode from 'vscode';
 import { Repository } from '../common/models/repository';
 import { FileChangeTreeItem } from '../common/treeItems';
-import { mapCommentsToHead, parseDiff } from '../common/diff';
+import { mapCommentsToHead, parseDiff, mapPositionHeadToDiffHunk, getDiffLine, parseDiffHunk, mapDiffHunkToHeadRange } from '../common/diff';
 import * as _ from 'lodash';
 import { GitContentProvider } from './gitContentProvider';
 import { Comment } from '../common/models/comment';
@@ -140,12 +140,38 @@ export class ReviewMode implements vscode.DecorationProvider {
 
 		this._command = vscode.commands.registerCommand(this._prNumber + '-post', async (uri: vscode.Uri, range: vscode.Range, thread: vscode.CommentThread, text: string) => {
 			try {
-				let ret = await pr.createCommentReply(text, thread.threadId);
-				return {
-					body: new vscode.MarkdownString(ret.data.body),
-					userName: ret.data.user.login,
-					gravatar: ret.data.user.avatar_url
-				};
+				if (thread && thread.threadId) {
+					let ret = await pr.createCommentReply(text, thread.threadId);
+					return {
+						commentId: ret.data.id,
+						body: new vscode.MarkdownString(ret.data.body),
+						userName: ret.data.user.login,
+						gravatar: ret.data.user.avatar_url
+					};
+				} else {
+					let fileName = uri.path;
+					let matchedFiles = this._localFileChanges.filter(fileChange => path.resolve(this._repository.path, fileChange.fileName) === fileName);
+					if (matchedFiles && matchedFiles.length) {
+						let matchedFile = matchedFiles[0];
+						// git diff sha -- fileName
+						let contentDiff = await this._repository.diff(matchedFile.fileName, this._lastCommitSha);
+						let position = mapPositionHeadToDiffHunk(matchedFile.patch, contentDiff, range.start.line);
+
+						if (position < 0) {
+							return;
+						}
+
+						// there is no thread Id, which means it's a new thread
+						let ret = await pr.createComment(text, matchedFile.fileName, position);
+
+						return {
+							commentId: ret.data.id,
+							body: new vscode.MarkdownString(ret.data.body),
+							userName: ret.data.user.login,
+							gravatar: ret.data.user.avatar_url
+						};
+					}
+				}
 			} catch (e) {
 				return null;
 			}
@@ -292,10 +318,7 @@ export class ReviewMode implements vscode.DecorationProvider {
 				let comments = sections[i];
 
 				const comment = comments[0];
-				// If the position is null, the comment is on a line that has been changed. Fall back to using original position.
-				const commentPosition = comment.position === null ? comment.original_position : comment.position - 1;
-				const commentAbsolutePosition = comment.diff_hunk_range.start + commentPosition;
-				const pos = new vscode.Position(comment.currentPosition ? comment.currentPosition - 1 - 1 : commentAbsolutePosition - /* after line */ 1 - /* it's zero based*/ 1, 0);
+				const pos = new vscode.Position(comment.absolutePosition ? comment.absolutePosition - 1 : 0, 0);
 				const range = new vscode.Range(pos, pos);
 
 				ret.push({
@@ -352,16 +375,33 @@ export class ReviewMode implements vscode.DecorationProvider {
 		this._documentCommentProvider = vscode.workspace.registerDocumentCommentProvider({
 			onDidChangeCommentThreads: this._onDidChangeCommentThreads.event,
 			provideDocumentComments: async (document: vscode.TextDocument, token: vscode.CancellationToken) => {
-				let lastLine = document.lineCount;
-				let lastColumn = document.lineAt(lastLine - 1).text.length;
-				let ranges = [
-					new vscode.Range(1, 1, lastLine, lastColumn)
-				];
+				let ranges: vscode.Range[] = [];
 
 				let matchingComments: Comment[];
 				if (document.uri.scheme === 'review') {
 					// from scm viewlet
 					matchingComments = this._commentsCache.get(document.uri.toString());
+					let matchedFiles = this._localFileChanges.filter(fileChange => path.resolve(this._repository.path, fileChange.fileName) === document.uri.toString());
+					if (matchedFiles && matchedFiles.length) {
+						let matchedFile = matchedFiles[0];
+
+						matchingComments.forEach(comment => {
+							let diffLine = getDiffLine(matchedFiles[0].patch, comment.position === null ? comment.original_position : comment.position);
+
+							if (diffLine) {
+								comment.absolutePosition = diffLine.newLineNumber;
+							}
+						});
+
+						let diffHunkReader = parseDiffHunk(matchedFile.patch);
+						let diffHunkIter = diffHunkReader.next();
+
+						while (!diffHunkIter.done) {
+							let diffHunk = diffHunkIter.value;
+							ranges.push(new vscode.Range(diffHunk.newLineNumber, 1, diffHunk.newLineNumber + diffHunk.newLength - 1, 1));
+							diffHunkIter = diffHunkReader.next();
+						}
+					}
 				} else if (document.uri.scheme === 'file') {
 					// local file
 					let fileName = document.uri.path;
@@ -371,7 +411,19 @@ export class ReviewMode implements vscode.DecorationProvider {
 						// git diff sha -- fileName
 						let contentDiff = await this._repository.diff(matchedFile.fileName, this._lastCommitSha);
 						matchingComments = this._comments.filter(comment => path.resolve(this._repository.path, comment.path) === fileName);
-						matchingComments = mapCommentsToHead(contentDiff, matchingComments);
+						matchingComments = mapCommentsToHead(matchedFile.patch, contentDiff, matchingComments);
+
+						let diffHunkReader = parseDiffHunk(matchedFile.patch);
+						let diffHunkIter = diffHunkReader.next();
+
+						while (!diffHunkIter.done) {
+							let diffHunk = diffHunkIter.value;
+							let mappedDiffHunk = mapDiffHunkToHeadRange(diffHunk, contentDiff);
+							if (mappedDiffHunk[0] >= 0 && mappedDiffHunk[1] >= 0) {
+								ranges.push(new vscode.Range(mappedDiffHunk[0], 1, mappedDiffHunk[1], 1));
+							}
+							diffHunkIter = diffHunkReader.next();
+						}
 					}
 				}
 

src/vs/workbench/parts/comments/electron-browser/commentsEditorContribution.ts

@@ -36,6 +36,7 @@ import { editorBackground, editorForeground } from 'vs/platform/theme/common/col
 import { ZoneWidget, IOptions } from 'vs/editor/contrib/zoneWidget/zoneWidget';
 import { ReviewModel, ReviewStyle } from 'vs/workbench/parts/comments/common/reviewModel';
 import { ICommentService } from '../../../services/comments/electron-browser/commentService';
+import { CommentThreadCollapsibleState } from '../../../api/node/extHostTypes';
 
 export const ctxReviewPanelVisible = new RawContextKey<boolean>('reviewPanelVisible', false);
 export const ID = 'editor.contrib.review';
@@ -316,12 +317,14 @@ export class ReviewZoneWidget extends ZoneWidget {
 				}
 			});
 
-
 			const formActions = $('.form-actions').appendTo(commentForm).getHTMLElement();
 
 			const button = $('button').appendTo(formActions).getHTMLElement();
 			button.onclick = async () => {
-				let newComment = await this.commandService.executeCommand(this._replyCommand.id, this.editor.getModel().uri, new Range(lineNumber, 1, lineNumber, 1), this._commentThread, textArea.value);
+				let newComment = await this.commandService.executeCommand(this._replyCommand.id, this.editor.getModel().uri, {
+					start: { line: lineNumber, column: 1 },
+					end: { line: lineNumber, column: 1 }
+				}, this._commentThread, textArea.value);
 				if (newComment) {
 					textArea.value = '';
 					this._commentThread.comments.push(newComment);
@@ -621,7 +624,7 @@ export class ReviewController implements IEditorContribution {
 			}
 		}
 
-		if (this._zoneWidget && this._zoneWidget.position.lineNumber === lineNumber) {
+		if (this._zoneWidget && this._zoneWidget.position && this._zoneWidget.position.lineNumber === lineNumber) {
 			return;
 		}
 
@@ -643,7 +646,8 @@ export class ReviewController implements IEditorContribution {
 					endLineNumber: lineNumber,
 					endColumn: 0
 				},
-				reply: null
+				reply: newCommentInfo[0],
+				collapsibleState: CommentThreadCollapsibleState.Expanded,
 				// actions: newCommentAction.actions
 			}, newCommentInfo[0], {}, this.themeService, this.commandService);
 
@@ -651,7 +655,6 @@ export class ReviewController implements IEditorContribution {
 				this._zoneWidget = null;
 			});
 			this._zoneWidget.display(lineNumber);
-			this._zoneWidget.toggleExpand();
 		}
 	}