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

Notebook: executing cells turns editor dirty? #96403

Closed
bpasero opened this issue Apr 28, 2020 · 7 comments
Closed

Notebook: executing cells turns editor dirty? #96403

bpasero opened this issue Apr 28, 2020 · 7 comments
Assignees
Labels
api bug Issue identified by VS Code Team member as probable bug notebook verified Verification succeeded
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Apr 28, 2020

Refs: #96279

I executed the notebook and wasn't sure why this turns the document dirty? We are surely not writing the results into the file are we?

Kapture 2020-04-28 at 15 06 21

@bpasero bpasero added web Issues related to running VSCode in the web notebook labels Apr 28, 2020
@jrieken jrieken added this to the May 2020 milestone Apr 28, 2020
@jrieken
Copy link
Member

jrieken commented Apr 28, 2020

Yeah, that's something we need to look at. Notebooks can and often do save the output (we have decided against that for issues notebooks) but currently notebook extensions cannot control that, it's the UI that does this (and it shouldn't). For instance, an issues notebook shouldnt become dirty when running but for instance it should become dirty when lock/unlock a cell (since that's saved to disk)

@bpasero
Copy link
Member Author

bpasero commented Apr 28, 2020

What worries me more is that my input seems to get lost a lot. Is the cell input not saved into the file?

@jrieken
Copy link
Member

jrieken commented Apr 28, 2020

The input should always be saved! Internally, all cells are collected and then vscode.workspace.fs is used to write a blob of JSON text

@rebornix rebornix added the api label Apr 28, 2020
@bpasero bpasero removed the web Issues related to running VSCode in the web label Apr 29, 2020
@jrieken
Copy link
Member

jrieken commented May 13, 2020

@rebornix Is this still the core or did you move this already to the extension side? My plan is

  • running doesn't change dirty change (e.g no change event)
  • lock/unlock cell does emit a change event so that the UI can show dirty

@jrieken
Copy link
Member

jrieken commented May 29, 2020

I have pushed a change that propagates NotebookContentProvider#onDidChangeNotebook, internally that's exposed as INotebookTextModel#onDidChangeUnknown. Still work to do, eg. for having undoable change events...

@jrieken
Copy link
Member

jrieken commented Jun 3, 2020

Turns out this was done on purpose via #94073, 8f82b87. I will go ahead and undo those changes. Extensions that do want to save output should have all the necessary events and API by now (e.g listen to output change, emit dirty/change event)

@jrieken jrieken added the bug Issue identified by VS Code Team member as probable bug label Jun 3, 2020
@jrieken
Copy link
Member

jrieken commented Jun 3, 2020

closed via 99fa777

@jrieken jrieken closed this as completed Jun 3, 2020
@bpasero bpasero added the verified Verification succeeded label Jun 4, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api bug Issue identified by VS Code Team member as probable bug notebook verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants