fix(monaco): patch monaco-editor broken paste action#41
Merged
johnyanarella merged 1 commit intomainfrom May 1, 2026
Merged
Conversation
📝 WalkthroughWalkthroughPatch updates Monaco Editor's PasteAction to remove runtime dependency on Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
patches/monaco-editor.patch (4)
835-837:⚠️ Potential issue | 🔴 CriticalWrong model event name breaks
onDidValidateModelVersionAPIAt Line 836,
model.onDidValidateModelVersiondoes not match the newly added model API (onDidValidateVersion). This will fail at runtime.Proposed fix
export function onDidValidateModelVersion(model, listener) { - return model.onDidValidateModelVersion(listener); + return model.onDidValidateVersion(listener); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 835 - 837, The wrapper function onDidValidateModelVersion calls a non-existent method model.onDidValidateModelVersion causing runtime failure; update the implementation to call the correct new model API method model.onDidValidateVersion and return its result (i.e., replace model.onDidValidateModelVersion with model.onDidValidateVersion in the onDidValidateModelVersion function) so the exported wrapper forwards to the correct event on the model.
565-567:⚠️ Potential issue | 🟠 MajorRegister the new emitter for disposal to avoid listener leaks
At Line 566,
_onDidValidateVersionis created but not registered via_register(...). That bypasses model disposal cleanup.Proposed fix
- this._onDidValidateVersion = new Emitter(); + this._onDidValidateVersion = this._register(new Emitter()); this.onDidValidateVersion = this._onDidValidateVersion.event;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 565 - 567, The new Emitter instance _onDidValidateVersion created alongside onDidValidateVersion is not registered for disposal, causing potential listener leaks; update the owner class constructor or init sequence to call this._register(this._onDidValidateVersion) so the emitter is disposed when the model is disposed, ensuring _onDidValidateVersion is cleaned up like other emitters (follow the pattern used for other emitters registered via _register in the same class).
300-304:⚠️ Potential issue | 🔴 Critical
shadowCaretRangeFromPointcan dereferencenullafter folded-node fallbackAt Line 301,
elcan becomenull(previousSiblingabsent), but Line 304 callsel.getBoundingClientRect()unconditionally. This can throw at runtime.Proposed fix
- while (el && el.firstChild && el.firstChild.nodeType !== el.firstChild.TEXT_NODE && el.lastChild && (el.lastChild.firstChild || isInlineFolded(el.lastChild))) { - el = isInlineFolded(el.lastChild) ? el.lastChild.previousSibling : el.lastChild; + while (el && el.firstChild && el.firstChild.nodeType !== el.firstChild.TEXT_NODE && el.lastChild && (el.lastChild.firstChild || isInlineFolded(el.lastChild))) { + const next = isInlineFolded(el.lastChild) ? el.lastChild.previousSibling : el.lastChild; + if (!next) { + break; + } + el = next; } + if (!el) { + return range; + } // Grab its rect const rect = el.getBoundingClientRect();Also applies to: 311-313
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 300 - 304, In shadowCaretRangeFromPoint, the loop can set el to null (via previousSibling) and then el.getBoundingClientRect() is called unguarded; update the code around the loop and the subsequent rect access to null-check el before calling getBoundingClientRect() (e.g., if (!el) return/null/continue or find a safe ancestor), and apply the same guard to the similar occurrence around the code at the other block (lines handling the folded-node fallback near 311-313) so you never dereference el when it may be null; reference the variable el and the function shadowCaretRangeFromPoint when making the change.
241-253:⚠️ Potential issue | 🔴 CriticalCancellation currently leaves pending
Delayerpromises unsettledAt Line 241, removing
doRejectmeans callers awaiting the pending promise can hang forever aftercancel(). This is a correctness regression.Proposed fix
cancel() { this.cancelTimeout(); if (this.completionPromise) { - // this.doReject?.(new CancellationError()); + this.doReject?.(new CancellationError()); this.completionPromise = null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 241 - 253, The change removed calling this.doReject(new CancellationError()) inside Delayer.cancel(), leaving awaiting callers hanging; restore settling the pending promise on cancellation by calling the resolver instead of throwing — e.g., invoke this.doResolve?.(undefined) or resolve with a CancellationToken/marker so the completionPromise is always settled, then set this.completionPromise = null; ensure you reference Delayer.cancel(), this.doResolve / this.doReject, CancellationError and completionPromise when making the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@patches/monaco-editor.patch`:
- Around line 835-837: The wrapper function onDidValidateModelVersion calls a
non-existent method model.onDidValidateModelVersion causing runtime failure;
update the implementation to call the correct new model API method
model.onDidValidateVersion and return its result (i.e., replace
model.onDidValidateModelVersion with model.onDidValidateVersion in the
onDidValidateModelVersion function) so the exported wrapper forwards to the
correct event on the model.
- Around line 565-567: The new Emitter instance _onDidValidateVersion created
alongside onDidValidateVersion is not registered for disposal, causing potential
listener leaks; update the owner class constructor or init sequence to call
this._register(this._onDidValidateVersion) so the emitter is disposed when the
model is disposed, ensuring _onDidValidateVersion is cleaned up like other
emitters (follow the pattern used for other emitters registered via _register in
the same class).
- Around line 300-304: In shadowCaretRangeFromPoint, the loop can set el to null
(via previousSibling) and then el.getBoundingClientRect() is called unguarded;
update the code around the loop and the subsequent rect access to null-check el
before calling getBoundingClientRect() (e.g., if (!el) return/null/continue or
find a safe ancestor), and apply the same guard to the similar occurrence around
the code at the other block (lines handling the folded-node fallback near
311-313) so you never dereference el when it may be null; reference the variable
el and the function shadowCaretRangeFromPoint when making the change.
- Around line 241-253: The change removed calling this.doReject(new
CancellationError()) inside Delayer.cancel(), leaving awaiting callers hanging;
restore settling the pending promise on cancellation by calling the resolver
instead of throwing — e.g., invoke this.doResolve?.(undefined) or resolve with a
CancellationToken/marker so the completionPromise is always settled, then set
this.completionPromise = null; ensure you reference Delayer.cancel(),
this.doResolve / this.doReject, CancellationError and completionPromise when
making the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 6718b203-ad40-4677-8b9a-94a834356c62
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
patches/monaco-editor.patch
coryrylan
approved these changes
May 1, 2026
johnyanarella
force-pushed
the
topic/monaco-editor-menu-paste-bug
branch
from
a9f75ff to
f9d7ad2
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
patches/monaco-editor.patch (5)
835-837:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winWrong event accessor in
onDidValidateModelVersion.Line 836 calls
model.onDidValidateModelVersion(...), but the model API added in this patch isonDidValidateVersion(...). This will throw at runtime.Proposed fix
export function onDidValidateModelVersion(model, listener) { - return model.onDidValidateModelVersion(listener); + return model.onDidValidateVersion(listener); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 835 - 837, The exported helper onDidValidateModelVersion currently calls a non-existent model.onDidValidateModelVersion and will throw; change its implementation to call the correct API model.onDidValidateVersion(listener) so the export function onDidValidateModelVersion(model, listener) delegates to model.onDidValidateVersion with the same listener parameter.
565-567:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegister
_onDidValidateVersionfor disposal.The new emitter is created but not added to the
Disposablestore. Listeners can outlive model disposal.Proposed fix
- this._onDidValidateVersion = new Emitter(); + this._onDidValidateVersion = this._register(new Emitter()); this.onDidValidateVersion = this._onDidValidateVersion.event;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 565 - 567, The new Emitter instance _onDidValidateVersion is not added to the instance's disposable collection, so it can leak listeners; update the constructor or initialization area to register this._onDidValidateVersion with the existing disposable store (e.g., push/add this._onDidValidateVersion to the array or Disposable collection used by the class), ensuring it gets disposed when the model/component is disposed; keep the public onDidValidateVersion as this._onDidValidateVersion.event and only add the Emitter object itself to the disposables.
973-993:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard stale async diagnostics before applying markers/version.
You capture
modelVersionIdat Line 974, but before applying markers (Line 986) andsetModelVersionValidated(Line 992) there is no version equality check. Older async results can overwrite newer edits and mark the wrong version as validated.Proposed fix
- if (model.getLanguageId() !== this._selector) { + if (model.getLanguageId() !== this._selector || model.getVersionId() !== modelVersionId) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 973 - 993, You captured modelVersionId from model.getVersionId() but never verify it before applying editor.setModelMarkers and editor.setModelVersionValidated, allowing stale async diagnostics to overwrite newer edits; inside the DiagnosticsAdapter method (the block with worker = await this._worker(model.uri) and later calls to editor.setModelMarkers and editor.setModelVersionValidated) add a guard that re-checks model.getVersionId() === modelVersionId immediately before setting markers and before calling editor.setModelVersionValidated, and if they differ simply return (do not apply markers or mark the model version validated).
241-253:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Delayer.cancel()now leaves pending promises unsettled.At Line 242 the reject path is commented out, and at Line 253
completionPromiseis nulled without settling it. Any caller awaiting that promise can hang indefinitely.Proposed fix
cancel() { this.cancelTimeout(); if (this.completionPromise) { - // this.doReject?.(new CancellationError()); - this.completionPromise = null; + const pending = this.completionPromise; + this.doReject?.(new CancellationError()); + pending?.catch(() => {}); // consume expected cancellation during disposal paths + this.completionPromise = null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 241 - 253, Delayer.cancel() currently comments out the reject and clears completionPromise leaving waiting callers hanging; restore promise settlement by calling this.doResolve?.(undefined) (or a Cancellation sentinel) before setting this.completionPromise = null so awaiting callers get a resolved value instead of hanging, and then clear this.doResolve and this.doReject to avoid leaks; update the cancel implementation to settle via this.doResolve (not throwing) and then null out completionPromise, doResolve and doReject.
1-1204:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSplit this patch into focused changes before merge.
This file now bundles many unrelated behavioral/API changes while the PR objective is only paste-action repair. Because this patch is applied globally via
patchedDependencies, this dramatically increases regression and upgrade risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 1 - 1204, The patch mixes the PasteAction workaround with many unrelated changes (CSS animation vars, Delayer.cancel removal of rejection, scrollableElement.updateOptions, mouseHandler mouseleave ShadowDOM fix, shadowCaretRangeFromPoint tweaks, DelegatingEditor.dispose, DiffEditor/editor options tweaks, TextModel setVersionValidated API additions, DiagnosticsAdapter/ts worker changes, standalone API additions, etc.), increasing risk; split this into focused commits/PRs: create one minimal patch that only changes the PasteAction area in clipboard.js (the triggerPaste handling and related comments) and keep createMonacoEditorAPI/standaloneEditor exports untouched in that PR, then move each other logical change (Delayer, scrollable options, ShadowDOM fixes, TypeScriptWorker diff handling, model validation API, DelegatingEditor.dispose, CSS tokens) into separate, self-contained PRs/files that include tests and rationale; revert all non-clipboard changes from this commit and ensure pasted-only PR updates the one unique symbol Clipboard.PasteAction handling in clipboard.js and associated registerExecCommandImpl logic, while other edits reference their specific symbols (Delayer.cancel, AbstractScrollableElement.updateOptions, MouseHandler._mouseLeaveMonitor, shadowCaretRangeFromPoint, DelegatingEditor.dispose, TypeScriptWorker.fileNameIsDiff/getScriptFileNames, TextModel.setVersionValidated, DiagnosticsAdapter._doValidate) and are submitted separately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@patches/monaco-editor.patch`:
- Around line 835-837: The exported helper onDidValidateModelVersion currently
calls a non-existent model.onDidValidateModelVersion and will throw; change its
implementation to call the correct API model.onDidValidateVersion(listener) so
the export function onDidValidateModelVersion(model, listener) delegates to
model.onDidValidateVersion with the same listener parameter.
- Around line 565-567: The new Emitter instance _onDidValidateVersion is not
added to the instance's disposable collection, so it can leak listeners; update
the constructor or initialization area to register this._onDidValidateVersion
with the existing disposable store (e.g., push/add this._onDidValidateVersion to
the array or Disposable collection used by the class), ensuring it gets disposed
when the model/component is disposed; keep the public onDidValidateVersion as
this._onDidValidateVersion.event and only add the Emitter object itself to the
disposables.
- Around line 973-993: You captured modelVersionId from model.getVersionId() but
never verify it before applying editor.setModelMarkers and
editor.setModelVersionValidated, allowing stale async diagnostics to overwrite
newer edits; inside the DiagnosticsAdapter method (the block with worker = await
this._worker(model.uri) and later calls to editor.setModelMarkers and
editor.setModelVersionValidated) add a guard that re-checks model.getVersionId()
=== modelVersionId immediately before setting markers and before calling
editor.setModelVersionValidated, and if they differ simply return (do not apply
markers or mark the model version validated).
- Around line 241-253: Delayer.cancel() currently comments out the reject and
clears completionPromise leaving waiting callers hanging; restore promise
settlement by calling this.doResolve?.(undefined) (or a Cancellation sentinel)
before setting this.completionPromise = null so awaiting callers get a resolved
value instead of hanging, and then clear this.doResolve and this.doReject to
avoid leaks; update the cancel implementation to settle via this.doResolve (not
throwing) and then null out completionPromise, doResolve and doReject.
- Around line 1-1204: The patch mixes the PasteAction workaround with many
unrelated changes (CSS animation vars, Delayer.cancel removal of rejection,
scrollableElement.updateOptions, mouseHandler mouseleave ShadowDOM fix,
shadowCaretRangeFromPoint tweaks, DelegatingEditor.dispose, DiffEditor/editor
options tweaks, TextModel setVersionValidated API additions,
DiagnosticsAdapter/ts worker changes, standalone API additions, etc.),
increasing risk; split this into focused commits/PRs: create one minimal patch
that only changes the PasteAction area in clipboard.js (the triggerPaste
handling and related comments) and keep createMonacoEditorAPI/standaloneEditor
exports untouched in that PR, then move each other logical change (Delayer,
scrollable options, ShadowDOM fixes, TypeScriptWorker diff handling, model
validation API, DelegatingEditor.dispose, CSS tokens) into separate,
self-contained PRs/files that include tests and rationale; revert all
non-clipboard changes from this commit and ensure pasted-only PR updates the one
unique symbol Clipboard.PasteAction handling in clipboard.js and associated
registerExecCommandImpl logic, while other edits reference their specific
symbols (Delayer.cancel, AbstractScrollableElement.updateOptions,
MouseHandler._mouseLeaveMonitor, shadowCaretRangeFromPoint,
DelegatingEditor.dispose, TypeScriptWorker.fileNameIsDiff/getScriptFileNames,
TextModel.setVersionValidated, DiagnosticsAdapter._doValidate) and are submitted
separately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 6d8dd54c-b090-4485-98e5-41362bd808c2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
patches/monaco-editor.patch
Signed-off-by: John Yanarella <jyanarella@nvidia.com>
johnyanarella
force-pushed
the
topic/monaco-editor-menu-paste-bug
branch
from
f9d7ad2 to
aaae1a3
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
patches/monaco-editor.patch (5)
300-304:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winGuard null when skipping trailing
.inline-foldednodesAt Line 301,
elcan becomenullwhenel.lastChildis.inline-foldedand has nopreviousSibling; Line 304 then dereferencesel.Suggested fix
- while (el && el.firstChild && el.firstChild.nodeType !== el.firstChild.TEXT_NODE && el.lastChild && (el.lastChild.firstChild || isInlineFolded(el.lastChild))) { - el = isInlineFolded(el.lastChild) ? el.lastChild.previousSibling : el.lastChild; + while (el && el.firstChild && el.firstChild.nodeType !== el.firstChild.TEXT_NODE && el.lastChild && (el.lastChild.firstChild || isInlineFolded(el.lastChild))) { + const next = isInlineFolded(el.lastChild) ? el.lastChild.previousSibling : el.lastChild; + if (!next) { + break; + } + el = next; } + if (!el) { + return range; + } const rect = el.getBoundingClientRect();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 300 - 304, The loop that walks trailing nodes can set el to null (when isInlineFolded(el.lastChild) is true but el.lastChild.previousSibling is null), yet the code immediately calls el.getBoundingClientRect(); update the logic in the block containing the while loop and subsequent rect retrieval (references: variable el and helper isInlineFolded) to guard against el becoming null—either ensure the assignment never produces null (e.g., pick a safe fallback sibling) or add an explicit check after the loop to return/skip or handle the null case before calling el.getBoundingClientRect().
973-993:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent stale TS diagnostics from marking old versions as validated
You capture
modelVersionId(Line 974) but never verify it before applying markers/firing validation. Async results from older versions can be applied to newer text.Suggested fix
if (model.isDisposed()) { return; } // PATCH: Don't apply model markers if the language has changed. (Aligns this behavior with other language services.) if (model.getLanguageId() !== this._selector) { return; } + if (model.getVersionId() !== modelVersionId) { + return; + } editor.setModelMarkers( model, this._selector, diagnostics.map((d) => this._convertDiagnostics(model, d)) ); editor.setModelVersionValidated(model, modelVersionId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 973 - 993, The patch captures modelVersionId via model.getVersionId() but does not check that the async diagnostics correspond to the same version before applying markers/validation; update the diagnostics application flow in DiagnosticsAdapter (the method that calls this._worker, editor.setModelMarkers and editor.setModelVersionValidated) to compare the stored modelVersionId with the model.getVersionId() right before calling editor.setModelMarkers and editor.setModelVersionValidated and only apply markers/mark validated when they match; keep the existing early-return for model.isDisposed() and still skip when model.getLanguageId() !== this._selector.
835-837:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
onDidValidateModelVersioncalls a non-existent model APILine 836 calls
model.onDidValidateModelVersion(...), but the model interface added in this patch exposesonDidValidateVersion(...). This will throw at runtime.Suggested fix
export function onDidValidateModelVersion(model, listener) { - return model.onDidValidateModelVersion(listener); + return model.onDidValidateVersion(listener); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 835 - 837, The exported helper onDidValidateModelVersion calls a non-existent model API; change its body to invoke the actual model method onDidValidateVersion by replacing model.onDidValidateModelVersion(listener) with model.onDidValidateVersion(listener), and verify the model interface/type declares onDidValidateVersion so the signature matches the exported function.
565-567:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegister
_onDidValidateVersionso it is disposed with the modelLine 566 creates an
Emitterwithout_register, so listeners can survive model disposal.Suggested fix
- this._onDidValidateVersion = new Emitter(); + this._onDidValidateVersion = this._register(new Emitter()); this.onDidValidateVersion = this._onDidValidateVersion.event;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 565 - 567, The new Emitter instance `_onDidValidateVersion` is created without being registered for disposal, so its listeners can outlive the model; update the initialization in the constructor (where `_onDidValidateVersion` and `onDidValidateVersion` are set) to register the emitter with the model's `_register` method (i.e., call `this._register(...)` when creating the Emitter) so the emitter is disposed when the model is disposed; keep the public `onDidValidateVersion` event assignment unchanged (use `_onDidValidateVersion.event`) so consumers still get the event but it will be cleaned up automatically.
241-253:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Delayer.cancel()now leaves pending promises unsettledLine 241 previously rejected in-flight
completionPromise; commenting it out means canceled callers can await forever, causing hangs/leaks.Suggested fix
cancel() { this.cancelTimeout(); if (this.completionPromise) { - // this.doReject?.(new CancellationError()); + this.doReject?.(new CancellationError()); this.completionPromise = null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@patches/monaco-editor.patch` around lines 241 - 253, Delayer.cancel currently comments out the rejection that settled in-flight completionPromise, leaving callers awaiting forever; restore proper settlement by calling this.doReject?.(new CancellationError()) before clearing completionPromise (i.e., revert the commented-out rejection) so pending promises are settled on cancellation; ensure you reference the Delayer.cancel method, the this.doReject call, the CancellationError type, and the completionPromise field when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@patches/monaco-editor.patch`:
- Around line 300-304: The loop that walks trailing nodes can set el to null
(when isInlineFolded(el.lastChild) is true but el.lastChild.previousSibling is
null), yet the code immediately calls el.getBoundingClientRect(); update the
logic in the block containing the while loop and subsequent rect retrieval
(references: variable el and helper isInlineFolded) to guard against el becoming
null—either ensure the assignment never produces null (e.g., pick a safe
fallback sibling) or add an explicit check after the loop to return/skip or
handle the null case before calling el.getBoundingClientRect().
- Around line 973-993: The patch captures modelVersionId via
model.getVersionId() but does not check that the async diagnostics correspond to
the same version before applying markers/validation; update the diagnostics
application flow in DiagnosticsAdapter (the method that calls this._worker,
editor.setModelMarkers and editor.setModelVersionValidated) to compare the
stored modelVersionId with the model.getVersionId() right before calling
editor.setModelMarkers and editor.setModelVersionValidated and only apply
markers/mark validated when they match; keep the existing early-return for
model.isDisposed() and still skip when model.getLanguageId() !== this._selector.
- Around line 835-837: The exported helper onDidValidateModelVersion calls a
non-existent model API; change its body to invoke the actual model method
onDidValidateVersion by replacing model.onDidValidateModelVersion(listener) with
model.onDidValidateVersion(listener), and verify the model interface/type
declares onDidValidateVersion so the signature matches the exported function.
- Around line 565-567: The new Emitter instance `_onDidValidateVersion` is
created without being registered for disposal, so its listeners can outlive the
model; update the initialization in the constructor (where
`_onDidValidateVersion` and `onDidValidateVersion` are set) to register the
emitter with the model's `_register` method (i.e., call `this._register(...)`
when creating the Emitter) so the emitter is disposed when the model is
disposed; keep the public `onDidValidateVersion` event assignment unchanged (use
`_onDidValidateVersion.event`) so consumers still get the event but it will be
cleaned up automatically.
- Around line 241-253: Delayer.cancel currently comments out the rejection that
settled in-flight completionPromise, leaving callers awaiting forever; restore
proper settlement by calling this.doReject?.(new CancellationError()) before
clearing completionPromise (i.e., revert the commented-out rejection) so pending
promises are settled on cancellation; ensure you reference the Delayer.cancel
method, the this.doReject call, the CancellationError type, and the
completionPromise field when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 49357d34-9ac6-4b34-b8a9-990479041c38
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
patches/monaco-editor.patch
Collaborator
|
🎉 This issue has been resolved in version 0.0.10 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit