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

Decouple saving and running the worksheet #5238

Merged
merged 2 commits into from Oct 15, 2018

Conversation

Projects
None yet
2 participants
@smarter
Copy link
Member

smarter commented Oct 10, 2018

  • Do not automatically save the worksheet when the run command is
    executed.
  • Make it possible to save the worksheet without running it (by setting
    the configuration value dotty.runWorksheetOnSave to false)

@smarter smarter requested a review from Duhemm Oct 10, 2018

@smarter smarter force-pushed the dotty-staging:worksheet-stuff-2 branch 2 times, most recently from 1b4b69c to 208af0b Oct 10, 2018

this.removeRedundantBlankLines().then(_ => this.reset())
}

/**
* Run the worksheet in `document`, display a progress bar during the run.
*/
run(): Thenable<{}> {
this.prepareRun()

This comment has been minimized.

@Duhemm

Duhemm Oct 11, 2018

Contributor

So you run the worksheet without it being saved, which will work because the compiler doesn't run it from the source that is in the file, but from the source that it received in textDocument/didChange, right?

This comment has been minimized.

@smarter
Show resolved Hide resolved vscode-dotty/src/worksheet.ts Outdated
Show resolved Hide resolved vscode-dotty/src/worksheet.ts

@Duhemm Duhemm assigned smarter and unassigned Duhemm Oct 11, 2018

Decouple saving and running the worksheet
- Do not automatically save the worksheet when the run command is
  executed.
- Make it possible to save the worksheet without running it (by setting
  the configuration value dotty.runWorksheetOnSave to false)

@smarter smarter force-pushed the dotty-staging:worksheet-stuff-2 branch from 208af0b to 7c871cc Oct 15, 2018

Async-related improvements
- Replaced removeRedundantBlankLines by removeRedundantBlankLinesEdit which
doesn't have side-effects but returns an array of TextEdit
- Fixed onWillSaveTextDocument to call event.waitUntil which is the only
way to actually apply changes synchronously.
- Removed the return in onDidSaveTextDocument, the result of the lambda
for an event isn't actually used for anything
- Changed Worksheet#run() to have a more precise result type and to only
run something if prepareRun() succeeded.

@smarter smarter force-pushed the dotty-staging:worksheet-stuff-2 branch from 7c871cc to 6640161 Oct 15, 2018

@Duhemm

Duhemm approved these changes Oct 15, 2018

@Duhemm Duhemm merged commit 7087a6c into lampepfl:master Oct 15, 2018

2 checks passed

CLA User signed CLA
Details
continuous-integration/drone/pr the build was successful
Details

@Duhemm Duhemm deleted the dotty-staging:worksheet-stuff-2 branch Oct 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment