Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Editor properties delivered before document changes to ext host #101956

Closed
roblourens opened this issue Jul 8, 2020 · 3 comments
Closed

Editor properties delivered before document changes to ext host #101956

roblourens opened this issue Jul 8, 2020 · 3 comments
Assignees
@jrieken @alexdima
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Milestone
July 2020

Comments

@roblourens
Copy link
Member

Saw this with notebooks but I can repro it in a normal editor.

  • Open a markdown file
  • Split the editor
  • cmd+a
  • duplicate lines down, or paste a large amount of content at the bottom of the file
  • See this stack from the markdown extension
Error: Illegal value for `line`
	at ExtHostDocumentData._lineAt (/Users/roblou/code/vscode/out/vs/workbench/api/common/extHostDocumentData.js:106:23)
	at Object.lineAt (/Users/roblou/code/vscode/out/vs/workbench/api/common/extHostDocumentData.js:57:53)
	at getVisibleLine (/Users/roblou/code/vscode/extensions/markdown-language-features/out/util/topmostLineMonitor.js:57:34)
	at /Users/roblou/code/vscode/extensions/markdown-language-features/out/util/topmostLineMonitor.js:20:30
	at Emitter.fire (/Users/roblou/code/vscode/out/vs/base/common/event.js:456:38)
	at ExtHostEditors.$acceptEditorPropertiesChanged (/Users/roblou/code/vscode/out/vs/workbench/api/common/extHostTextEditors.js:112:58)
	at RPCProtocol._doInvokeHandler (/Users/roblou/code/vscode/out/vs/workbench/services/extensions/common/rpcProtocol.js:335:27)
	at RPCProtocol._invokeHandler (/Users/roblou/code/vscode/out/vs/workbench/services/extensions/common/rpcProtocol.js:320:45)
	at RPCProtocol._receiveRequest (/Users/roblou/code/vscode/out/vs/workbench/services/extensions/common/rpcProtocol.js:247:32)
	at RPCProtocol._receiveOneMessage (/Users/roblou/code/vscode/out/vs/workbench/services/extensions/common/rpcProtocol.js:175:26)
	at /Users/roblou/code/vscode/out/vs/workbench/services/extensions/common/rpcProtocol.js:65:52
	at Emitter.fire (/Users/roblou/code/vscode/out/vs/base/common/event.js:456:38)

Seems that the markdown extension listens to onDidChangeTextEditorVisibleRanges and does

	const firstVisiblePosition = editor.visibleRanges[0].start;
	const lineNumber = firstVisiblePosition.line;
	const line = editor.document.lineAt(lineNumber);

When this is called, the document does not know about the new content, but editor.visibleRanges has been adjusted for the new content. So if the top visible line is in the range of the new content, it will throw. Debugging, it seems like
$acceptEditorPropertiesChanged is sent before $acceptModelChanged, which would explain why they are out of sync.
@rebornix rebornix assigned jrieken and alexdima and unassigned rebornix Jul 8, 2020
@rebornix
Copy link
Member

rebornix commented Jul 8, 2020

I know @alexdima and @jrieken worked hard before to make $acceptEditorPropertiesChanged correct.

@jrieken jrieken added the bug Issue identified by VS Code Team member as probable bug label Jul 9, 2020
@jrieken jrieken added this to the July 2020 milestone Jul 9, 2020
@jrieken
Copy link
Member

jrieken commented Jul 9, 2020

Great find. Thanks @roblourens

@alexdima
Copy link
Member

alexdima commented Jul 9, 2020

Great catch!

@alexdima alexdima changed the title Ext host doc model out of sync with editor properties for split editor Editor properties delivered before document changes to ext host Jul 9, 2020
@roblourens roblourens added the verified Verification succeeded label Aug 6, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Aug 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jrieken jrieken

@alexdima alexdima

Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Projects
None yet
Milestone
July 2020
Development

No branches or pull requests
5 participants
@roblourens @rebornix @jrieken @alexdima and others