fix: harden PR18 editor delegation#21
Closed
lajarre wants to merge 25 commits into
Closed
Conversation
Convert ModalEditor into a class-mixin factory `createModalEditor(Base)` so pi-vim can wrap a custom editor installed by another extension instead of replacing it. The session_start handler now reads `ctx.ui.getEditorComponent()` (Pi 0.71+) and, when present, builds the modal editor as a subclass of that extension's class rather than the default `CustomEditor`. This resolves the silent collision described in earendil-works/pi#3935. With no prior factory installed, behavior is unchanged: the canonical `ModalEditor` is used so existing `instanceof ModalEditor` checks still pass. API surface: - `createModalEditor(Base)` is now exported for extensions composing on top of pi-vim. It returns a `class extends Base` mixin. - `ModalEditor` remains exported as both a value and a type, defined as `createModalEditor(CustomEditor)`. - Constructor signature is now a passthrough `(...args)` to support the mixin chain. `labelColorizers` moved from a constructor argument to a `setColorizers(colorizers)` method, called post-construction by the session_start handler. Test code that previously passed colorizers to the constructor now uses the setter. Adds a modal-editor test that asserts the composed editor is an instance of the previously installed factory's class, that the probe runs once during install, and that standalone use still produces an `instanceof ModalEditor` instance. Bumps the @mariozechner/pi-coding-agent devDependency to ^0.71.0 so the typecheck sees the new `ExtensionUIContext.getEditorComponent` member. The peerDependencies range stays at `*` so older pi versions remain supported at runtime via the optional-chaining call (`?.`). Bumps pack-check size thresholds modestly to accommodate the README section and createModalEditor wrapper. Adds node_modules/ to .gitignore since it was missing.
Validate delegated editor state before compatibility handoff, preserve the legacy ModalEditor colorizer argument, expose delegate key-release opt-in, and fail image e2e package metadata errors as infrastructure failures.
Keep delegated key-release opt-in active only while pi-vim is in insert mode, ignore ordinary release events in normal and ex modes, and preserve bracketed paste chunks that contain release-like payload text.
Owner
Author
|
Follow-up after external review:
Verification after the follow-up:
|
Filter release events that remain after paste stripping, route normal fallthrough input through the insert delegate, and raise the pack budget for the current package size.
Owner
Author
|
Superseded by #23 (replacement-first, wrapper-tolerant — we decided not to ship the wrap-of-previous-editor approach). Context in earendil-works/pi#4798. |
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.
scope and intent
This PR hardens the editor-delegation bridge introduced in #18 so pi-vim is a good Pi 0.71+ citizen: it can be installed alongside another extension that also installs a custom editor (concrete case:
@jordyvd/pi-image-attachments) and both behaviors compose.The delegation layer is framed explicitly as an interim bridge, not pi-vim's long-term editor architecture. We're shipping it because the user-facing collision is real today and Pi 0.71+ has accepted
getEditorComponent()as the composition primitive. Further investment in growing the delegate surface should be weighed against an alternative direction (pi-vim as a full editor replacement, with stock Pi editor features re-implemented as pi-vim plugins) that may make more sense longer term and that depends on Pi's own evolution.review findings addressed
This PR fixes review findings raised on #18:
ModalEditor(..., colorizers)constructor slot while still passing base editor options throughwantsKeyReleasethrough the pi-vim wrapper so TUI does not filter release events before delegationFAIL-INFRAinstead of falling back to the registrycontext
The custom-editor wrangling has multiple contracts in play.
pi-tuifilters key-release events beforehandleInputbased onfocusedComponent.wantsKeyRelease; pi-vim wraps a previous editor rather than replacing it; later decorators can setwantsKeyReleaseon the wrapper; and existing consumers may still pass mode-label colorizers in the fourth constructor argument whileCustomEditoralso uses that slot for options. The tests cover those interactions so this remains an explicit compatibility layer rather than only the immediate PR18 symptom.verification
git diff --checknpm run checknpm run pack:checknode --import tsx/esm script/image-attachments-e2e.tsnpm run checkandnpm run pack:check