Better shifting of comment threads when editor changes (microsoft#227246

)

Part of microsoft/vscode-pull-request-github#6171

src/vs/editor/common/languages.ts

@@ -1895,7 +1895,6 @@ export interface CommentThread<T = IRange> {
 	canReply: boolean;
 	input?: CommentInput;
 	onDidChangeInput: Event<CommentInput | undefined>;
-	onDidChangeRange: Event<T | undefined>;
 	onDidChangeLabel: Event<string | undefined>;
 	onDidChangeCollapsibleState: Event<CommentThreadCollapsibleState | undefined>;
 	onDidChangeState: Event<CommentThreadState | undefined>;

src/vs/workbench/api/browser/mainThreadComments.ts

@@ -85,7 +85,6 @@ export class MainThreadCommentThread<T> implements languages.CommentThread<T> {
 
 	set range(range: T | undefined) {
 		this._range = range;
-		this._onDidChangeRange.fire(this._range);
 	}
 
 	get range(): T | undefined {
@@ -103,9 +102,6 @@ export class MainThreadCommentThread<T> implements languages.CommentThread<T> {
 		return this._canReply;
 	}
 
-	private readonly _onDidChangeRange = new Emitter<T | undefined>();
-	public onDidChangeRange = this._onDidChangeRange.event;
-
 	private _collapsibleState: languages.CommentThreadCollapsibleState | undefined;
 	get collapsibleState() {
 		return this._collapsibleState;
@@ -222,7 +218,6 @@ export class MainThreadCommentThread<T> implements languages.CommentThread<T> {
 		this._onDidChangeComments.dispose();
 		this._onDidChangeInput.dispose();
 		this._onDidChangeLabel.dispose();
-		this._onDidChangeRange.dispose();
 		this._onDidChangeState.dispose();
 	}
 

src/vs/workbench/contrib/comments/browser/commentGlyphWidget.ts

@@ -12,6 +12,8 @@ import { darken, editorBackground, editorForeground, listInactiveSelectionBackgr
 import { themeColorFromId } from '../../../../platform/theme/common/themeService.js';
 import { IEditorDecorationsCollection } from '../../../../editor/common/editorCommon.js';
 import { CommentThreadState } from '../../../../editor/common/languages.js';
+import { Disposable, toDisposable } from '../../../../base/common/lifecycle.js';
+import { Emitter } from '../../../../base/common/event.js';
 
 export const overviewRulerCommentingRangeForeground = registerColor('editorGutter.commentRangeForeground', { dark: opaque(listInactiveSelectionBackground, editorBackground), light: darken(opaque(listInactiveSelectionBackground, editorBackground), .05), hcDark: Color.white, hcLight: Color.black }, nls.localize('editorGutterCommentRangeForeground', 'Editor gutter decoration color for commenting ranges. This color should be opaque.'));
 const overviewRulerCommentForeground = registerColor('editorOverviewRuler.commentForeground', overviewRulerCommentingRangeForeground, nls.localize('editorOverviewRuler.commentForeground', 'Editor overview ruler decoration color for resolved comments. This color should be opaque.'));
@@ -20,18 +22,30 @@ const overviewRulerCommentUnresolvedForeground = registerColor('editorOverviewRu
 const editorGutterCommentGlyphForeground = registerColor('editorGutter.commentGlyphForeground', { dark: editorForeground, light: editorForeground, hcDark: Color.black, hcLight: Color.white }, nls.localize('editorGutterCommentGlyphForeground', 'Editor gutter decoration color for commenting glyphs.'));
 registerColor('editorGutter.commentUnresolvedGlyphForeground', editorGutterCommentGlyphForeground, nls.localize('editorGutterCommentUnresolvedGlyphForeground', 'Editor gutter decoration color for commenting glyphs for unresolved comment threads.'));
 
-export class CommentGlyphWidget {
+export class CommentGlyphWidget extends Disposable {
 	public static description = 'comment-glyph-widget';
 	private _lineNumber!: number;
 	private _editor: ICodeEditor;
 	private _threadState: CommentThreadState | undefined;
 	private readonly _commentsDecorations: IEditorDecorationsCollection;
 	private _commentsOptions: ModelDecorationOptions;
 
+	private readonly _onDidChangeLineNumber = this._register(new Emitter<number>());
+	public readonly onDidChangeLineNumber = this._onDidChangeLineNumber.event;
+
 	constructor(editor: ICodeEditor, lineNumber: number) {
+		super();
 		this._commentsOptions = this.createDecorationOptions();
 		this._editor = editor;
 		this._commentsDecorations = this._editor.createDecorationsCollection();
+		this._register(this._commentsDecorations.onDidChange(e => {
+			const range = (this._commentsDecorations.length > 0 ? this._commentsDecorations.getRange(0) : null);
+			if (range && range.endLineNumber !== this._lineNumber) {
+				this._lineNumber = range.endLineNumber;
+				this._onDidChangeLineNumber.fire(this._lineNumber);
+			}
+		}));
+		this._register(toDisposable(() => this._commentsDecorations.clear()));
 		this.setLineNumber(lineNumber);
 	}
 
@@ -87,8 +101,4 @@ export class CommentGlyphWidget {
 			preference: [ContentWidgetPositionPreference.EXACT]
 		};
 	}
-
-	dispose() {
-		this._commentsDecorations.clear();
-	}
 }

src/vs/workbench/contrib/comments/browser/commentThreadZoneWidget.ts

@@ -373,6 +373,14 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
 		if (range) {
 			this._commentGlyph = new CommentGlyphWidget(this.editor, range?.endLineNumber ?? -1);
 			this._commentGlyph.setThreadState(this._commentThread.state);
+			this._globalToDispose.add(this._commentGlyph.onDidChangeLineNumber(async e => {
+				if (!this._commentThread.range) {
+					return;
+				}
+				const shift = e - (this._commentThread.range.endLineNumber);
+				const newRange = new Range(this._commentThread.range.startLineNumber + shift, this._commentThread.range.startColumn, this._commentThread.range.endLineNumber + shift, this._commentThread.range.endColumn);
+				this._commentThread.range = newRange;
+			}));
 		}
 
 		await this._commentThreadWidget.display(this.editor.getOption(EditorOption.lineHeight), shouldReveal);
@@ -396,22 +404,6 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
 			await this.update(this._commentThread);
 		}));
 
-		this._commentThreadDisposables.push(this._commentThread.onDidChangeRange(range => {
-			// Move comment glyph widget and show position if the line has changed.
-			const lineNumber = this._commentThread.range?.startLineNumber ?? 1;
-			let shouldMoveWidget = false;
-			if (this._commentGlyph) {
-				if (this._commentGlyph.getPosition().position!.lineNumber !== lineNumber) {
-					shouldMoveWidget = true;
-					this._commentGlyph.setLineNumber(lineNumber);
-				}
-			}
-
-			if (shouldMoveWidget && this._isExpanded) {
-				this.show(this.arrowPosition(this._commentThread.range), 2);
-			}
-		}));
-
 		this._commentThreadDisposables.push(this._commentThread.onDidChangeCollapsibleState(state => {
 			if (state === languages.CommentThreadCollapsibleState.Expanded && !this._isExpanded) {
 				this.show(this.arrowPosition(this._commentThread.range), 2);