-
Notifications
You must be signed in to change notification settings - Fork 1
fix(monaco): address timing issue in validation patch #67
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -845,23 +845,36 @@ index 9490e25363fb401bfc0dae094012675c1e8a5ba9..adfa224edb41f454732cb16ee66b2998 | |
| setTheme: setTheme, | ||
| remeasureFonts: remeasureFonts, | ||
| diff --git a/esm/vs/language/common/lspLanguageFeatures.js b/esm/vs/language/common/lspLanguageFeatures.js | ||
| index 12a49bd4ac441dcbf3db21a25fb15614294f17d8..85016213666ec595890f20192353a922df09a2cf 100644 | ||
| index 12a49bd4ac441dcbf3db21a25fb15614294f17d8..b5087cf444492a8bcabed2a748efcb9561175cde 100644 | ||
| --- a/esm/vs/language/common/lspLanguageFeatures.js | ||
| +++ b/esm/vs/language/common/lspLanguageFeatures.js | ||
| @@ -138,8 +138,11 @@ class DiagnosticsAdapter { | ||
| @@ -133,13 +133,23 @@ class DiagnosticsAdapter { | ||
| this._disposables.length = 0; | ||
| } | ||
| _doValidate(resource, languageId) { | ||
| + // PATCH: Enable workers to communicate that syntax validation is complete for a specific model version | ||
| + const model = editor.getModel(resource); | ||
| + if (!model) { | ||
| + return; | ||
| + } | ||
| + const modelVersionId = model.getVersionId(); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Capture the versionId at the time of the validation request (this mirrors the unpatched behavior of the separate typescript code path). |
||
| this._worker(resource).then((worker) => { | ||
| return worker.doValidation(resource.toString()); | ||
| }).then((diagnostics) => { | ||
| + // PATCH: Enable workers to communicate that syntax validation is complete for a specific model version | ||
| + // let model = editor.getModel(resource); | ||
| const markers = diagnostics.map((d) => toDiagnostics(resource, d)); | ||
| let model = editor.getModel(resource); | ||
| - let model = editor.getModel(resource); | ||
| - if (model && model.getLanguageId() === languageId) { | ||
| + // PATCH: Gracefully handle the case where the model was disposed during async validation. | ||
| + if (model && !model.isDisposed() && model.getLanguageId() === languageId) { | ||
| + // PATCH: Gracefully handle the case where the model changed or was disposed during async validation. | ||
| + if (model && !model.isDisposed() && model.getLanguageId() === languageId && model.getVersionId() === modelVersionId) { | ||
| editor.setModelMarkers(model, languageId, markers); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bail out if the version of the model has advanced since validation starts (this mirrors the unpatched behavior of the separate typescript code path). |
||
| + // PATCH: Enable workers to communicate that syntax validation is complete for a specific model version | ||
| + editor.setModelVersionValidated(model, model.getVersionId()); | ||
| + editor.setModelVersionValidated(model, modelVersionId); | ||
| } | ||
| }).then(void 0, (err) => { | ||
| console.error(err); | ||
| @@ -482,26 +485,11 @@ class DocumentSymbolAdapter { | ||
| @@ -482,26 +492,11 @@ class DocumentSymbolAdapter { | ||
| if (!items) { | ||
| return; | ||
| } | ||
|
|
@@ -1166,4 +1179,4 @@ index 414405319ded9b3671a070b7405b7150afaa8321..c8b924d17e513acb02b8f8a74b75d8a7 | |
| + const element = this.domNode.getRootNode().querySelector(`#${this._list.getElementID(index)}`); | ||
| if (element) { | ||
| element.style.width = 'auto'; | ||
| const width = element.getBoundingClientRect().width; | ||
| const width = element.getBoundingClientRect().width; | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bail out if async validation arrives here after the model is disposed.