Typing in a comment, switching editors and switching back will not restore the cursor position in the comment box (#230466)

Fixes #229214
This commit is contained in:
Alex Ross
2024-10-04 10:40:52 +02:00
committed by GitHub
parent 83e44613e4
commit ce7b68b48d
8 changed files with 72 additions and 54 deletions
+6 -1
View File
@@ -2001,11 +2001,16 @@ export interface Comment {
}
export interface PendingCommentThread {
body: string;
range: IRange | undefined;
uri: URI;
uniqueOwner: string;
isReply: boolean;
comment: PendingComment;
}
export interface PendingComment {
body: string;
cursor: IPosition;
}
/**
+6 -1
View File
@@ -7999,11 +7999,16 @@ declare namespace monaco.languages {
}
export interface PendingCommentThread {
body: string;
range: IRange | undefined;
uri: Uri;
uniqueOwner: string;
isReply: boolean;
comment: PendingComment;
}
export interface PendingComment {
body: string;
cursor: IPosition;
}
export interface CodeLens {
@@ -14,7 +14,6 @@ import { MarkdownRenderer } from '../../../../editor/browser/widget/markdownRend
import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js';
import { ICommentService } from './commentService.js';
import { LayoutableEditor, MIN_EDITOR_HEIGHT, SimpleCommentEditor, calculateEditorHeight } from './simpleCommentEditor.js';
import { Selection } from '../../../../editor/common/core/selection.js';
import { Emitter, Event } from '../../../../base/common/event.js';
import { INotificationService } from '../../../../platform/notification/common/notification.js';
import { ToolBar } from '../../../../base/browser/ui/toolbar/toolbar.js';
@@ -50,6 +49,7 @@ import { IKeybindingService } from '../../../../platform/keybinding/common/keybi
import { MarshalledCommentThread } from '../../../common/comments.js';
import { IHoverService } from '../../../../platform/hover/browser/hover.js';
import { IResolvedTextEditorModel, ITextModelService } from '../../../../editor/common/services/resolverService.js';
import { Position } from '../../../../editor/common/core/position.js';
class CommentsActionRunner extends ActionRunner {
protected override async runAction(action: IAction, context: any[]): Promise<void> {
@@ -103,7 +103,7 @@ export class CommentNode<T extends IRange | ICellRange> extends Disposable {
private readonly parentEditor: LayoutableEditor,
private commentThread: languages.CommentThread<T>,
public comment: languages.Comment,
private pendingEdit: string | undefined,
private pendingEdit: languages.PendingComment | undefined,
private owner: string,
private resource: URI,
private parentThread: ICommentThreadWidget,
@@ -503,7 +503,14 @@ export class CommentNode<T extends IRange | ICellRange> extends Disposable {
this._commentEditorModel = modelRef;
this._commentEditor.setModel(this._commentEditorModel.object.textEditorModel);
this._commentEditor.setValue(this.pendingEdit ?? this.commentBodyValue);
this._commentEditor.setValue(this.pendingEdit?.body ?? this.commentBodyValue);
if (this.pendingEdit) {
this._commentEditor.setPosition(this.pendingEdit.cursor);
} else {
const lastLine = this._commentEditorModel.object.textEditorModel.getLineCount();
const lastColumn = this._commentEditorModel.object.textEditorModel.getLineLength(lastLine) + 1;
this._commentEditor.setPosition(new Position(lastLine, lastColumn));
}
this.pendingEdit = undefined;
this._commentEditor.layout({ width: container.clientWidth - 14, height: this._editorHeight });
this._commentEditor.focus();
@@ -513,10 +520,6 @@ export class CommentNode<T extends IRange | ICellRange> extends Disposable {
this._commentEditor!.focus();
});
const lastLine = this._commentEditorModel.object.textEditorModel.getLineCount();
const lastColumn = this._commentEditorModel.object.textEditorModel.getLineLength(lastLine) + 1;
this._commentEditor.setSelection(new Selection(lastLine, lastColumn, lastLine, lastColumn));
const commentThread = this.commentThread;
commentThread.input = {
uri: this._commentEditor.getModel()!.uri,
@@ -571,10 +574,10 @@ export class CommentNode<T extends IRange | ICellRange> extends Disposable {
return false;
}
getPendingEdit(): string | undefined {
getPendingEdit(): languages.PendingComment | undefined {
const model = this._commentEditor?.getModel();
if (model && model.getValueLength() > 0) {
return model.getValue();
if (this._commentEditor && model && model.getValueLength() > 0) {
return { body: model.getValue(), cursor: this._commentEditor.getPosition()! };
}
return undefined;
}
@@ -31,6 +31,7 @@ import { ICellRange } from '../../notebook/common/notebookRange.js';
import { LayoutableEditor, MIN_EDITOR_HEIGHT, SimpleCommentEditor, calculateEditorHeight } from './simpleCommentEditor.js';
import { IHoverService } from '../../../../platform/hover/browser/hover.js';
import { IContextMenuService } from '../../../../platform/contextview/browser/contextView.js';
import { Position } from '../../../../editor/common/core/position.js';
let INMEM_MODEL_ID = 0;
export const COMMENTEDITOR_DECORATION_KEY = 'commenteditordecoration';
@@ -57,7 +58,7 @@ export class CommentReply<T extends IRange | ICellRange> extends Disposable {
private _contextKeyService: IContextKeyService,
private _commentMenus: CommentMenus,
private _commentOptions: languages.CommentOptions | undefined,
private _pendingComment: string | undefined,
private _pendingComment: languages.PendingComment | undefined,
private _parentThread: ICommentThreadWidget,
focus: boolean,
private _actionRunDelegate: (() => void) | null,
@@ -96,10 +97,13 @@ export class CommentReply<T extends IRange | ICellRange> extends Disposable {
}
const model = await this.textModelService.createModelReference(resource);
model.object.textEditorModel.setValue(this._pendingComment || '');
model.object.textEditorModel.setValue(this._pendingComment?.body || '');
this._register(model);
this.commentEditor.setModel(model.object.textEditorModel);
if (this._pendingComment) {
this.commentEditor.setPosition(this._pendingComment.cursor);
}
this.calculateEditorHeight();
this._register(model.object.textEditorModel.onDidChangeContent(() => {
@@ -157,20 +161,21 @@ export class CommentReply<T extends IRange | ICellRange> extends Disposable {
}
}
public getPendingComment(): string | undefined {
public getPendingComment(): languages.PendingComment | undefined {
const model = this.commentEditor.getModel();
if (model && model.getValueLength() > 0) { // checking length is cheap
return model.getValue();
return { body: model.getValue(), cursor: this.commentEditor.getPosition() ?? new Position(1, 1) };
}
return undefined;
}
public setPendingComment(comment: string) {
this._pendingComment = comment;
public setPendingComment(pending: languages.PendingComment) {
this._pendingComment = pending;
this.expandReplyArea();
this.commentEditor.setValue(comment);
this.commentEditor.setValue(pending.body);
this.commentEditor.setPosition(pending.cursor);
}
public layout(widthInPixel: number) {
@@ -254,7 +259,7 @@ export class CommentReply<T extends IRange | ICellRange> extends Disposable {
commentEditor.setValue(input.value);
if (input.value === '') {
this._pendingComment = '';
this._pendingComment = { body: '', cursor: new Position(1, 1) };
commentForm.classList.remove('expand');
commentEditor.getDomNode()!.style.outline = '';
this._error.textContent = '';
@@ -339,7 +344,7 @@ export class CommentReply<T extends IRange | ICellRange> extends Disposable {
domNode.style.outline = '';
}
this.commentEditor.setValue('');
this._pendingComment = '';
this._pendingComment = { body: '', cursor: new Position(1, 1) };
this.form.classList.remove('expand');
this._error.textContent = '';
this._error.classList.add('hidden');
@@ -49,7 +49,7 @@ export class CommentThreadBody<T extends IRange | ICellRange = IRange> extends D
readonly container: HTMLElement,
private _options: IMarkdownRendererOptions,
private _commentThread: languages.CommentThread<T>,
private _pendingEdits: { [key: number]: string } | undefined,
private _pendingEdits: { [key: number]: languages.PendingComment } | undefined,
private _scopedInstatiationService: IInstantiationService,
private _parentCommentThreadWidget: ICommentThreadWidget,
@ICommentService private commentService: ICommentService,
@@ -142,8 +142,8 @@ export class CommentThreadBody<T extends IRange | ICellRange = IRange> extends D
});
}
getPendingEdits(): { [key: number]: string } {
const pendingEdits: { [key: number]: string } = {};
getPendingEdits(): { [key: number]: languages.PendingComment } {
const pendingEdits: { [key: number]: languages.PendingComment } = {};
this._commentElements.forEach(element => {
if (element.isEditing) {
const pendingEdit = element.getPendingEdit();
@@ -68,8 +68,8 @@ export class CommentThreadWidget<T extends IRange | ICellRange = IRange> extends
private _contextKeyService: IContextKeyService,
private _scopedInstantiationService: IInstantiationService,
private _commentThread: languages.CommentThread<T>,
private _pendingComment: string | undefined,
private _pendingEdits: { [key: number]: string } | undefined,
private _pendingComment: languages.PendingComment | undefined,
private _pendingEdits: { [key: number]: languages.PendingComment } | undefined,
private _markdownOptions: IMarkdownRendererOptions,
private _commentOptions: languages.CommentOptions | undefined,
private _containerDelegate: {
@@ -333,11 +333,11 @@ export class CommentThreadWidget<T extends IRange | ICellRange = IRange> extends
return this._body.getCommentCoords(commentUniqueId);
}
getPendingEdits(): { [key: number]: string } {
getPendingEdits(): { [key: number]: languages.PendingComment } {
return this._body.getPendingEdits();
}
getPendingComment(): string | undefined {
getPendingComment(): languages.PendingComment | undefined {
if (this._commentReply) {
return this._commentReply.getPendingComment();
}
@@ -345,9 +345,9 @@ export class CommentThreadWidget<T extends IRange | ICellRange = IRange> extends
return undefined;
}
setPendingComment(comment: string) {
this._pendingComment = comment;
this._commentReply?.setPendingComment(comment);
setPendingComment(pending: languages.PendingComment) {
this._pendingComment = pending;
this._commentReply?.setPendingComment(pending);
}
getDimensions() {
@@ -378,7 +378,7 @@ export class CommentThreadWidget<T extends IRange | ICellRange = IRange> extends
const activeComment = this._body.activeComment;
if (activeComment) {
return activeComment.submitComment();
} else if ((this._commentReply?.getPendingComment()?.length ?? 0) > 0) {
} else if ((this._commentReply?.getPendingComment()?.body.length ?? 0) > 0) {
return this._commentReply?.submitComment();
}
}
@@ -128,8 +128,8 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
editor: ICodeEditor,
private _uniqueOwner: string,
private _commentThread: languages.CommentThread,
private _pendingComment: string | undefined,
private _pendingEdits: { [key: number]: string } | undefined,
private _pendingComment: languages.PendingComment | undefined,
private _pendingEdits: { [key: number]: languages.PendingComment } | undefined,
@IInstantiationService instantiationService: IInstantiationService,
@IThemeService private themeService: IThemeService,
@ICommentService private commentService: ICommentService,
@@ -242,17 +242,17 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
}
}
public getPendingComments(): { newComment: string | undefined; edits: { [key: number]: string } } {
public getPendingComments(): { newComment: languages.PendingComment | undefined; edits: { [key: number]: languages.PendingComment } } {
return {
newComment: this._commentThreadWidget.getPendingComment(),
edits: this._commentThreadWidget.getPendingEdits()
};
}
public setPendingComment(comment: string) {
this._pendingComment = comment;
public setPendingComment(pending: languages.PendingComment) {
this._pendingComment = pending;
this.expand();
this._commentThreadWidget.setPendingComment(comment);
this._commentThreadWidget.setPendingComment(pending);
}
protected _fillContainer(container: HTMLElement): void {
@@ -464,8 +464,8 @@ export class CommentController implements IEditorContribution {
private _emptyThreadsToAddQueue: [Range | undefined, IEditorMouseEvent | undefined][] = [];
private _computeCommentingRangePromise!: CancelablePromise<ICommentInfo[]> | null;
private _computeCommentingRangeScheduler!: Delayer<Array<ICommentInfo | null>> | null;
private _pendingNewCommentCache: { [key: string]: { [key: string]: string } };
private _pendingEditsCache: { [key: string]: { [key: string]: { [key: number]: string } } }; // uniqueOwner -> threadId -> uniqueIdInThread -> pending comment
private _pendingNewCommentCache: { [key: string]: { [key: string]: languages.PendingComment } };
private _pendingEditsCache: { [key: string]: { [key: string]: { [key: number]: languages.PendingComment } } }; // uniqueOwner -> threadId -> uniqueIdInThread -> pending comment
private _inProcessContinueOnComments: Map<string, languages.PendingCommentThread[]> = new Map();
private _editorDisposables: IDisposable[] = [];
private _activeCursorHasCommentingRange: IContextKey<boolean>;
@@ -578,12 +578,12 @@ export class CommentController implements IEditorContribution {
}
}
if (pendingNewComment !== lastCommentBody) {
if (pendingNewComment.body !== lastCommentBody) {
pendingComments.push({
uniqueOwner: zone.uniqueOwner,
uri: zone.editor.getModel()!.uri,
range: zone.commentThread.range,
body: pendingNewComment,
comment: pendingNewComment,
isReply: (zone.commentThread.comments !== undefined) && (zone.commentThread.comments.length > 0)
});
}
@@ -896,7 +896,7 @@ export class CommentController implements IEditorContribution {
});
let continueOnCommentText: string | undefined;
if ((continueOnCommentIndex !== undefined) && continueOnCommentIndex >= 0) {
continueOnCommentText = this._inProcessContinueOnComments.get(uniqueOwner)?.splice(continueOnCommentIndex, 1)[0].body;
continueOnCommentText = this._inProcessContinueOnComments.get(uniqueOwner)?.splice(continueOnCommentIndex, 1)[0].comment.body;
}
const pendingCommentText = (this._pendingNewCommentCache[uniqueOwner] && this._pendingNewCommentCache[uniqueOwner][thread.threadId])
@@ -999,18 +999,18 @@ export class CommentController implements IEditorContribution {
const matchedZones = this._commentWidgets.filter(zoneWidget => zoneWidget.uniqueOwner === thread.uniqueOwner && Range.lift(zoneWidget.commentThread.range)?.equalsRange(thread.range));
if (thread.isReply && matchedZones.length) {
this.commentService.removeContinueOnComment({ uniqueOwner: thread.uniqueOwner, uri: editorURI, range: thread.range, isReply: true });
matchedZones[0].setPendingComment(thread.body);
matchedZones[0].setPendingComment(thread.comment);
} else if (matchedZones.length) {
this.commentService.removeContinueOnComment({ uniqueOwner: thread.uniqueOwner, uri: editorURI, range: thread.range, isReply: false });
const existingPendingComment = matchedZones[0].getPendingComments().newComment;
// We need to try to reconcile the existing pending comment with the incoming pending comment
let pendingComment: string;
if (!existingPendingComment || thread.body.includes(existingPendingComment)) {
pendingComment = thread.body;
} else if (existingPendingComment.includes(thread.body)) {
let pendingComment: languages.PendingComment;
if (!existingPendingComment || thread.comment.body.includes(existingPendingComment.body)) {
pendingComment = thread.comment;
} else if (existingPendingComment.body.includes(thread.comment.body)) {
pendingComment = existingPendingComment;
} else {
pendingComment = `${existingPendingComment}\n${thread.body}`;
pendingComment = { body: `${existingPendingComment}\n${thread.comment.body}`, cursor: thread.comment.cursor };
}
matchedZones[0].setPendingComment(pendingComment);
} else if (!thread.isReply) {
@@ -1062,7 +1062,7 @@ export class CommentController implements IEditorContribution {
return undefined;
}
private async displayCommentThread(uniqueOwner: string, thread: languages.CommentThread, shouldReveal: boolean, pendingComment: string | undefined, pendingEdits: { [key: number]: string } | undefined): Promise<void> {
private async displayCommentThread(uniqueOwner: string, thread: languages.CommentThread, shouldReveal: boolean, pendingComment: languages.PendingComment | undefined, pendingEdits: { [key: number]: languages.PendingComment } | undefined): Promise<void> {
const editor = this.editor?.getModel();
if (!editor) {
return;
@@ -1075,7 +1075,7 @@ export class CommentController implements IEditorContribution {
if (thread.range && !pendingComment) {
continueOnCommentReply = this.commentService.removeContinueOnComment({ uniqueOwner, uri: editor.uri, range: thread.range, isReply: true });
}
const zoneWidget = this.instantiationService.createInstance(ReviewZoneWidget, this.editor, uniqueOwner, thread, pendingComment ?? continueOnCommentReply?.body, pendingEdits);
const zoneWidget = this.instantiationService.createInstance(ReviewZoneWidget, this.editor, uniqueOwner, thread, pendingComment ?? continueOnCommentReply?.comment, pendingEdits);
await zoneWidget.display(thread.range, shouldReveal);
this._commentWidgets.push(zoneWidget);
this.openCommentsView(thread);
@@ -1361,12 +1361,12 @@ export class CommentController implements IEditorContribution {
const providerEditsCacheStore = this._pendingEditsCache[info.uniqueOwner];
info.threads = info.threads.filter(thread => !thread.isDisposed);
for (const thread of info.threads) {
let pendingComment: string | undefined = undefined;
let pendingComment: languages.PendingComment | undefined = undefined;
if (providerCacheStore) {
pendingComment = providerCacheStore[thread.threadId];
}
let pendingEdits: { [key: number]: string } | undefined = undefined;
let pendingEdits: { [key: number]: languages.PendingComment } | undefined = undefined;
if (providerEditsCacheStore) {
pendingEdits = providerEditsCacheStore[thread.threadId];
}
@@ -1412,7 +1412,7 @@ export class CommentController implements IEditorContribution {
lastCommentBody = lastComment.body.value;
}
}
if (pendingNewComment && (pendingNewComment !== lastCommentBody)) {
if (pendingNewComment && (pendingNewComment.body !== lastCommentBody)) {
if (!providerNewCommentCacheStore) {
this._pendingNewCommentCache[zone.uniqueOwner] = {};
}