Fix new comment position and event leak

Use the current comment position when creating
a new thread, not the one that was
when user started editing the comment,
because the position may change, e.g. if the user
added new lines before the comment.

Keep the editor selection when comments are
updated via documment comment provider
event so user flow is not disturbed
when comments are updated.

Add button styler to local disposables to
fix event leak.

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

@@ -23,9 +23,8 @@ export class CommentGlyphWidget {
 
 	constructor(editor: ICodeEditor, lineNumber: number) {
 		this._commentsOptions = this.createDecorationOptions();
-		this._lineNumber = lineNumber;
 		this._editor = editor;
-		this.update();
+		this.setLineNumber(lineNumber);
 	}
 
 	private createDecorationOptions(): ModelDecorationOptions {
@@ -41,11 +40,12 @@ export class CommentGlyphWidget {
 		return ModelDecorationOptions.createDynamic(decorationOptions);
 	}
 
-	update() {
+	setLineNumber(lineNumber: number): void {
+		this._lineNumber = lineNumber;
 		let commentsDecorations = [{
 			range: {
-				startLineNumber: this._lineNumber, startColumn: 1,
-				endLineNumber: this._lineNumber, endColumn: 1
+				startLineNumber: lineNumber, startColumn: 1,
+				endLineNumber: lineNumber, endColumn: 1
 			},
 			options: this._commentsOptions
 		}];
@@ -53,15 +53,14 @@ export class CommentGlyphWidget {
 		this.commentsDecorations = this._editor.deltaDecorations(this.commentsDecorations, commentsDecorations);
 	}
 
-	setLineNumber(lineNumber: number): void {
-		this._lineNumber = lineNumber;
-		this.update();
-	}
-
 	getPosition(): IContentWidgetPosition {
+		const range = this._editor.hasModel() && this.commentsDecorations && this.commentsDecorations.length
+			? this._editor.getModel().getDecorationRange(this.commentsDecorations[0])
+			: null;
+
 		return {
 			position: {
-				lineNumber: this._lineNumber,
+				lineNumber: range ? range.startLineNumber : this._lineNumber,
 				column: 1
 			},
 			preference: [ContentWidgetPositionPreference.EXACT]
@@ -73,4 +72,4 @@ export class CommentGlyphWidget {
 			this._editor.deltaDecorations(this.commentsDecorations, []);
 		}
 	}
-}
\ No newline at end of file
+}

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

@@ -98,7 +98,7 @@ export class ReviewZoneWidget extends ZoneWidget {
 		commentThread: modes.CommentThread,
 		pendingComment: string,
 		draftMode: modes.DraftMode,
-		options: IOptions = {}
+		options: IOptions = { keepEditorSelection: true }
 	) {
 		super(editor, options);
 		this._resizeObserver = null;
@@ -270,9 +270,10 @@ export class ReviewZoneWidget extends ZoneWidget {
 		const lineNumber = this._commentThread.range.startLineNumber;
 		if (this._commentGlyph.getPosition().position.lineNumber !== lineNumber) {
 			this._commentGlyph.setLineNumber(lineNumber);
-			if (!this._isCollapsed) {
-				this.show({ lineNumber, column: 1 }, 2);
-			}
+		}
+
+		if (!this._isCollapsed) {
+			this.show({ lineNumber, column: 1 }, 2);
 		}
 	}
 
@@ -383,7 +384,7 @@ export class ReviewZoneWidget extends ZoneWidget {
 
 	private createCommentWidgetActions(container: HTMLElement, model: ITextModel) {
 		const button = new Button(container);
-		attachButtonStyler(button, this.themeService);
+		this._localToDispose.push(attachButtonStyler(button, this.themeService));
 		button.label = 'Add comment';
 
 		button.enabled = model.getValueLength() > 0;

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

@@ -389,7 +389,7 @@ export class ReviewController implements IEditorContribution {
 				}
 			});
 			added.forEach(thread => {
-				let zoneWidget = new ReviewZoneWidget(this.instantiationService, this.modeService, this.modelService, this.themeService, this.commentService, this.openerService, this.dialogService, this.notificationService, this.editor, e.owner, thread, null, draftMode, {});
+				let zoneWidget = new ReviewZoneWidget(this.instantiationService, this.modeService, this.modelService, this.themeService, this.commentService, this.openerService, this.dialogService, this.notificationService, this.editor, e.owner, thread, null, draftMode);
 				zoneWidget.display(thread.range.startLineNumber);
 				this._commentWidgets.push(zoneWidget);
 				this._commentInfos.filter(info => info.owner === e.owner)[0].threads.push(thread);
@@ -420,7 +420,7 @@ export class ReviewController implements IEditorContribution {
 			},
 			reply: replyCommand,
 			collapsibleState: CommentThreadCollapsibleState.Expanded,
-		}, pendingComment, draftMode, {});
+		}, pendingComment, draftMode);
 
 		this.localToDispose.push(this._newCommentWidget.onDidClose(e => {
 			this.clearNewCommentWidget();
@@ -565,7 +565,7 @@ export class ReviewController implements IEditorContribution {
 					thread.collapsibleState = modes.CommentThreadCollapsibleState.Expanded;
 				}
 
-				let zoneWidget = new ReviewZoneWidget(this.instantiationService, this.modeService, this.modelService, this.themeService, this.commentService, this.openerService, this.dialogService, this.notificationService, this.editor, info.owner, thread, pendingComment, info.draftMode, {});
+				let zoneWidget = new ReviewZoneWidget(this.instantiationService, this.modeService, this.modelService, this.themeService, this.commentService, this.openerService, this.dialogService, this.notificationService, this.editor, info.owner, thread, pendingComment, info.draftMode);
 				zoneWidget.display(thread.range.startLineNumber);
 				this._commentWidgets.push(zoneWidget);
 			});