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.

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

Using applyEdit with replaceCellMetadata or replaceCellOutput fails with no output #105624

Closed
colombod opened this issue Aug 28, 2020 · 5 comments
Assignees
Labels
author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug notebook verified Verification succeeded
Milestone

Comments

@colombod
Copy link
Contributor

@jrieken @roblourens

  • VSCode Version: 1.49.0-insider (user setup)
  • OS Version: Win 10.0.19041

using the api

         const cellIndex = document.cells.findIndex(c => c === cell);
         if (cellIndex >= 0) {
             const edit = new vscode.WorkspaceEdit();
             edit.replaceCellMetadata(document.uri, cellIndex, metadata);
             vscode.workspace.applyEdit(edit);
         }

doesn't update after first usage

@rebornix rebornix added bug Issue identified by VS Code Team member as probable bug notebook labels Aug 28, 2020
@rebornix rebornix added this to the August 2020 milestone Aug 28, 2020
@jrieken
Copy link
Member

jrieken commented Aug 31, 2020

There were still quite some issues with this on Friday, so trying today's insiders might show other results. Here are a bunch of tests that seem to be green.

doesn't update after first usage

This rings a bell. You aren't awaiting the applyEdit-call which might be the problem here after all

@jrieken jrieken added the info-needed Issue requires more information from poster label Aug 31, 2020
@jrieken
Copy link
Member

jrieken commented Sep 1, 2020

Seeing an error because transformEditsOutputs is missing. Pushing a fix for that but sometimes there is also errors like this

Error: Notebook 'file:///Users/jrieken/Code/_samples/devfest/103594.github-issues' has changed in the meantime
    at BulkCellEdits.apply (bulkCellEdits.js:43)
    at async BulkEdit._performCellEdits (bulkEditService.js:86)
    at async BulkEdit.perform (bulkEditService.js:65)
    at async BulkEditService.apply (bulkEditService.js:132)

That's a bad sign because it means we don't propagate all changes from the renderer to extension host side...

@jrieken
Copy link
Member

jrieken commented Sep 1, 2020

Ok, another source of trouble is the version id of the notebook. We send the version id that we know on the extension host and compare that with renderer's value of the version id. If they are different, we discard the edit. However, the are quite some places where the version id changes (in the renderer) without us sending a message to the extension host, e.g when cell text changes the notebook's version id increases but that's not forwarded to the extension host.

This needs some thinking and is part of larger clean-up work we need to do in notebook's land (fyi @rebornix). For now, I will disable the version check and optimistically accept all edits from the extension host

@jrieken jrieken removed the info-needed Issue requires more information from poster label Sep 1, 2020
@jrieken jrieken mentioned this issue Sep 1, 2020
5 tasks
@colombod
Copy link
Contributor Author

colombod commented Sep 1, 2020

Is there going to be a vscode update then that will allow me to merge this PR or should we call it off for the moment?

@jrieken
Copy link
Member

jrieken commented Sep 1, 2020

Should be in next/tomorrow's insiders... We are currently winding down for the August endgame, so until mid/end of week we will have new insiders builds and then we'll freeze insiders in preparation for the release

@jrieken jrieken closed this as completed Sep 3, 2020
@jrieken jrieken added the author-verification-requested Issues potentially verifiable by issue author label Sep 3, 2020
@connor4312 connor4312 added the verified Verification succeeded label Sep 3, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Oct 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug notebook verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants