Feature #94173: Scroll when inserting suggestion#94327
Feature #94173: Scroll when inserting suggestion#94327jrieken merged 6 commits intomicrosoft:masterfrom
Conversation
| this._memoryService.memorize(model, this.editor.getPosition(), item); | ||
|
|
||
| // keep line number for scrolling | ||
| const initialLineNumber = this.editor.getPosition().lineNumber; |
There was a problem hiding this comment.
You should be using this StableEditorScrollState
There was a problem hiding this comment.
@jrieken
Thanks for your review. I've update the PR.
fed8165 to
8a4b4e9
Compare
|
Adding @alexdima to review the changes in the scroll state. Tested this, it kinda works. When inserting import-completion by current line doesn't move anymore, but when undoing it seems to shift up and that's not nice
|
|
@jrieken
|
alexdima
left a comment
There was a problem hiding this comment.
LGTM, except the pushUndoStop call in restoreRelativeVerticalPositionOfCursor
alexdima
left a comment
There was a problem hiding this comment.
There is still a call to editor.pushUndoStop in restoreRelativeVerticalPositionOfCursor. It needs to be removed from there, as restoring the scrolling position should not affect the undo stack in any way.
|
@alexdima |
| }); | ||
|
|
||
| scrollState.restoreRelativeVerticalPositionOfCursor(this.editor); | ||
| this.editor.pushUndoStop(); |
There was a problem hiding this comment.
Why are you adding an undo-stop at all?
There was a problem hiding this comment.
@jrieken
I'm sorry, I misunderstood. I'll remove it.
|
The scroll jump after undo still happens (#94327 (comment)). Adding another undo-stop is not the right approach here but I also don't know what to do. @alexdima can you advice? We now control the scroll state when doing the suggest edits but I don't thing we have a say when undoing these changes. |
|
Effectively, when doing and inserting an import, the editor is scrolled down by 1 line ( The undo does not capture the That is just how undo/redo works currently, and is not 100% awesome for this specific case, and we can make new issues about that if we want to, but I don't know how to tackle it, nor do I think that it should block this PR... |
This PR fixes #94173