feat(dialogs): add a text input dialog - #41
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The dialogs specs doc still states input behavior is not implemented yet, which is now incorrect and should be updated to avoid misleading documentation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a standalone input text-entry dialog to the bundled dialogs provider and factors the shared Ink render/cleanup lifecycle into a reusable runDialog helper so both select and input share the same session/cleanup semantics.
Changes:
- Added
Dialogs.input({ message, initialValue? })implementation with paste/control filtering, code-point backspace, submit/cancel semantics, and non-interactive stream rejection. - Extracted the shared render/race/cleanup skeleton from
selectintorunDialog, and rewiredselectto use it without changingselect’s externally observed behavior. - Added comprehensive Bun tests for the new input dialog behavior and updated plugin manual + change doc task checklists.
File summaries
| File | Description |
|---|---|
plugins/dialogs/index.ts |
Adds input dialog and extracts shared runDialog render/cleanup session used by both dialogs. |
test/dialogs-plugin.test.ts |
Adds input-dialog test harness and a broad test suite covering rendering, editing, cancellation/submission, failures, and cleanup. |
docs/manual/plugins.md |
Documents the input dialog contract and clarifies shared dialog session/cleanup behavior. |
docs/changes/0017-add-dialog-text-input-and-composition.md |
Checks off Task A items for change 0017 now that standalone input is implemented and tested. |
REVIEW.md |
Records the intentional “unresolved control sequence recognized by shape (CSI only)” limitation as a review constraint. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The implementation matches the stated dialog semantics, preserves select behavior via a shared runDialog, and is backed by thorough tests and aligned documentation updates.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
Adds a standalone text
inputdialog to the bundled dialogs provider. This is Task A of change 0017; composition is deliberately deferred.What
inputdoesinput({ message, initialValue? })resolvesPromise<string | undefined>.initialValuewhen supplied and empty otherwise.CSIcontrol sequences append nothing.undefinedwithout terminating the process, so an intentional empty value stays distinguishable from cancellation.selectalready had.Shared render session
select's render, race, and cleanup skeleton is nowrunDialog, which both dialogs use. Adding a second consumer of that ~90-line block was the alternative to duplicating it; extraction was clearly the smaller change.select's observable behavior is unchanged, including itsSelect renderer exited before the dialog completedrejection, whichrunDialogreproduces verbatim via a label parameter.Deferred to Task B
Composition — user-provided select options, the
fieldsproperty onSelectOption, and theSelectResult{ value, values }envelope — is a separate later PR.selectstill returnsPromise<T | undefined>here. Change 0017's status staysdraftand Task B's checkboxes stay unticked; that PR flips them.Known limit, recorded in
REVIEW.mdInk strips the leading escape before
useInputruns and exposes no flag saying it did, and the same handler must append multi-character pastes, so an unresolved control sequence can only be recognized by shape. The filter covers theCSIform; consequently a paste that is exactly aCSIbody —[25~on its own — enters nothing, and an unrecognizedSS3sequence still appends its payload. Both directions are documented in the manual and inREVIEW.mdso the trade-off is not re-litigated.Verification
HOME=$(mktemp -d) XDG_DATA_HOME=$(mktemp -d) bun run checkexits 0: 453 tests pass, 100% statement/function/line coverage on every production source. The env isolation is needed on a developer machine because the test run otherwise pulls the real~/.local/share/tx/marketplacesplugins into the coverage report; CI is unaffected.