From b5619a836e58808698dfc32dc21d2a3b2ed153fe Mon Sep 17 00:00:00 2001 From: Emmz Rendle Date: Thu, 28 May 2026 13:50:55 +0100 Subject: [PATCH 01/10] openspec: propose api-ergonomics-pass-1 + scaffold DEVLOG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Open the change that closes three of the five §14.4 ergonomics gaps surfaced by the dmon-wizard port: Line.FromText + string-accepting overloads (gap 5), opt-in AllowBack on Select/Choice dialogs (gap 1), and secret-default masking on the input dialog (gap 3). Surface-only; no breaking changes; all additions or opt-in flags. Targets dcli 0.2.0-rc.1. Also folds in a small docs task (§6.7) to refine the repo CLAUDE.md's "Where to look for historical context" subsection to match the canonical wording recommended by the personal devlog skill (covers both in-flight + archived DEVLOGs; the form shipped in PR #2 only covered the archived case). Scaffolds DEVLOG.md inside the change directory per the personal devlog skill's convention (`~/.claude/skills/devlog/SKILL.md`). First exercise of that skill end-to-end. Co-Authored-By: Claude Opus 4.7 --- .../api-ergonomics-pass-1/.openspec.yaml | 2 + .../changes/api-ergonomics-pass-1/DEVLOG.md | 61 ++++++++++ .../changes/api-ergonomics-pass-1/design.md | 108 ++++++++++++++++++ .../changes/api-ergonomics-pass-1/proposal.md | 41 +++++++ .../specs/fixed-region/spec.md | 84 ++++++++++++++ .../specs/styled-text/spec.md | 44 +++++++ .../changes/api-ergonomics-pass-1/tasks.md | 53 +++++++++ 7 files changed, 393 insertions(+) create mode 100644 openspec/changes/api-ergonomics-pass-1/.openspec.yaml create mode 100644 openspec/changes/api-ergonomics-pass-1/DEVLOG.md create mode 100644 openspec/changes/api-ergonomics-pass-1/design.md create mode 100644 openspec/changes/api-ergonomics-pass-1/proposal.md create mode 100644 openspec/changes/api-ergonomics-pass-1/specs/fixed-region/spec.md create mode 100644 openspec/changes/api-ergonomics-pass-1/specs/styled-text/spec.md create mode 100644 openspec/changes/api-ergonomics-pass-1/tasks.md diff --git a/openspec/changes/api-ergonomics-pass-1/.openspec.yaml b/openspec/changes/api-ergonomics-pass-1/.openspec.yaml new file mode 100644 index 0000000..352690f --- /dev/null +++ b/openspec/changes/api-ergonomics-pass-1/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-28 diff --git a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md new file mode 100644 index 0000000..ce79c75 --- /dev/null +++ b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md @@ -0,0 +1,61 @@ +# DEVLOG — `api-ergonomics-pass-1` + +> **Status: in-flight.** Maintained while applying the change per the OpenSpec apply +> workflow (see the project's `CLAUDE.md`). Captures per-section narrative the spec +> files don't carry: decisions under uncertainty, deviations, surfaced bugs, HITL +> verifications. On archive this file moves with the change to +> `openspec/changes/archive/YYYY-MM-DD-api-ergonomics-pass-1/DEVLOG.md` and the +> status flips to **shipped** (see `/devlog freeze`). + +## How to resume + +- Branch: **`change/api-ergonomics-pass-1`** (created from `main`). Stay on it. +- Working tree state: **CLEAN** (no section work has started; only OpenSpec artefacts + this DEVLOG are committed). +- Sanity check command: + `dotnet build -c Release && dotnet test -c Release && dotnet format --verify-no-changes && openspec validate api-ergonomics-pass-1 --strict` + → expect **0 warnings, 688 tests green** (baseline from `core-rendering-architecture`), clean format, valid. +- Resume point: **§1 — `Line.FromText` factory** (first unticked task: `1.1`). See the Section status table for what's done so far. +- Check the memory files listed at the bottom before briefing — several encode hard-won constraints that survive into this change (in particular: CA2007 / loop-thread discipline if any new dialog test touches the loop). + +## Section status + +One row per `## N.` section in `tasks.md`. Add a row when the section commits. + +| § | Section | Commit | Tests after | Notes | +|---|---------|--------|-------------|-------| +| — | (pending — §1 is next) | — | — | — | + +## Decisions & deviations + +Narrative log of anything that wasn't a straight read-off-the-spec-and-implement. One entry per decision, dated/section-scoped. Examples worth recording for this change: + +- Backspace-at-empty-position semantics for `AllowBack` (Decision 4 in `design.md`) — if a reviewer or implementation reveals a case where the heuristic fails, log it here and propose the resolution before proceeding to §3.6/§3.7 tests. +- Implicit `string → Line` conversion is **rejected** (Decision 1) — if a consumer migration would have been much cleaner with the implicit, log the case so a future change can revisit. +- Secret-default masking reuses the existing `_userEdited` flag (Decision 5) — if reviewer finds the flag is dirty (programmatic `SetText` mutates without flipping it), the resolution goes here. + +*(No decisions logged yet — change has not started.)* + +## Human-in-the-loop verifications + +Anything that can't be settled by automated gates. For each: section reference, exact copy-pasteable command, what the user should see, and current status. + +*(No HITL verifications anticipated for this change — the §14.4 manual harness invariants are preserved; the new surface is fully testable via `Dcli.Testing.HeadlessTerminal`. If a §5 demo update or §3 `AllowBack` keybinding reveals a real-terminal interaction worth eyeballing, log it here.)* + +## Open follow-ups / known gaps (after this change lands — NOT in scope here) + +Surface gaps for future changes. Link to memory files where the constraint is encoded so the next change's orchestrator picks them up. + +- **`DialogOutcome.Back` for multi-select** — deferred from this change (Backspace-at-empty-toggle-position is ambiguous in multi-select; see `design.md` Decision 4). A future change can revisit once a real consumer needs it. +- **`Input.Prompt` / `Input.ReadOnly` on the fixed-region input surface** — still deferred from §12 of the architecture change. Memory file: [[section14-api-ergonomics-findings]] entry 3-related; not blocked by this change. +- **`Scrollback.AppendRule`, incremental `Collapsible.AppendLine`, `PasteEvent` editor routing** — separate future changes; this change does not touch them. +- **VT-escape sanitisation of `Segment.Text`** — separate future change. Memory file: [[vt-escape-sanitization-gap]]. + +## Memory files (indexed by `~/.claude/projects/-Users-emmz-github-emmz-dcli/memory/MEMORY.md`) + +- [[section14-api-ergonomics-findings]] — the five §14.4 ergonomics gaps; this change closes three of them. Source of truth for which gaps are in scope vs deferred. +- [[ca2007-render-loop-thread-discipline]] — CA2007 is suppressed repo-wide; loop-thread correctness must be checked by hand. Applies to any new dialog test that posts commands to `LoopEngine.InputWriter`. +- [[restore-on-signal-rendering-state]] — restore-on-signal protocol is load-bearing; not touched by this change but worth knowing if any new render-path code is added. + +## Resume point + +> **Currently at §1.1 — Add `public static Line FromText(string text, Style? style = null)` to `src/Dcli/Line.cs`.** Working tree is clean. Next worker brief: implement §1 (`Line.FromText` factory) end-to-end, including XML docs (1.2) and tests (1.3). Single small section; expect one worker call, one reviewer audit, one commit. diff --git a/openspec/changes/api-ergonomics-pass-1/design.md b/openspec/changes/api-ergonomics-pass-1/design.md new file mode 100644 index 0000000..7c0a996 --- /dev/null +++ b/openspec/changes/api-ergonomics-pass-1/design.md @@ -0,0 +1,108 @@ +## Context + +`dcli` v1 shipped under change `core-rendering-architecture` (archived as `2026-05-28-core-rendering-architecture`). The §14.4 dmon-wizard port at `samples/Dcli.Demo.DmonWizard/` proved the public surface works but exposed concrete papercuts. Five gaps were recorded in memory `section14-api-ergonomics-findings`: + +1. `DialogOutcome.Back` is structurally dead (no v1 keybinding). +2. `MultiSelectAsync` is a validated upgrade — **not a gap**, recorded as positive. +3. `IsSecret` default rendering leaks clear-text on first paint. +4. No `internal → public` widening was required — **not a gap**, recorded as positive. +5. `new LineBuilder().Text(s).Build()` appears ~12× in the renderer — heavy string ceremony. + +Items #1, #3, #5 are blockers for the upcoming `dmon-migration` change. Items #2 and #4 are positive validations that need no action. + +The change targets the smallest possible diff that closes the three blockers cleanly, without touching unrelated deferred work (`Input.Prompt`/`ReadOnly`, `Scrollback.AppendRule`, incremental `Collapsible.AppendLine`, `PasteEvent` routing, VT-escape sanitisation). + +## Goals / Non-Goals + +**Goals:** +- Cut the call-site ceremony for label-only `Line`s by ~80% — `Line.FromText(s)` replaces `new LineBuilder().Text(s).Build()` everywhere. +- Make `DialogOutcome.Back` a real, reachable outcome under explicit consumer opt-in so wizard-style "go back one step" UIs work without synthetic list items. +- Stop leaking the seeded default's clear-text content when `IsSecret=true`. +- Preserve the v1 public surface: every change is an overload or opt-in flag. Existing `Dcli`-consumer code keeps compiling and behaving identically. +- Tests use only the `Dcli.Testing` headless harness — no new test-fakes invented in pass-1. + +**Non-Goals:** +- Multi-select `Back` outcome (deferred; the Backspace-at-empty-toggle-position semantics are ambiguous and a pass-2 design problem). +- An implicit `string → Line` conversion (rejected — see Decision 1). +- `Input.Prompt` / `Input.ReadOnly` on the fixed-region input surface (still deferred from §12; out of scope here). +- Any other §14.4-adjacent feature (`AppendRule`, `Collapsible.AppendLine`, `PasteEvent` editor routing). +- Touching the `Dcli.Testing` public surface. +- Touching internal mechanics (loop, scrollback model, parser). This change is **surface-only** — it adds factories, overloads, one keybinding arm, and one rendering tweak. + +## Decisions + +### 1. Explicit `Line.FromText` factory; reject implicit `string → Line` conversion + +`Line.FromText(string text, Style? style = null) → Line` is a static factory on `Line`. An implicit operator `string → Line` was considered and rejected. + +- **Why explicit:** `Line` is the rendering surface; styling is the whole point of having `Line` as a separate type from `string`. An implicit conversion would let consumers accidentally pass a `string` where a styled `Line` was intended, losing styling silently. The explicit factory is the seam — the type system stops accidental flattening. +- **Why a static method, not a constructor:** `Line` is a `record class` with `IReadOnlyList Segments` — adding a `Line(string, Style?)` constructor would compete with the existing `Line(IReadOnlyList)` and create ambiguity at construction sites. A named factory reads better at call sites (`Line.FromText("hello")`) and leaves the canonical constructor intact for the styled case. +- **Alternative considered — extension method (`"hello".AsLine()`):** rejected. Extension-method-on-string discoverability is poor for a core library type; the factory lives on `Line` itself where IntelliSense surfaces it under the type being constructed. + +### 2. String-accepting overloads, not type parameters + +The `*Request` records (`InputRequest`, `SelectRequest`, `MultiSelectRequest`, `ChoiceRequest`) each grow a sibling constructor / record signature accepting `string` / `IReadOnlyList` (or `params string[]`) variants. The originals stay. + +- **Why overloads, not generics:** introducing `SelectRequest` for `T : Line` would explode the type graph and break consumer call sites that already use the non-generic form. +- **Why `params string[]` AND `IReadOnlyList`:** `params` reads beautifully for inline literal lists (`new SelectRequest("Pick one", "Yes", "No", "Cancel")`) while `IReadOnlyList` covers the "I computed these strings" case. Both compose to the same internal representation by mapping each string through `Line.FromText`. +- **Backward compatibility:** every overload is an addition. The original `IReadOnlyList` constructors stay first in the file so default-parameter rules don't shift overload resolution. + +### 3. `IScrollback.Append(string)` overload + +Same shape as the request overloads. Internally calls `Append(Line.FromText(text))`. One line in `ScrollbackSurface`, one new method on `IScrollback`. + +- **Why on the interface:** consumers depend on `IScrollback`. Putting the overload only on the concrete `ScrollbackSurface` would invisibly fork the test-fake surface. The interface gets the overload; implementers (production + the tier-A fake) get a one-line default forwarder. + +### 4. `AllowBack` flag on `SelectRequest` / `ChoiceRequest`; Backspace-at-empty as the keybinding + +Pass-1 wires Back to a single overlay event: **Backspace pressed when the dialog is at its initial (no movement, no input) position**. + +- **Why Backspace specifically:** the Latin keyboard intuition for "go back" is Backspace; this is what every browser/wizard/CLI back-stack consumer expects. It does not collide with arrow nav (still consumes `↑↓`) or Enter (Submit) or Esc (Cancel). +- **Why "at empty position" / opt-in:** Backspace is a printable-ish key in many overlays. In a Select dialog with no list movement yet the keystroke is unambiguous; once the user has moved the selection it could plausibly mean "reset". The simplest semantic for pass-1: **only fire Back if the user has not yet moved the selection AND `AllowBack=true`.** Once the user has touched `↑↓`, Backspace becomes a no-op for the rest of that overlay session. Consumers that want unconditional Back can simply re-call the dialog. +- **Why opt-in (`AllowBack=false` default):** existing `SelectAsync` callers must not change behaviour. Today Backspace is a no-op in a Select dialog; that stays the case unless the consumer requests `AllowBack=true`. +- **Why no flag on `MultiSelectRequest`:** in multi-select, Backspace-at-empty-position could mean "unselect last" or "go back". Both are reasonable; neither dominates. Defer to a pass-2 design conversation rather than pick the wrong one now. +- **Fallback to `[`:** **rejected for pass-1.** A single keybinding is easier to test and document. If real users hit a conflict, pass-2 can add `[` as an alternate; no flag-design lock-in is needed today. +- **Alternative considered — Escape-twice:** rejected. Esc already means Cancel and conflicts with the modal-dismiss semantic. Two Escs would need a 500ms-style double-tap window — far more complex than Backspace-at-empty. + +### 5. Secret-default rendering tweak + +When `InputDialog` is constructed with `IsSecret=true` and a non-empty `Default`, the first paint renders the default as `'•' * displayWidth(default)` instead of the real string. Mechanism: + +- The dialog already tracks whether the user has edited the buffer (it has to, for `InputChanged` emission). Reuse that flag: while `!_userEdited && IsSecret`, paint as masked; otherwise paint normally (which is already masked for `IsSecret=true` via the existing render path). +- `Submit` is unaffected — returns the real buffer contents (the seeded `Default` if untouched, the edited text otherwise). +- **Why not always mask under `IsSecret`:** the existing render path *does* mask, but it masks the *current buffer contents*. The bug is that on first paint the buffer contains the default string un-marked-as-secret. The fix is exactly the "untouched + secret + non-empty default" case. + +### 6. No production-loop / scrollback / parser code touched + +This change is **surface-only**. The internal loop, scrollback model, fixed-region composer, frame painter, and parser stay identical. The whole change should fit in: +- `src/Dcli/Line.cs` (factory). +- `src/Dcli/IScrollback.cs` + `src/Dcli/ScrollbackSurface.cs` (overload). +- `src/Dcli/InputRequest.cs` / `SelectRequest.cs` / `MultiSelectRequest.cs` / `ChoiceRequest.cs` (overloads + `AllowBack` flag on the two relevant records). +- `src/Dcli/Internal/FixedRegion/Dialog.cs` + `ChoiceDialog.cs` (Back keybinding, gated on the new flag). +- `src/Dcli/Internal/FixedRegion/InputDialog.cs` (secret-default mask). +- The fakes (`tests/Dcli.Tests/FakeTerminalTests.cs`) get the same overloads as fire-and-forget pass-throughs — preserves tier-A symmetry. + +### 7. Tests ride on `Dcli.Testing` only + +Every new test lives in `tests/Dcli.Tests/` and uses `HeadlessTerminal` from `Dcli.Testing` rather than a hand-rolled fake. Tier-A `FakeTerminalTests` adds the new overload assertions so the fakeability contract still covers the new surface. + +## Risks / Trade-offs + +- **Adding `Line.FromText` invites convergence at call sites** — if a future change adds another shorthand, the conventions can drift (e.g. `Line.FromText(s, style)` vs `Line.Of(s)`). *Mitigation:* document the factory as the canonical short form; future changes that want different shorthands must justify why `FromText` doesn't cover their case. +- **Backspace-at-empty is a heuristic, not a contract** — a future overlay where Backspace has a different meaning (e.g. a future code-completion overlay) might collide. *Mitigation:* the trigger is opt-in (`AllowBack=false` default); collisions surface only when a consumer asks for both. +- **Pass-1 doesn't cover multi-select Back** — wizards that want a "back through a multi-select step" lose the affordance. *Mitigation:* consumers wrap multi-select in a state machine that maps "cancel" → "go back" and re-shows; or wait for pass-2. Document this on `MultiSelectRequest`. +- **Secret-default masking depends on the existing `_userEdited` flag being honest** — if a programmatic `SetText` mutates the buffer without flipping the flag, the mask would persist (or, depending on direction, the leak returns). *Mitigation:* the existing `InputDialog` already has to handle this for `InputChanged` emission, so the bug surface is shared, not new. Add a test that confirms a programmatic `SetText` (post-construction) flips the flag *or* keeps the mask, whichever is decided in code review. +- **Implicit `string → Line` rejected** — some consumers may write `term.Scrollback.Append("hello")` and expect default styling, and want to upgrade to styled later. They have to opt into `Line.FromText("hello", new Style(...))` or the full `LineBuilder` at that point. *Mitigation:* both forms read fine; the explicit upgrade path is healthier than silent stylelessness. + +## Migration Plan + +- Open the change on a branch `change/api-ergonomics-pass-1` off `main`. +- Apply per `tasks.md`, one §-section per commit, gated by the standard four gates + reviewer audit. +- No archived spec breakage — every delta is `MODIFIED` with strictly additive requirements. +- After archive, run `openspec sync-specs` (manual if needed) to fold the deltas into `openspec/specs/{styled-text,fixed-region}/spec.md`. +- Bump dcli version to `0.2.0-rc.1` (preview minor — additive, no breaking changes). `Dcli.Testing` versions in lockstep. +- Open the follow-up `dmon-migration` change *after* this lands so the migration uses the new surface directly. + +## Open Questions + +None. The Backspace-at-empty trigger is a deliberate pick (Decision 4); the secret-default tweak is unambiguous (Decision 5); the rejected-implicit-conversion is settled (Decision 1). diff --git a/openspec/changes/api-ergonomics-pass-1/proposal.md b/openspec/changes/api-ergonomics-pass-1/proposal.md new file mode 100644 index 0000000..72e9c41 --- /dev/null +++ b/openspec/changes/api-ergonomics-pass-1/proposal.md @@ -0,0 +1,41 @@ +## Why + +Porting `dmon-core`'s `WizardEngine` slice in §14.4 of the core-rendering-architecture change validated dcli's surface end-to-end but surfaced five concrete ergonomics gaps. Three of them — verbose `Line` lifting, the structurally-dead `DialogOutcome.Back`, and a leaky `IsSecret` default render — are blocking a clean migration of the rest of `Dmon.Terminal` onto dcli. This change closes those three before the dmon migration starts, so the migration is a single clean pass and the dcli public API doesn't churn twice on the same call sites. + +## What Changes + +- **`Line.FromText(string text, Style? style = null)` factory** added on `Line`. Replaces the `new LineBuilder().Text(s).Build()` ceremony for label-only strings. +- **String-accepting overloads** added across the consumer surface: + - `IScrollback.Append(string text)` alongside `Append(Line)`. + - `InputRequest` constructor / record accepts `string? Prompt` alongside `Line? Prompt`. + - `SelectRequest` / `MultiSelectRequest` / `ChoiceRequest` accept `IReadOnlyList` or `params string[]` items alongside their existing `Line` item lists. +- **Explicit conversion only** — an implicit `string → Line` conversion is **rejected**. `FromText` is the only seam, so callers don't accidentally pass strings where styled `Line`s were intended. +- **`OverlayCloseKind.Back` becomes reachable** via an opt-in `AllowBack` flag on `SelectRequest` and `ChoiceRequest` (default `false`, backward-compatible). When `true`, the dialog wires **Backspace at the empty initial position** (with `[` as fallback if Backspace becomes ambiguous in a future overlay) to `OverlayCloseKind.Back` → `DialogOutcome.Back`. `MultiSelectRequest` does **not** get this flag in pass-1 (Backspace at a multi-toggle position is semantically ambiguous; defer). +- **`InputDialog` masks its seeded `Default` when `IsSecret=true`**. Today bullets only kick in once the user edits, so the seeded default leaks clear-text on first paint. After this change the default is rendered as `'•' * displayWidth(default)` until the first edit; `Submit` still returns the real string. + +No breaking changes. Every public addition is an overload or an opt-in flag; existing call sites continue to compile and behave identically. + +## Capabilities + +### New Capabilities + +None — this change extends existing capabilities only. + +### Modified Capabilities + +- `styled-text`: ADDED requirement for the `Line.FromText` factory and the `string`-accepting consumer overloads (no behavioural change to existing requirements). +- `fixed-region`: MODIFIED dialog close semantics to add the `Back` outcome under opt-in `AllowBack`; MODIFIED `InputDialog` default-rendering when `IsSecret=true`. + +## Impact + +- **Public API (`Dcli`):** new factory + overloads on `Line`, `IScrollback`, `InputRequest`, `SelectRequest`, `MultiSelectRequest`, `ChoiceRequest`. Existing members unchanged. Net additions only. +- **Public API (`Dcli.Testing`):** none directly; the headless harness is unaffected. New tests use it as a substrate. +- **Production code:** small. `Line.FromText` is one new static method on a record. The dialog `Back` keybinding is one switch arm in `Dialog.HandleKey`/`ChoiceDialog.HandleKey` gated on the new request flag. The secret-default masking is one render-path change in `InputDialog.Render`. +- **Tests:** ~10–15 new tests across `LineTests` (factory), `FacadeTests` (string overloads round-trip), `DialogSelectionTests`/`ChoiceDialogTests` (`AllowBack`), `InputDialogTests` (secret-default mask). All ride on the `Dcli.Testing` headless harness. +- **Repo docs:** the project `CLAUDE.md` "Where to look for historical context on a shipped change" subsection (added in PR #2) gets refined during this change to match the canonical wording recommended by the personal `devlog` skill at `~/.claude/skills/devlog/SKILL.md`. The skill's wording covers both the in-flight DEVLOG inside the change directory AND the archived DEVLOG, where PR #2's wording only covered the archived case. Small documentation hygiene task; folded into this change for convenience. +- **Out of scope (deferred to later changes):** + - `DialogOutcome.Back` for multi-select dialogs. + - `Input.Prompt` / `Input.ReadOnly` on the fixed-region input surface (already deferred from §12). + - `Scrollback.AppendRule`, incremental `Collapsible.AppendLine`, `PasteEvent` editor-routing. + - VT-escape sanitisation of `Segment.Text` (tracked in memory `vt-escape-sanitization-gap`). +- **Consumers:** unblocks the upcoming `dmon-migration` change. Ergonomics gaps 2 and 4 from the §14.4 findings (MultiSelect-is-validated, no-widening-needed) require no work — they're already-shipped wins; this change is the three blockers only. diff --git a/openspec/changes/api-ergonomics-pass-1/specs/fixed-region/spec.md b/openspec/changes/api-ergonomics-pass-1/specs/fixed-region/spec.md new file mode 100644 index 0000000..9165100 --- /dev/null +++ b/openspec/changes/api-ergonomics-pass-1/specs/fixed-region/spec.md @@ -0,0 +1,84 @@ +## MODIFIED Requirements + +### Requirement: Awaitable modal dialogs + +The library SHALL expose select, multi-select, input, and choice dialogs as awaitable operations that return a `DialogResult` whose outcome is `Submitted`, `Back`, or `Cancelled`. + +`SelectRequest` and `ChoiceRequest` SHALL expose an opt-in `AllowBack` flag (default `false`). When `AllowBack` is `true`, the dialog SHALL produce `DialogOutcome.Back` if the user presses **Backspace** before moving the selection (i.e. before any `↑`/`↓` keystroke is consumed by the dialog). Once the selection has been moved, Backspace SHALL be a no-op for the remainder of that overlay session. When `AllowBack` is `false` (the default), Backspace SHALL have no effect on `SelectRequest`/`ChoiceRequest` overlays — preserving v1 behaviour. + +`MultiSelectRequest` SHALL NOT expose `AllowBack` in this revision; its Back-semantics are deferred. + +`InputRequest` SHALL NOT expose `AllowBack`; in an input dialog Backspace is an editing key. + +#### Scenario: Select submitted + +- **WHEN** the user highlights an item in a select dialog and presses Enter +- **THEN** the awaited result is `Submitted` carrying the chosen index + +#### Scenario: Cancelled by escape + +- **WHEN** the user presses Escape in a dialog +- **THEN** the awaited result is `Cancelled` + +#### Scenario: Multi-select toggling + +- **WHEN** the user presses space on items in a multi-select dialog +- **THEN** those items toggle in the returned selection set + +#### Scenario: Cancellation token closes the dialog + +- **WHEN** the `CancellationToken` passed to a dialog is cancelled +- **THEN** the overlay closes and the awaited result is `Cancelled` + +#### Scenario: AllowBack=true on Select produces Back + +- **WHEN** a `SelectRequest` with `AllowBack=true` is shown and the user presses Backspace before moving the selection +- **THEN** the awaited result is `DialogOutcome.Back` + +#### Scenario: AllowBack=true on Choice produces Back + +- **WHEN** a `ChoiceRequest` with `AllowBack=true` is shown and the user presses Backspace before moving the selection +- **THEN** the awaited result is `DialogOutcome.Back` + +#### Scenario: AllowBack=true is suppressed after movement + +- **WHEN** the user presses `↓` (consumed by the dialog) and then presses Backspace in a `SelectRequest` with `AllowBack=true` +- **THEN** Backspace is ignored for the rest of that overlay session — neither `Back` nor `Cancelled` is produced + +#### Scenario: AllowBack=false is the default + +- **WHEN** a `SelectRequest` or `ChoiceRequest` is constructed without setting `AllowBack` +- **THEN** Backspace has no effect on the dialog and existing v1 behaviour is preserved + +### Requirement: Owned input editor + +The library SHALL own an input editor supporting caret movement, multiline text, display-width-aware wrapping, history recall, and internal scrolling when its content exceeds its allotted height. + +When an `InputRequest` is constructed with `IsSecret = true` and a non-empty `Default`, the editor SHALL render the seeded default as `'•'` repeated by the default's display width on every paint that occurs before the user's first edit. Once the user makes any edit (insert, delete, paste, history-recall), the editor SHALL fall back to the existing secret-render path (which masks the current buffer contents as bullets on each paint). The `Submit` outcome SHALL return the real string contents — either the unedited `Default` or the edited text — regardless of how it was rendered. + +When `IsSecret = false`, the editor SHALL render the seeded default as plain text (unchanged from v1 behaviour). + +#### Scenario: Wrapping tracks the caret + +- **WHEN** input text exceeds the available width +- **THEN** the text wraps and the caret is positioned at the correct visual row and column + +#### Scenario: History recall + +- **WHEN** the user navigates input history +- **THEN** the buffer is replaced with the recalled entry + +#### Scenario: Secret default is masked before first edit + +- **WHEN** an `InputRequest` is shown with `IsSecret=true` and a non-empty `Default` and the user has not yet edited the buffer +- **THEN** the paint shows bullets (`•`) at every column the default occupies, never the default's clear-text content + +#### Scenario: Secret default reveals real text on submit + +- **WHEN** the user submits an `InputRequest` with `IsSecret=true` and a non-empty `Default` without editing the buffer +- **THEN** the awaited `DialogResult.Value` is the real `Default` string (not bullets) + +#### Scenario: Non-secret default renders as plain text + +- **WHEN** an `InputRequest` is shown with `IsSecret=false` and a non-empty `Default` +- **THEN** the paint shows the default's clear-text content (no masking) diff --git a/openspec/changes/api-ergonomics-pass-1/specs/styled-text/spec.md b/openspec/changes/api-ergonomics-pass-1/specs/styled-text/spec.md new file mode 100644 index 0000000..81f39c4 --- /dev/null +++ b/openspec/changes/api-ergonomics-pass-1/specs/styled-text/spec.md @@ -0,0 +1,44 @@ +## ADDED Requirements + +### Requirement: Convenience construction from plain strings + +The library SHALL expose a `Line.FromText(string text, Style? style = null)` static factory and SHALL accept plain `string` values wherever a `Line` is currently required on consumer-facing construction surfaces, so label-only call sites do not have to lift through `LineBuilder`. The library SHALL NOT define an implicit conversion from `string` to `Line`; the factory and the overloads SHALL be the only seams. + +The string-accepting overloads SHALL exist on: +- `IScrollback.Append(string text)` alongside `Append(Line line)`. +- `InputRequest` accepting a `string? Prompt` alongside its existing `Line? Prompt`. +- `SelectRequest` accepting `IReadOnlyList` and `params string[]` items alongside its existing `Line` item list. +- `MultiSelectRequest` accepting `IReadOnlyList` and `params string[]` items alongside its existing `Line` item list. +- `ChoiceRequest` accepting `IReadOnlyList` and `params string[]` options alongside its existing `Line` option list. + +Each overload SHALL be semantically equivalent to constructing the corresponding `Line` via `Line.FromText` with default style and passing it to the existing API. + +#### Scenario: FromText produces an unstyled line + +- **WHEN** a caller invokes `Line.FromText("hello")` with no style argument +- **THEN** the returned `Line` contains a single `Segment` whose text is `"hello"` and whose style is the default (no foreground/background, `Format.None`) + +#### Scenario: FromText respects an explicit style + +- **WHEN** a caller invokes `Line.FromText("err", new Style(Format: Format.Bold))` +- **THEN** the returned `Line` contains a single `Segment` whose text is `"err"` and whose style has `Format.Bold` + +#### Scenario: String-accepting Scrollback.Append is equivalent to the Line form + +- **WHEN** a caller invokes `terminal.Scrollback.Append("hello")` +- **THEN** the appended object is equal to what `terminal.Scrollback.Append(Line.FromText("hello"))` would have produced + +#### Scenario: String-accepting dialog requests are equivalent to the Line form + +- **WHEN** a caller constructs a `SelectRequest` (or `MultiSelectRequest` or `ChoiceRequest`) with `params string[]` or `IReadOnlyList` items +- **THEN** the resulting items are equal to those produced by mapping each string through `Line.FromText` with default style + +#### Scenario: InputRequest accepts a string prompt + +- **WHEN** a caller constructs an `InputRequest` with a `string` prompt +- **THEN** the resulting request's effective prompt is equal to `Line.FromText(prompt)` with default style + +#### Scenario: No implicit conversion is defined + +- **WHEN** a consumer attempts to pass a plain `string` to an API that takes only `Line` (with no string overload added by this change) +- **THEN** the code SHALL fail to compile — there SHALL be no implicit `string`→`Line` conversion defined anywhere in the public surface diff --git a/openspec/changes/api-ergonomics-pass-1/tasks.md b/openspec/changes/api-ergonomics-pass-1/tasks.md new file mode 100644 index 0000000..c1ffbdd --- /dev/null +++ b/openspec/changes/api-ergonomics-pass-1/tasks.md @@ -0,0 +1,53 @@ +## 1. `Line.FromText` factory + +- [ ] 1.1 Add `public static Line FromText(string text, Style? style = null)` to `src/Dcli/Line.cs`; returns a `Line` with a single `Segment(text, style ?? default)` +- [ ] 1.2 XML docs on `FromText` describing it as the canonical short form for label-only `Line`s +- [ ] 1.3 Tests in `tests/Dcli.Tests/StyledTextTests.cs`: default style; explicit style; empty string; multi-rune string + +## 2. String-accepting consumer overloads + +- [ ] 2.1 `src/Dcli/IScrollback.cs` — add `void Append(string text)` on the interface +- [ ] 2.2 `src/Dcli/ScrollbackSurface.cs` — implement `Append(string)` as `Append(Line.FromText(text))` +- [ ] 2.3 `src/Dcli/InputRequest.cs` — add overload accepting `string? Prompt` alongside the existing `Line? Prompt` (preserve `Default`/`IsSecret`) +- [ ] 2.4 `src/Dcli/SelectRequest.cs` — add overloads accepting `IReadOnlyList` and `params string[]` items +- [ ] 2.5 `src/Dcli/MultiSelectRequest.cs` — add overloads accepting `IReadOnlyList` and `params string[]` items +- [ ] 2.6 `src/Dcli/ChoiceRequest.cs` — add overloads accepting `IReadOnlyList` and `params string[]` options +- [ ] 2.7 `tests/Dcli.Tests/FacadeTests.cs` — round-trip tests for each string overload (string form produces the same model state as the explicit-`Line` form) +- [ ] 2.8 `tests/Dcli.Tests/FakeTerminalTests.cs` — extend the tier-A fake with the new overloads (forward to the `Line` variant); assert the fake records them identically +- [ ] 2.9 Compile-fail/lint check: confirm no implicit `string → Line` conversion is defined (no operator on `Line`); document in the XML doc on `Line.FromText` that this is intentional + +## 3. `AllowBack` flag on `SelectRequest` / `ChoiceRequest` + +- [ ] 3.1 `src/Dcli/SelectRequest.cs` — add `bool AllowBack = false` (default false, additive) +- [ ] 3.2 `src/Dcli/ChoiceRequest.cs` — add `bool AllowBack = false` +- [ ] 3.3 `src/Dcli/Internal/FixedRegion/Dialog.cs` — handle Backspace: if `AllowBack && !_hasMoved`, set `CloseRequest = OverlayCloseKind.Back`; otherwise no-op (preserve v1) +- [ ] 3.4 `src/Dcli/Internal/FixedRegion/Dialog.cs` — set `_hasMoved = true` on `↑`/`↓` (or whatever the current movement keys are); confirm Backspace before any movement still produces Back +- [ ] 3.5 Map `OverlayCloseKind.Back` → `DialogOutcome.Back` in the loop's dismiss hook (the wiring already routes Submit/Cancel; add the Back arm) +- [ ] 3.6 Tests in `tests/Dcli.Tests/DialogSelectionTests.cs`: `AllowBack=true` + Backspace-at-empty produces `DialogOutcome.Back`; `AllowBack=true` + `↓` then Backspace is a no-op; `AllowBack=false` (default) + Backspace is a no-op +- [ ] 3.7 Equivalent tests for `ChoiceRequest` in a new or existing `ChoiceDialogTests.cs` +- [ ] 3.8 Confirm `MultiSelectRequest` did NOT receive an `AllowBack` member (this is deliberate; one test asserting it compiles without) + +## 4. Secret-default masking in `InputDialog` + +- [ ] 4.1 `src/Dcli/Internal/FixedRegion/InputDialog.cs` — when `IsSecret && !_userEdited && Default is non-empty`, render the buffer as `'•'` repeated by `DisplayWidth.Measure(Default)` instead of the raw default +- [ ] 4.2 Ensure the existing edit-detection (used for `InputChanged` emission) is the source of truth for `_userEdited`; do not introduce a second flag +- [ ] 4.3 `Submit` returns the real string (assert this — should be unchanged) +- [ ] 4.4 Tests in `tests/Dcli.Tests/InputDialogTests.cs`: secret + default + no edits → masked render; secret + default + Submit → real string; secret + default + one edit then revert → still masked? (decide via the existing `_userEdited` semantics; document in the test) +- [ ] 4.5 Non-secret + default test: paint shows clear-text default (regression guard) + +## 5. Demo updates + +- [ ] 5.1 `samples/Dcli.Demo.DmonWizard/Engine/WizardRenderer.cs` — replace the ~12 `new LineBuilder().Text(s).Build()` sites with `Line.FromText(s)` to validate the ergonomics end-to-end +- [ ] 5.2 Where possible in `samples/Dcli.Demo/`, switch label-only `Line`s to `Line.FromText` or the string overloads +- [ ] 5.3 Add a `AllowBack=true` use case to a wizard step in `Dcli.Demo.DmonWizard` so the new affordance has a live demonstration + +## 6. Validation & packaging + +- [ ] 6.1 `dotnet build -c Release` — 0 warnings +- [ ] 6.2 `dotnet test -c Release` — green (688 baseline + ~12 new) +- [ ] 6.3 `dotnet format --verify-no-changes` — clean +- [ ] 6.4 `openspec validate api-ergonomics-pass-1 --strict` — valid +- [ ] 6.5 Bump `Dcli.csproj` and `Dcli.Testing.csproj` `Version` from `0.1.0-rc.1` → `0.2.0-rc.1` (additive minor) +- [ ] 6.6 `dotnet pack -c Release` produces `dcli.0.2.0-rc.1.{nupkg,snupkg}` + `dcli.testing.0.2.0-rc.1.{nupkg,snupkg}` cleanly +- [ ] 6.7 Update the repo `CLAUDE.md`'s "Where to look for historical context on a shipped change" subsection to match the canonical wording recommended by the personal `devlog` skill at `~/.claude/skills/devlog/SKILL.md` (covers BOTH the in-flight DEVLOG inside the change directory AND the archived DEVLOG — the wording shipped in PR #2 only covered the archived case) +- [ ] 6.8 Update `openspec/changes/api-ergonomics-pass-1/DEVLOG.md` per the `devlog` skill conventions: a row in the Section status table for each section commit, deviations as they happen, resume-point bumped after each section From 3dd518c67f95e667b0f93bc1ff03bec34710987c Mon Sep 17 00:00:00 2001 From: Emmz Rendle Date: Thu, 28 May 2026 13:58:08 +0100 Subject: [PATCH 02/10] feat(api-ergonomics-pass-1): Line.FromText factory (section 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 1.1 Add `public static Line FromText(string text, Style? style = null)` to src/Dcli/Line.cs; returns a Line with a single Segment(text, style ?? default). - 1.2 XML docs on FromText describe it as the canonical short form for label-only Lines, and explicitly state that no implicit string→Line conversion is defined (deliberate API choice; design.md Decision 1). - 1.3 StyledTextTests gains 5 facts covering default style, explicit style, empty string, multi-rune round-trip, and structural-equality with the manual `new Line(new[]{ new Segment(text) })` form. Gates: build 0 warnings, 693 tests green (688 baseline + 5), dotnet format clean, openspec validate --strict clean. Co-Authored-By: Claude Opus 4.7 --- .../changes/api-ergonomics-pass-1/DEVLOG.md | 4 +- .../changes/api-ergonomics-pass-1/tasks.md | 6 +-- src/Dcli/Line.cs | 29 +++++++++++ tests/Dcli.Tests/StyledTextTests.cs | 51 +++++++++++++++++++ 4 files changed, 85 insertions(+), 5 deletions(-) diff --git a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md index ce79c75..7eaae51 100644 --- a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md +++ b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md @@ -23,7 +23,7 @@ One row per `## N.` section in `tasks.md`. Add a row when the section commits. | § | Section | Commit | Tests after | Notes | |---|---------|--------|-------------|-------| -| — | (pending — §1 is next) | — | — | — | +| 1 | `Line.FromText` factory | _pending_ | 693 (688 + 5) | XML doc also states "no implicit `string → Line` conversion is defined" — covers the doc half of task 2.9 ahead of §2. | ## Decisions & deviations @@ -58,4 +58,4 @@ Surface gaps for future changes. Link to memory files where the constraint is en ## Resume point -> **Currently at §1.1 — Add `public static Line FromText(string text, Style? style = null)` to `src/Dcli/Line.cs`.** Working tree is clean. Next worker brief: implement §1 (`Line.FromText` factory) end-to-end, including XML docs (1.2) and tests (1.3). Single small section; expect one worker call, one reviewer audit, one commit. +> **Currently at §2.1 — `IScrollback.Append(string text)` overload.** §1 shipped (`Line.FromText` factory + 5 tests; build 0 warnings, 693 tests green, format/validate clean). Next worker brief: implement §2 end-to-end — `IScrollback.Append(string)` (2.1–2.2), the `*Request` string overloads (2.3–2.6), `FacadeTests` round-trips (2.7), `FakeTerminalTests` tier-A symmetry (2.8), and confirm no implicit conversion via grep (2.9; the doc-half is already in `Line.FromText`'s XML). diff --git a/openspec/changes/api-ergonomics-pass-1/tasks.md b/openspec/changes/api-ergonomics-pass-1/tasks.md index c1ffbdd..1ae4c7a 100644 --- a/openspec/changes/api-ergonomics-pass-1/tasks.md +++ b/openspec/changes/api-ergonomics-pass-1/tasks.md @@ -1,8 +1,8 @@ ## 1. `Line.FromText` factory -- [ ] 1.1 Add `public static Line FromText(string text, Style? style = null)` to `src/Dcli/Line.cs`; returns a `Line` with a single `Segment(text, style ?? default)` -- [ ] 1.2 XML docs on `FromText` describing it as the canonical short form for label-only `Line`s -- [ ] 1.3 Tests in `tests/Dcli.Tests/StyledTextTests.cs`: default style; explicit style; empty string; multi-rune string +- [x] 1.1 Add `public static Line FromText(string text, Style? style = null)` to `src/Dcli/Line.cs`; returns a `Line` with a single `Segment(text, style ?? default)` +- [x] 1.2 XML docs on `FromText` describing it as the canonical short form for label-only `Line`s +- [x] 1.3 Tests in `tests/Dcli.Tests/StyledTextTests.cs`: default style; explicit style; empty string; multi-rune string ## 2. String-accepting consumer overloads diff --git a/src/Dcli/Line.cs b/src/Dcli/Line.cs index 05ee8f8..65aad18 100644 --- a/src/Dcli/Line.cs +++ b/src/Dcli/Line.cs @@ -30,6 +30,35 @@ public Line(IEnumerable segments) { } + /// + /// Canonical short form for a label-only — wraps a single plain-text + /// string in one with an optional . + /// + /// Use this factory when the entire line is a single unstyled (or uniformly styled) string. + /// For multi-segment lines use instead. + /// + /// + /// No implicit conversion from to is defined. + /// This is a deliberate API choice: an implicit conversion would let callers accidentally pass + /// a bare where a styled was intended, silently + /// discarding any styling. Use Line.FromText(s) explicitly at every call site. + /// + /// + /// The text content. Must not be . + /// + /// The style to apply to the segment. When the segment carries + /// default(Style) (terminal defaults, no formatting attributes). + /// + /// + /// A containing a single with the given + /// and resolved style. + /// + /// + /// Thrown when is . + /// + public static Line FromText(string text, Style? style = null) => + new(new[] { new Segment(text ?? throw new ArgumentNullException(nameof(text)), style ?? default) }); + /// /// Returns when contains exactly the same /// segments in the same order. diff --git a/tests/Dcli.Tests/StyledTextTests.cs b/tests/Dcli.Tests/StyledTextTests.cs index 0cebb7a..c014f91 100644 --- a/tests/Dcli.Tests/StyledTextTests.cs +++ b/tests/Dcli.Tests/StyledTextTests.cs @@ -261,6 +261,57 @@ public void LineEmptyIsValid() Assert.Empty(line.Segments); } + // ----------------------------------------------------------------------------------------- + // 2.3 Line.FromText + // ----------------------------------------------------------------------------------------- + + [Fact] + public void LineFromTextDefaultStyleProducesUnstyledSegment() + { + // Spec: FromText produces an unstyled line — no style argument → default(Style). + Line line = Line.FromText("hello"); + Assert.Single(line.Segments); + Assert.Equal("hello", line.Segments[0].Text); + Assert.Equal(default, line.Segments[0].Style); + } + + [Fact] + public void LineFromTextExplicitStyleIsCarried() + { + // Spec: FromText respects an explicit style. + Style bold = new(Format: Format.Bold); + Line line = Line.FromText("err", bold); + Assert.Single(line.Segments); + Assert.Equal("err", line.Segments[0].Text); + Assert.True(line.Segments[0].Style.Format.HasFlag(Format.Bold)); + } + + [Fact] + public void LineFromTextEmptyStringProducesSingleEmptySegment() + { + Line line = Line.FromText(""); + Assert.Single(line.Segments); + Assert.Equal("", line.Segments[0].Text); + } + + [Fact] + public void LineFromTextMultiRuneStringRoundTripsTextIntact() + { + // Unicode content (combining accent + emoji) must survive unchanged. + const string text = "héllo🦊"; + Line line = Line.FromText(text); + Assert.Equal(text, line.Segments[0].Text); + } + + [Fact] + public void LineFromTextStructurallyEqualsManualConstruction() + { + // Line has sequence equality; FromText must produce the same value as the long form. + Line fromFactory = Line.FromText("hello"); + Line manual = new(new[] { new Segment("hello") }); + Assert.Equal(manual, fromFactory); + } + // ----------------------------------------------------------------------------------------- // 2.3 LineBuilder // ----------------------------------------------------------------------------------------- From 55cd4de96bc3d6ffd29a082515293c25787a3260 Mon Sep 17 00:00:00 2001 From: Emmz Rendle Date: Thu, 28 May 2026 14:09:26 +0100 Subject: [PATCH 03/10] feat(api-ergonomics-pass-1): string-accepting consumer overloads (section 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 2.1 Add `void Append(string text)` to `IScrollback` in src/Dcli/ITerminal.cs. - 2.2 Implement `Append(string)` in ScrollbackSurface as a null-guarded forwarder to `Append(Line.FromText(text))`. - 2.3 Add `InputRequest(string? prompt, string? Default = null, bool IsSecret = false)` secondary ctor chaining to the existing `Line?` primary. - 2.4-2.6 Add `IReadOnlyList` and `params string[]` ctors to SelectRequest, MultiSelectRequest, ChoiceRequest. All route items through `Line.FromText` with default style, preserving the existing `Title`/`Prompt` as `Line?`. Null-guards run before LINQ materialisation. - 2.7 FacadeTests gains a §2 round-trip section: each new overload is asserted equal to the canonical `Line` form via `Line` sequence equality, with one non-ASCII case (`"héllo🦊"`) covering the forwarding path. - 2.8 FakeTerminalTests' tier-A FakeScrollback implements the new `Append(string)` and asserts string/Line forms record identically. - 2.9 No `implicit operator` exists anywhere in src/; the doc seam stays on `Line.FromText` (added in §1). File layout deviation noted in DEVLOG: tasks.md named per-record files that don't exist (IScrollback lives in ITerminal.cs; all four request records live in DialogRequests.cs); implemented in the real files. `new InputRequest(null)` is ambiguous (documented behaviour); the natural `new InputRequest()` and the explicit `(string?)null` / `(Line?)null` forms are unambiguous and covered by tests. Gates: build 0 warnings, 702 tests green (+9 over §1), format clean, openspec validate --strict clean, `grep -rn 'implicit operator' src/` empty. Co-Authored-By: Claude Opus 4.7 --- .../changes/api-ergonomics-pass-1/DEVLOG.md | 9 +- .../changes/api-ergonomics-pass-1/tasks.md | 18 +-- src/Dcli/DialogRequests.cs | 91 +++++++++++++- src/Dcli/ITerminal.cs | 7 ++ src/Dcli/ScrollbackSurface.cs | 9 ++ tests/Dcli.Tests/FacadeTests.cs | 112 ++++++++++++++++++ tests/Dcli.Tests/FakeTerminalTests.cs | 28 +++++ 7 files changed, 258 insertions(+), 16 deletions(-) diff --git a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md index 7eaae51..ba4e307 100644 --- a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md +++ b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md @@ -23,7 +23,8 @@ One row per `## N.` section in `tasks.md`. Add a row when the section commits. | § | Section | Commit | Tests after | Notes | |---|---------|--------|-------------|-------| -| 1 | `Line.FromText` factory | _pending_ | 693 (688 + 5) | XML doc also states "no implicit `string → Line` conversion is defined" — covers the doc half of task 2.9 ahead of §2. | +| 1 | `Line.FromText` factory | `3dd518c` | 693 (688 + 5) | XML doc also states "no implicit `string → Line` conversion is defined" — covers the doc half of task 2.9 ahead of §2. | +| 2 | String-accepting consumer overloads | _pending_ | 702 (693 + 9) | Tasks.md target paths (`IScrollback.cs`, `*Request.cs` per record) didn't match real layout; actually edited `ITerminal.cs` (interface) and `DialogRequests.cs` (all four records together). `new InputRequest(null)` is ambiguous between the `Line?` primary and the new `string?` secondary; resolved at call sites via `new InputRequest()` (primary all-defaulted) or explicit `(string?)null` / `Line.FromText(...)`. Tests cover both. | ## Decisions & deviations @@ -33,7 +34,9 @@ Narrative log of anything that wasn't a straight read-off-the-spec-and-implement - Implicit `string → Line` conversion is **rejected** (Decision 1) — if a consumer migration would have been much cleaner with the implicit, log the case so a future change can revisit. - Secret-default masking reuses the existing `_userEdited` flag (Decision 5) — if reviewer finds the flag is dirty (programmatic `SetText` mutates without flipping it), the resolution goes here. -*(No decisions logged yet — change has not started.)* +**§2 — `tasks.md` paths deviate from real layout.** Tasks 2.1, 2.3–2.6 named per-record files (`src/Dcli/IScrollback.cs`, `src/Dcli/InputRequest.cs`, `src/Dcli/SelectRequest.cs`, etc.) that don't exist. The actual layout consolidates: `IScrollback` lives in `src/Dcli/ITerminal.cs` next to the façade interface, and all four `*Request` records live in `src/Dcli/DialogRequests.cs`. Implemented in the real files; did not split them out (that would have been scope creep — Decision 6 is surface-only). No spec amendment needed; the file paths in `tasks.md` were ergonomic shorthand for "the file that contains this type". + +**§2 — `new InputRequest(null)` is ambiguous (expected).** With both `InputRequest(Line? Prompt = null, …)` and `InputRequest(string? prompt, …)` present, a bare `null` literal as the first argument cannot resolve. This is documented behaviour for C# overload resolution and matches the design's "explicit only" stance (Decision 1). Mitigation: the natural empty-input call `new InputRequest()` resolves unambiguously to the primary (all defaults); callers who want a null prompt explicitly use `new InputRequest((string?)null)` or `new InputRequest((Line?)null)`. Both forms covered by `FacadeTests` (`InputRequestDefaultCtorRemainsUnambiguous`, `InputRequestNullStringPromptProducesNullPrompt`). ## Human-in-the-loop verifications @@ -58,4 +61,4 @@ Surface gaps for future changes. Link to memory files where the constraint is en ## Resume point -> **Currently at §2.1 — `IScrollback.Append(string text)` overload.** §1 shipped (`Line.FromText` factory + 5 tests; build 0 warnings, 693 tests green, format/validate clean). Next worker brief: implement §2 end-to-end — `IScrollback.Append(string)` (2.1–2.2), the `*Request` string overloads (2.3–2.6), `FacadeTests` round-trips (2.7), `FakeTerminalTests` tier-A symmetry (2.8), and confirm no implicit conversion via grep (2.9; the doc-half is already in `Line.FromText`'s XML). +> **Currently at §3.1 — `AllowBack` flag on `SelectRequest`.** §2 shipped (string-accepting overloads across `IScrollback`/`*Request`s + tier-A fake symmetry + facade round-trips; 702 tests green, 0 warnings, format/validate clean, zero `implicit operator` hits in `src/`). Next worker brief: implement §3 end-to-end — add `bool AllowBack = false` to `SelectRequest`/`ChoiceRequest` (3.1–3.2), wire Backspace-at-empty in `Dialog.cs` (3.3–3.4), map `OverlayCloseKind.Back` → `DialogOutcome.Back` in the dismiss hook (3.5), tests in `DialogSelectionTests`/`ChoiceDialogTests` (3.6–3.7), and confirm `MultiSelectRequest` deliberately lacks `AllowBack` (3.8). diff --git a/openspec/changes/api-ergonomics-pass-1/tasks.md b/openspec/changes/api-ergonomics-pass-1/tasks.md index 1ae4c7a..7147f2a 100644 --- a/openspec/changes/api-ergonomics-pass-1/tasks.md +++ b/openspec/changes/api-ergonomics-pass-1/tasks.md @@ -6,15 +6,15 @@ ## 2. String-accepting consumer overloads -- [ ] 2.1 `src/Dcli/IScrollback.cs` — add `void Append(string text)` on the interface -- [ ] 2.2 `src/Dcli/ScrollbackSurface.cs` — implement `Append(string)` as `Append(Line.FromText(text))` -- [ ] 2.3 `src/Dcli/InputRequest.cs` — add overload accepting `string? Prompt` alongside the existing `Line? Prompt` (preserve `Default`/`IsSecret`) -- [ ] 2.4 `src/Dcli/SelectRequest.cs` — add overloads accepting `IReadOnlyList` and `params string[]` items -- [ ] 2.5 `src/Dcli/MultiSelectRequest.cs` — add overloads accepting `IReadOnlyList` and `params string[]` items -- [ ] 2.6 `src/Dcli/ChoiceRequest.cs` — add overloads accepting `IReadOnlyList` and `params string[]` options -- [ ] 2.7 `tests/Dcli.Tests/FacadeTests.cs` — round-trip tests for each string overload (string form produces the same model state as the explicit-`Line` form) -- [ ] 2.8 `tests/Dcli.Tests/FakeTerminalTests.cs` — extend the tier-A fake with the new overloads (forward to the `Line` variant); assert the fake records them identically -- [ ] 2.9 Compile-fail/lint check: confirm no implicit `string → Line` conversion is defined (no operator on `Line`); document in the XML doc on `Line.FromText` that this is intentional +- [x] 2.1 `src/Dcli/IScrollback.cs` — add `void Append(string text)` on the interface +- [x] 2.2 `src/Dcli/ScrollbackSurface.cs` — implement `Append(string)` as `Append(Line.FromText(text))` +- [x] 2.3 `src/Dcli/InputRequest.cs` — add overload accepting `string? Prompt` alongside the existing `Line? Prompt` (preserve `Default`/`IsSecret`) +- [x] 2.4 `src/Dcli/SelectRequest.cs` — add overloads accepting `IReadOnlyList` and `params string[]` items +- [x] 2.5 `src/Dcli/MultiSelectRequest.cs` — add overloads accepting `IReadOnlyList` and `params string[]` items +- [x] 2.6 `src/Dcli/ChoiceRequest.cs` — add overloads accepting `IReadOnlyList` and `params string[]` options +- [x] 2.7 `tests/Dcli.Tests/FacadeTests.cs` — round-trip tests for each string overload (string form produces the same model state as the explicit-`Line` form) +- [x] 2.8 `tests/Dcli.Tests/FakeTerminalTests.cs` — extend the tier-A fake with the new overloads (forward to the `Line` variant); assert the fake records them identically +- [x] 2.9 Compile-fail/lint check: confirm no implicit `string → Line` conversion is defined (no operator on `Line`); document in the XML doc on `Line.FromText` that this is intentional ## 3. `AllowBack` flag on `SelectRequest` / `ChoiceRequest` diff --git a/src/Dcli/DialogRequests.cs b/src/Dcli/DialogRequests.cs index f94d328..4bf5c98 100644 --- a/src/Dcli/DialogRequests.cs +++ b/src/Dcli/DialogRequests.cs @@ -6,7 +6,31 @@ namespace Dcli; /// The list items to display. An empty list is allowed; Submit on an /// empty list returns with value -1. /// Optional leading title row rendered above the list. -public sealed record SelectRequest(IReadOnlyList Items, Line? Title = null); +public sealed record SelectRequest(IReadOnlyList Items, Line? Title = null) +{ + /// + /// Constructs a from plain-text item strings. + /// Shorthand equivalent to passing items.Select(Line.FromText).ToList() as Items. + /// + /// The plain-text items to display. + /// Optional leading title row. + public SelectRequest(IReadOnlyList items, Line? title = null) + : this(ConvertItems(items), title) { } + + /// + /// Constructs a from a params array of plain-text item strings. + /// Shorthand equivalent to passing items.Select(Line.FromText).ToList() as Items. + /// + /// The plain-text items to display. + public SelectRequest(params string[] items) + : this((IReadOnlyList)items, null) { } + + private static List ConvertItems(IReadOnlyList items) + { + ArgumentNullException.ThrowIfNull(items); + return items.Select(s => Line.FromText(s)).ToList(); + } +} /// /// Parameters for — a multi-select list dialog where @@ -14,7 +38,31 @@ public sealed record SelectRequest(IReadOnlyList Items, Line? Title = null /// /// The list items to display. /// Optional leading title row rendered above the list. -public sealed record MultiSelectRequest(IReadOnlyList Items, Line? Title = null); +public sealed record MultiSelectRequest(IReadOnlyList Items, Line? Title = null) +{ + /// + /// Constructs a from plain-text item strings. + /// Shorthand equivalent to passing items.Select(Line.FromText).ToList() as Items. + /// + /// The plain-text items to display. + /// Optional leading title row. + public MultiSelectRequest(IReadOnlyList items, Line? title = null) + : this(ConvertItems(items), title) { } + + /// + /// Constructs a from a params array of plain-text item strings. + /// Shorthand equivalent to passing items.Select(Line.FromText).ToList() as Items. + /// + /// The plain-text items to display. + public MultiSelectRequest(params string[] items) + : this((IReadOnlyList)items, null) { } + + private static List ConvertItems(IReadOnlyList items) + { + ArgumentNullException.ThrowIfNull(items); + return items.Select(s => Line.FromText(s)).ToList(); + } +} /// /// Parameters for — a single-select choice dialog. @@ -23,7 +71,31 @@ public sealed record MultiSelectRequest(IReadOnlyList Items, Line? Title = /// /// The choice options to display. /// Optional leading prompt row rendered above the options. -public sealed record ChoiceRequest(IReadOnlyList Options, Line? Prompt = null); +public sealed record ChoiceRequest(IReadOnlyList Options, Line? Prompt = null) +{ + /// + /// Constructs a from plain-text option strings. + /// Shorthand equivalent to passing options.Select(Line.FromText).ToList() as Options. + /// + /// The plain-text options to display. + /// Optional leading prompt row. + public ChoiceRequest(IReadOnlyList options, Line? prompt = null) + : this(ConvertOptions(options), prompt) { } + + /// + /// Constructs a from a params array of plain-text option strings. + /// Shorthand equivalent to passing options.Select(Line.FromText).ToList() as Options. + /// + /// The plain-text options to display. + public ChoiceRequest(params string[] options) + : this((IReadOnlyList)options, null) { } + + private static List ConvertOptions(IReadOnlyList options) + { + ArgumentNullException.ThrowIfNull(options); + return options.Select(s => Line.FromText(s)).ToList(); + } +} /// /// Parameters for — a free-text entry dialog. @@ -41,4 +113,15 @@ public sealed record ChoiceRequest(IReadOnlyList Options, Line? Prompt = n /// in the rendered overlay, preserving column-width arithmetic. The /// always carries the real (unmasked) entered text. /// -public sealed record InputRequest(Line? Prompt = null, string? Default = null, bool IsSecret = false); +public sealed record InputRequest(Line? Prompt = null, string? Default = null, bool IsSecret = false) +{ + /// + /// Constructs an with a plain-text prompt string. + /// Shorthand equivalent to passing Line.FromText(prompt) as Prompt. + /// + /// Optional plain-text prompt string; means no prompt row. + /// Optional pre-filled text. + /// When , input characters are masked. + public InputRequest(string? prompt, string? Default = null, bool IsSecret = false) + : this(prompt is null ? null : Line.FromText(prompt), Default, IsSecret) { } +} diff --git a/src/Dcli/ITerminal.cs b/src/Dcli/ITerminal.cs index dba194a..cbb9ea4 100644 --- a/src/Dcli/ITerminal.cs +++ b/src/Dcli/ITerminal.cs @@ -21,6 +21,13 @@ public interface IScrollback /// The line to append. void Append(Line line); + /// + /// Appends a plain-text line to the scrollback live window. + /// Shorthand equivalent to Append(Line.FromText(text)). + /// + /// The plain text to append as a single unstyled line. + void Append(string text); + /// /// Begins a new live block in the scrollback, returning a handle for incremental mutation. /// diff --git a/src/Dcli/ScrollbackSurface.cs b/src/Dcli/ScrollbackSurface.cs index 26512ee..74a3554 100644 --- a/src/Dcli/ScrollbackSurface.cs +++ b/src/Dcli/ScrollbackSurface.cs @@ -106,6 +106,15 @@ public void Append(Line line) _loop.Post(new AppendToScrollbackCommand(line)); } + /// + /// Appends a plain-text line to the scrollback live window. + /// + public void Append(string text) + { + ArgumentNullException.ThrowIfNull(text); + Append(Line.FromText(text)); + } + /// /// Begins a new live block in the scrollback, returning a handle for incremental mutation. /// diff --git a/tests/Dcli.Tests/FacadeTests.cs b/tests/Dcli.Tests/FacadeTests.cs index f2a0a21..eba38fe 100644 --- a/tests/Dcli.Tests/FacadeTests.cs +++ b/tests/Dcli.Tests/FacadeTests.cs @@ -599,4 +599,116 @@ public async Task CtrlEnterFallsThroughAsKeyPressedNotSubmitted() Assert.DoesNotContain(events, e => e is InputSubmitted); } } + + // ───────────────────────────────────────────────────────────────────────── + // §2 — String-overload round-trip tests + // ───────────────────────────────────────────────────────────────────────── + + [Fact] + public async Task ScrollbackAppendStringProducesSameLiveRowAsAppendLine() + { + // IScrollback.Append("hello") must produce the same visible row as + // IScrollback.Append(Line.FromText("hello")). + (DcliTerminal termLine, CapturingOutputSink sinkLine, VirtualClock clockLine, _) = CreateTerminal(); + (DcliTerminal termString, CapturingOutputSink sinkString, VirtualClock clockString, _) = CreateTerminal(); + + await using (termLine) + await using (termString) + { + termLine.Scrollback.Append(Line.FromText("round-trip")); + await SettleAsync(termLine, clockLine); + + termString.Scrollback.Append("round-trip"); + await SettleAsync(termString, clockString); + + Assert.NotNull(sinkLine.LastModel); + Assert.NotNull(sinkString.LastModel); + + bool foundViaLine = sinkLine.LastModel.LiveWindowRows + .Any(row => row.Segments.Any(s => s.Text.Contains("round-trip", StringComparison.Ordinal))); + bool foundViaString = sinkString.LastModel.LiveWindowRows + .Any(row => row.Segments.Any(s => s.Text.Contains("round-trip", StringComparison.Ordinal))); + + Assert.True(foundViaLine, "Append(Line) should produce a live row with text 'round-trip'."); + Assert.True(foundViaString, "Append(string) should produce a live row with text 'round-trip'."); + } + } + + [Fact] + public void SelectRequestParamsStringAndIReadOnlyListStringAndLineListAreEqual() + { + // All three construction forms must produce SelectRequest.Items with the same segments. + SelectRequest viaParams = new("a", "b"); + SelectRequest viaList = new(new List { "a", "b" }); + SelectRequest viaLineList = new([Line.FromText("a"), Line.FromText("b")]); + + Assert.Equal(viaLineList.Items, viaParams.Items); + Assert.Equal(viaLineList.Items, viaList.Items); + } + + [Fact] + public void MultiSelectRequestParamsStringAndIReadOnlyListStringAndLineListAreEqual() + { + MultiSelectRequest viaParams = new("a", "b"); + MultiSelectRequest viaList = new(new List { "a", "b" }); + MultiSelectRequest viaLineList = new([Line.FromText("a"), Line.FromText("b")]); + + Assert.Equal(viaLineList.Items, viaParams.Items); + Assert.Equal(viaLineList.Items, viaList.Items); + } + + [Fact] + public void ChoiceRequestParamsStringAndIReadOnlyListStringAndLineListAreEqual() + { + ChoiceRequest viaParams = new("yes", "no"); + ChoiceRequest viaList = new(new List { "yes", "no" }); + ChoiceRequest viaLineList = new([Line.FromText("yes"), Line.FromText("no")]); + + Assert.Equal(viaLineList.Options, viaParams.Options); + Assert.Equal(viaLineList.Options, viaList.Options); + } + + [Fact] + public void ChoiceRequestNonAsciiRoundTrip() + { + // Verify that the string→Line.FromText forwarding path preserves non-ASCII text intact. + ChoiceRequest viaParams = new("héllo🦊", "café"); + ChoiceRequest viaLineList = new([Line.FromText("héllo🦊"), Line.FromText("café")]); + + Assert.Equal(viaLineList.Options, viaParams.Options); + } + + [Fact] + public void InputRequestStringPromptProducesSamePromptAsLineFromText() + { + InputRequest viaString = new InputRequest("hi"); + InputRequest viaLine = new InputRequest(Line.FromText("hi")); + + Assert.NotNull(viaString.Prompt); + Assert.NotNull(viaLine.Prompt); + Assert.Equal(viaLine.Prompt, viaString.Prompt); + + // Defaults preserved. + Assert.Null(viaString.Default); + Assert.False(viaString.IsSecret); + } + + [Fact] + public void InputRequestDefaultCtorRemainsUnambiguous() + { + // new InputRequest() must compile and produce null Prompt. + InputRequest req = new(); + Assert.Null(req.Prompt); + Assert.Null(req.Default); + Assert.False(req.IsSecret); + } + + [Fact] + public void InputRequestNullStringPromptProducesNullPrompt() + { + // new InputRequest((string?)null) must forward null → null Prompt. + InputRequest req = new InputRequest((string?)null); + Assert.Null(req.Prompt); + } + } diff --git a/tests/Dcli.Tests/FakeTerminalTests.cs b/tests/Dcli.Tests/FakeTerminalTests.cs index d97dba0..52de19d 100644 --- a/tests/Dcli.Tests/FakeTerminalTests.cs +++ b/tests/Dcli.Tests/FakeTerminalTests.cs @@ -24,6 +24,12 @@ public void Append(Line line) Appended.Add(line); } + public void Append(string text) + { + ArgumentNullException.ThrowIfNull(text); + Append(Line.FromText(text)); + } + public ILiveBlock BeginLive() { BeginLiveCount++; @@ -492,4 +498,26 @@ public async Task GetTerminalSizeReturnsConfiguredValue() Assert.Equal(24, rows); } } + + // ── §2 — string-overload symmetry ──────────────────────────────────────── + + [Fact] + public void FakeScrollbackAppendStringProducesSameRecordingAsAppendLine() + { + FakeScrollback fake = new(); + + // Two independent fakes, same text — one called with Line, one with string. + FakeScrollback fakeViaLine = new(); + FakeScrollback fakeViaString = new(); + + fakeViaLine.Append(Line.FromText("hello")); + fakeViaString.Append("hello"); + + // Both record exactly one line with the same segment text. + Assert.Single(fakeViaLine.Appended); + Assert.Single(fakeViaString.Appended); + Assert.Equal( + fakeViaLine.Appended[0].Segments.Select(s => s.Text), + fakeViaString.Appended[0].Segments.Select(s => s.Text)); + } } From fb90d3490c9db3023a3e0f9b54a003b4f215431f Mon Sep 17 00:00:00 2001 From: Emmz Rendle Date: Thu, 28 May 2026 14:21:27 +0100 Subject: [PATCH 04/10] feat(api-ergonomics-pass-1): AllowBack flag on SelectRequest/ChoiceRequest (section 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 3.1 SelectRequest gains `bool AllowBack = false` as a third positional parameter (additive, backward-compatible). String-accepting secondary ctors from §2 forward it. - 3.2 ChoiceRequest gains the same `AllowBack` parameter and secondary-ctor forwarding. - 3.3 Dialog.HandleKey gains a new Back rule between Escape and arrow nav: Backspace fires `OverlayCloseKind.Back` when `_allowBack && !_hasMoved && _filterText.Length == 0`. The `_filterText.Length == 0` guard defends against any future widening of `TypeToFilter` to Select/Choice. - 3.4 The ↑/↓ arms now flip `_hasMoved = true` before MoveUp/MoveDown so Backspace post-movement is suppressed for the rest of the overlay session. - 3.5 `OverlayCloseKind` gains a `Back` member. `Terminal.OpenModalAsync` replaces the Submit/Cancel ternary with a switch expression: Submit → buildResult, Back → DialogResult(Back, default), default → Cancelled (covers both `Cancel` and `null` defensively). `SelectAsync`/`ChoiceAsync` forward `req.AllowBack` into the Dialog ctor. `MultiSelectAsync` does NOT pass `allowBack` — MultiSelect cannot return Back. - 3.6 DialogSelectionTests gains a §3 block: AllowBack=true + Backspace-at-empty produces Back; AllowBack=true after ↓ is a no-op (dialog stays open; Esc produces Cancelled); AllowBack=false (default) ignores Backspace. - 3.7 New ChoiceDialogTests.cs with the equivalent three Choice tests. - 3.8 Reflection test pins `MultiSelectRequest` deliberately omitting AllowBack (Decision 4). - DialogOutcome.Back XML doc updated: no longer "Not produced by any v1 dialog" — now produced under opt-in AllowBack=true. Gates: build 0 warnings, 709 tests green (+7 over §2), format clean, openspec validate --strict clean. Co-Authored-By: Claude Opus 4.7 --- .../changes/api-ergonomics-pass-1/DEVLOG.md | 5 +- .../changes/api-ergonomics-pass-1/tasks.md | 16 +- src/Dcli/DialogOutcome.cs | 15 +- src/Dcli/DialogRequests.cs | 32 ++- src/Dcli/Internal/FixedRegion/Dialog.cs | 40 ++- src/Dcli/Internal/FixedRegion/IOverlay.cs | 6 + src/Dcli/Terminal.cs | 17 +- tests/Dcli.Tests/ChoiceDialogTests.cs | 242 ++++++++++++++++++ tests/Dcli.Tests/DialogSelectionTests.cs | 122 +++++++++ 9 files changed, 461 insertions(+), 34 deletions(-) create mode 100644 tests/Dcli.Tests/ChoiceDialogTests.cs diff --git a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md index ba4e307..62b48e5 100644 --- a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md +++ b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md @@ -24,7 +24,8 @@ One row per `## N.` section in `tasks.md`. Add a row when the section commits. | § | Section | Commit | Tests after | Notes | |---|---------|--------|-------------|-------| | 1 | `Line.FromText` factory | `3dd518c` | 693 (688 + 5) | XML doc also states "no implicit `string → Line` conversion is defined" — covers the doc half of task 2.9 ahead of §2. | -| 2 | String-accepting consumer overloads | _pending_ | 702 (693 + 9) | Tasks.md target paths (`IScrollback.cs`, `*Request.cs` per record) didn't match real layout; actually edited `ITerminal.cs` (interface) and `DialogRequests.cs` (all four records together). `new InputRequest(null)` is ambiguous between the `Line?` primary and the new `string?` secondary; resolved at call sites via `new InputRequest()` (primary all-defaulted) or explicit `(string?)null` / `Line.FromText(...)`. Tests cover both. | +| 2 | String-accepting consumer overloads | `55cd4de` | 702 (693 + 9) | Tasks.md target paths (`IScrollback.cs`, `*Request.cs` per record) didn't match real layout; actually edited `ITerminal.cs` (interface) and `DialogRequests.cs` (all four records together). `new InputRequest(null)` is ambiguous between the `Line?` primary and the new `string?` secondary; resolved at call sites via `new InputRequest()` (primary all-defaulted) or explicit `(string?)null` / `Line.FromText(...)`. Tests cover both. | +| 3 | `AllowBack` flag on `SelectRequest`/`ChoiceRequest` | _pending_ | 709 (702 + 7) | No separate `ChoiceDialog` — `Choice` reuses the unified `Dialog` (`Terminal.cs:195`); created new `ChoiceDialogTests.cs` rather than splitting the class. `OverlayCloseKind` enum ordering becomes `Submit, Back, Cancel` (internal enum so no ABI consumer impact). The `Dialog.HandleKey` Backspace-Back branch is placed BEFORE the existing `TypeToFilter` Backspace-trim branch, with a `_filterText.Length == 0` guard defending against any future widening of `TypeToFilter` to Select/Choice. | ## Decisions & deviations @@ -61,4 +62,4 @@ Surface gaps for future changes. Link to memory files where the constraint is en ## Resume point -> **Currently at §3.1 — `AllowBack` flag on `SelectRequest`.** §2 shipped (string-accepting overloads across `IScrollback`/`*Request`s + tier-A fake symmetry + facade round-trips; 702 tests green, 0 warnings, format/validate clean, zero `implicit operator` hits in `src/`). Next worker brief: implement §3 end-to-end — add `bool AllowBack = false` to `SelectRequest`/`ChoiceRequest` (3.1–3.2), wire Backspace-at-empty in `Dialog.cs` (3.3–3.4), map `OverlayCloseKind.Back` → `DialogOutcome.Back` in the dismiss hook (3.5), tests in `DialogSelectionTests`/`ChoiceDialogTests` (3.6–3.7), and confirm `MultiSelectRequest` deliberately lacks `AllowBack` (3.8). +> **Currently at §4.1 — Secret-default masking in `InputDialog`.** §3 shipped (`AllowBack` flag + Backspace-at-empty wiring + 7 tests; 709 tests green, 0 warnings, format/validate clean). Next worker brief: implement §4 end-to-end — mask the seeded `Default` when `IsSecret && !_userEdited` (4.1), reuse the existing edit-detection flag (4.2), confirm `Submit` returns the real string (4.3), tests in `InputDialogTests` (4.4–4.5). diff --git a/openspec/changes/api-ergonomics-pass-1/tasks.md b/openspec/changes/api-ergonomics-pass-1/tasks.md index 7147f2a..d077133 100644 --- a/openspec/changes/api-ergonomics-pass-1/tasks.md +++ b/openspec/changes/api-ergonomics-pass-1/tasks.md @@ -18,14 +18,14 @@ ## 3. `AllowBack` flag on `SelectRequest` / `ChoiceRequest` -- [ ] 3.1 `src/Dcli/SelectRequest.cs` — add `bool AllowBack = false` (default false, additive) -- [ ] 3.2 `src/Dcli/ChoiceRequest.cs` — add `bool AllowBack = false` -- [ ] 3.3 `src/Dcli/Internal/FixedRegion/Dialog.cs` — handle Backspace: if `AllowBack && !_hasMoved`, set `CloseRequest = OverlayCloseKind.Back`; otherwise no-op (preserve v1) -- [ ] 3.4 `src/Dcli/Internal/FixedRegion/Dialog.cs` — set `_hasMoved = true` on `↑`/`↓` (or whatever the current movement keys are); confirm Backspace before any movement still produces Back -- [ ] 3.5 Map `OverlayCloseKind.Back` → `DialogOutcome.Back` in the loop's dismiss hook (the wiring already routes Submit/Cancel; add the Back arm) -- [ ] 3.6 Tests in `tests/Dcli.Tests/DialogSelectionTests.cs`: `AllowBack=true` + Backspace-at-empty produces `DialogOutcome.Back`; `AllowBack=true` + `↓` then Backspace is a no-op; `AllowBack=false` (default) + Backspace is a no-op -- [ ] 3.7 Equivalent tests for `ChoiceRequest` in a new or existing `ChoiceDialogTests.cs` -- [ ] 3.8 Confirm `MultiSelectRequest` did NOT receive an `AllowBack` member (this is deliberate; one test asserting it compiles without) +- [x] 3.1 `src/Dcli/SelectRequest.cs` — add `bool AllowBack = false` (default false, additive) +- [x] 3.2 `src/Dcli/ChoiceRequest.cs` — add `bool AllowBack = false` +- [x] 3.3 `src/Dcli/Internal/FixedRegion/Dialog.cs` — handle Backspace: if `AllowBack && !_hasMoved`, set `CloseRequest = OverlayCloseKind.Back`; otherwise no-op (preserve v1) +- [x] 3.4 `src/Dcli/Internal/FixedRegion/Dialog.cs` — set `_hasMoved = true` on `↑`/`↓` (or whatever the current movement keys are); confirm Backspace before any movement still produces Back +- [x] 3.5 Map `OverlayCloseKind.Back` → `DialogOutcome.Back` in the loop's dismiss hook (the wiring already routes Submit/Cancel; add the Back arm) +- [x] 3.6 Tests in `tests/Dcli.Tests/DialogSelectionTests.cs`: `AllowBack=true` + Backspace-at-empty produces `DialogOutcome.Back`; `AllowBack=true` + `↓` then Backspace is a no-op; `AllowBack=false` (default) + Backspace is a no-op +- [x] 3.7 Equivalent tests for `ChoiceRequest` in a new or existing `ChoiceDialogTests.cs` +- [x] 3.8 Confirm `MultiSelectRequest` did NOT receive an `AllowBack` member (this is deliberate; one test asserting it compiles without) ## 4. Secret-default masking in `InputDialog` diff --git a/src/Dcli/DialogOutcome.cs b/src/Dcli/DialogOutcome.cs index 54ad9b7..7f47136 100644 --- a/src/Dcli/DialogOutcome.cs +++ b/src/Dcli/DialogOutcome.cs @@ -5,10 +5,10 @@ namespace Dcli; /// /// /// -/// and are produced by the v1 list-based dialogs -/// (, , -/// ). exists for API/wizard-flow -/// compatibility but is not produced by any v1 dialog (no key maps to it in this release). +/// and are produced by all v1 list-based dialogs. +/// is produced by and +/// when the request has AllowBack = true and the user +/// presses Backspace before moving the selection cursor. /// /// public enum DialogOutcome @@ -17,7 +17,12 @@ public enum DialogOutcome Submitted, /// - /// Reserved for wizard-flow "go back" semantics. Not produced by any v1 dialog. + /// The user pressed Backspace to navigate back in a wizard flow. + /// Produced by and + /// when the request has AllowBack = true and the user presses Backspace before + /// moving the selection cursor (and before entering any filter text when type-to-filter + /// is active). is when this + /// outcome is returned. /// Back, diff --git a/src/Dcli/DialogRequests.cs b/src/Dcli/DialogRequests.cs index 4bf5c98..e3fdbbc 100644 --- a/src/Dcli/DialogRequests.cs +++ b/src/Dcli/DialogRequests.cs @@ -6,7 +6,13 @@ namespace Dcli; /// The list items to display. An empty list is allowed; Submit on an /// empty list returns with value -1. /// Optional leading title row rendered above the list. -public sealed record SelectRequest(IReadOnlyList Items, Line? Title = null) +/// +/// When , pressing Backspace before moving the selection cursor (and +/// before entering any filter text) closes the dialog with . +/// Intended for wizard flows where the user can step backwards. Defaults to +/// ; existing callers are unaffected. +/// +public sealed record SelectRequest(IReadOnlyList Items, Line? Title = null, bool AllowBack = false) { /// /// Constructs a from plain-text item strings. @@ -14,8 +20,12 @@ public sealed record SelectRequest(IReadOnlyList Items, Line? Title = null /// /// The plain-text items to display. /// Optional leading title row. - public SelectRequest(IReadOnlyList items, Line? title = null) - : this(ConvertItems(items), title) { } + /// + /// When , Backspace at position zero (before any movement) closes + /// the dialog with . Defaults to . + /// + public SelectRequest(IReadOnlyList items, Line? title = null, bool allowBack = false) + : this(ConvertItems(items), title, allowBack) { } /// /// Constructs a from a params array of plain-text item strings. @@ -71,7 +81,13 @@ private static List ConvertItems(IReadOnlyList items) /// /// The choice options to display. /// Optional leading prompt row rendered above the options. -public sealed record ChoiceRequest(IReadOnlyList Options, Line? Prompt = null) +/// +/// When , pressing Backspace before moving the selection cursor (and +/// before entering any filter text) closes the dialog with . +/// Intended for wizard flows where the user can step backwards. Defaults to +/// ; existing callers are unaffected. +/// +public sealed record ChoiceRequest(IReadOnlyList Options, Line? Prompt = null, bool AllowBack = false) { /// /// Constructs a from plain-text option strings. @@ -79,8 +95,12 @@ public sealed record ChoiceRequest(IReadOnlyList Options, Line? Prompt = n /// /// The plain-text options to display. /// Optional leading prompt row. - public ChoiceRequest(IReadOnlyList options, Line? prompt = null) - : this(ConvertOptions(options), prompt) { } + /// + /// When , Backspace at position zero (before any movement) closes + /// the dialog with . Defaults to . + /// + public ChoiceRequest(IReadOnlyList options, Line? prompt = null, bool allowBack = false) + : this(ConvertOptions(options), prompt, allowBack) { } /// /// Constructs a from a params array of plain-text option strings. diff --git a/src/Dcli/Internal/FixedRegion/Dialog.cs b/src/Dcli/Internal/FixedRegion/Dialog.cs index 2d13477..7625b10 100644 --- a/src/Dcli/Internal/FixedRegion/Dialog.cs +++ b/src/Dcli/Internal/FixedRegion/Dialog.cs @@ -52,11 +52,17 @@ internal sealed class Dialog : IModalOverlay /// of the budget is reserved for it, and List.MaxRows is set to /// at most MaxRows - 1 so the total output never exceeds the budget. /// - internal Dialog(bool multiSelect = false, bool modal = true, bool typeToFilter = false, Line? title = null) + /// + /// When , Backspace closes the dialog with + /// provided the user has not yet moved the selection + /// cursor and the filter text is empty. Defaults to . + /// + internal Dialog(bool multiSelect = false, bool modal = true, bool typeToFilter = false, Line? title = null, bool allowBack = false) { Modal = modal; TypeToFilter = typeToFilter; Title = title; + _allowBack = allowBack; List = new ScrollableList(multiSelect); } @@ -100,7 +106,11 @@ public int MaxRows /// /// Enter → = Submit; consumed. /// Escape → = Cancel; consumed. - /// ↑ / ↓ → navigate the list; consumed. + /// Backspace when constructed with allowBack: true, the user + /// has not yet moved the selection cursor, and is empty → + /// = Back; consumed. (Does not fire when the cursor has moved or + /// filter text is present, so it cannot mask the type-to-filter Backspace-trim behaviour.) + /// ↑ / ↓ → navigate the list (sets the internal moved flag); consumed. /// Space (U+0020) when → toggle current; consumed. /// Printable rune (≥ U+0020, ≠ U+007F) when → append to ; consumed. Backspace → trim ; consumed. /// Any remaining key when → consumed (modal catch-all). @@ -123,19 +133,31 @@ public bool HandleKey(KeyEvent key) return true; } - // 3. Arrow navigation + // 3. Backspace-Back: fires only when AllowBack is true, the user has not yet moved the + // selection cursor, and no filter text has been accumulated. The _filterText guard + // ensures this branch cannot mask the type-to-filter Backspace-trim path (rule 5). + if (key.Code.Kind == KeyCode.KeyCodeKind.Named && key.Code.NamedValue == NamedKey.Backspace && + _allowBack && !_hasMoved && _filterText.Length == 0) + { + CloseRequest = OverlayCloseKind.Back; + return true; + } + + // 4. Arrow navigation (sets _hasMoved so the Back branch above is no longer eligible) if (key.Code.Kind == KeyCode.KeyCodeKind.Named && key.Code.NamedValue == NamedKey.Up) { + _hasMoved = true; List.MoveUp(); return true; } if (key.Code.Kind == KeyCode.KeyCodeKind.Named && key.Code.NamedValue == NamedKey.Down) { + _hasMoved = true; List.MoveDown(); return true; } - // 4. Space → toggle when multi-select (takes precedence over type-to-filter for space) + // 5. Space → toggle when multi-select (takes precedence over type-to-filter for space) if (key.Code.Kind == KeyCode.KeyCodeKind.UnicodeScalar && key.Code.RuneValue.Value == ' ' && List.MultiSelect) @@ -144,7 +166,7 @@ public bool HandleKey(KeyEvent key) return true; } - // 5. Type-to-filter: printable runes and Backspace + // 6. Type-to-filter: printable runes and Backspace if (TypeToFilter) { if (key.Code.Kind == KeyCode.KeyCodeKind.Named && key.Code.NamedValue == NamedKey.Backspace) @@ -165,11 +187,11 @@ public bool HandleKey(KeyEvent key) } } - // 6. Modal catch-all: consume everything so the input editor gets nothing + // 7. Modal catch-all: consume everything so the input editor gets nothing if (Modal) return true; - // 7. Non-modal fall-through + // 8. Non-modal fall-through return false; } @@ -237,6 +259,10 @@ public IReadOnlyList Render(int width) private int _maxRows = 10; // default matches ScrollableList default private readonly StringBuilder _filterText = new(); + private readonly bool _allowBack; + + // Set to true on the first ↑/↓ press; once moved, Backspace-Back is no longer eligible. + private bool _hasMoved; /// /// Removes the last grapheme cluster (text element) from . diff --git a/src/Dcli/Internal/FixedRegion/IOverlay.cs b/src/Dcli/Internal/FixedRegion/IOverlay.cs index 768eba3..be688a3 100644 --- a/src/Dcli/Internal/FixedRegion/IOverlay.cs +++ b/src/Dcli/Internal/FixedRegion/IOverlay.cs @@ -25,6 +25,12 @@ internal enum OverlayCloseKind /// The user confirmed (Enter). Submit, + /// + /// The user navigated back (Backspace at position zero before any movement, + /// when was constructed with allowBack: true). + /// + Back, + /// The user cancelled (Escape). Cancel, } diff --git a/src/Dcli/Terminal.cs b/src/Dcli/Terminal.cs index 9deb76e..7320ba3 100644 --- a/src/Dcli/Terminal.cs +++ b/src/Dcli/Terminal.cs @@ -139,7 +139,7 @@ public Task> SelectAsync( CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(req); - Dialog dialog = new(multiSelect: false, modal: true, typeToFilter: false, title: req.Title); + Dialog dialog = new(multiSelect: false, modal: true, typeToFilter: false, title: req.Title, allowBack: req.AllowBack); dialog.List.SetItems(req.Items); return OpenModalAsync( dialog, @@ -192,7 +192,7 @@ public Task> ChoiceAsync( CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(req); - Dialog dialog = new(multiSelect: false, modal: true, typeToFilter: false, title: req.Prompt); + Dialog dialog = new(multiSelect: false, modal: true, typeToFilter: false, title: req.Prompt, allowBack: req.AllowBack); dialog.List.SetItems(req.Options); return OpenModalAsync( dialog, @@ -237,7 +237,9 @@ public Task> InputAsync( /// /// Parameterless factory called on the loop thread when the overlay is submitted. /// The closure captures the overlay and reads its state (e.g. text, selection index). - /// Only called on ; Cancel produces a default result. + /// Only called on ; + /// and produce default-value results with the + /// corresponding . /// /// External cancellation token. private Task> OpenModalAsync( @@ -259,9 +261,12 @@ private Task> OpenModalAsync( Action completion = () => { // Running on the loop thread. Read overlay state and complete the TCS. - DialogResult result = overlay.CloseRequest == OverlayCloseKind.Submit - ? buildResult() - : new DialogResult(DialogOutcome.Cancelled, default!); + DialogResult result = overlay.CloseRequest switch + { + OverlayCloseKind.Submit => buildResult(), + OverlayCloseKind.Back => new DialogResult(DialogOutcome.Back, default!), + _ => new DialogResult(DialogOutcome.Cancelled, default!), + }; tcs.TrySetResult(result); // Dispose the CT registration to prevent a stale cancel command from posting later. registrationHolder[0].Dispose(); diff --git a/tests/Dcli.Tests/ChoiceDialogTests.cs b/tests/Dcli.Tests/ChoiceDialogTests.cs new file mode 100644 index 0000000..871697e --- /dev/null +++ b/tests/Dcli.Tests/ChoiceDialogTests.cs @@ -0,0 +1,242 @@ +using Dcli.Internal.FixedRegion; +using Dcli.Internal.RenderLoop; +using Xunit; + +namespace Dcli.Tests; + +/// +/// Tests for §3 AllowBack on . +/// Mirrors the Select-side AllowBack tests in . +/// +public sealed class ChoiceDialogTests +{ + // ── Test infrastructure ─────────────────────────────────────────────────── + + private sealed class ConstantSizeSource(int cols, int rows) : ITerminalSizeSource + { + public (int Columns, int Rows) GetSize() => (cols, rows); + } + + private sealed class CapturingOutputSink : IOutputSink + { + public void Paint(RenderModel model) { } + public void EmitRestoreSequence() { } + } + + private sealed class VirtualClock : IClock + { + private readonly object _lock = new(); + private TimeSpan _now; + private readonly List<(TimeSpan Deadline, TaskCompletionSource Tcs, CancellationTokenSource LinkedCts)> _waiters = []; + + internal VirtualClock(TimeSpan? initial = null) => _now = initial ?? TimeSpan.Zero; + + public TimeSpan Now { get { lock (_lock) { return _now; } } } + + internal void Advance(TimeSpan by) + { + List<(TaskCompletionSource Tcs, CancellationTokenSource LinkedCts)> toComplete = []; + lock (_lock) + { + _now += by; + for (int i = _waiters.Count - 1; i >= 0; i--) + { + if (_waiters[i].Deadline <= _now) + { + toComplete.Add((_waiters[i].Tcs, _waiters[i].LinkedCts)); + _waiters.RemoveAt(i); + } + } + } + foreach ((TaskCompletionSource tcs, CancellationTokenSource linkedCts) in toComplete) + { + tcs.TrySetResult(); + linkedCts.Dispose(); + } + } + + public Task WaitUntilAsync(TimeSpan deadline, CancellationToken cancellationToken) + { + lock (_lock) + { + if (deadline <= _now) + return Task.CompletedTask; + } + CancellationTokenSource linkedCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + linkedCts.Token.Register( + static state => ((TaskCompletionSource)state!).TrySetCanceled(), + tcs, + useSynchronizationContext: false); + lock (_lock) + { + if (deadline <= _now) { linkedCts.Dispose(); return Task.CompletedTask; } + _waiters.Add((deadline, tcs, linkedCts)); + } + return tcs.Task; + } + } + + private static (LoopEngine Engine, VirtualClock Clock) CreateEngine(int cols = 80, int rows = 24) + { + VirtualClock clock = new(TimeSpan.Zero); + CapturingOutputSink sink = new(); + LoopEngine engine = new( + new ConstantSizeSource(cols, rows), + clock, + sink, + minFrameInterval: TimeSpan.Zero); + return (engine, clock); + } + + private static Line PlainLine(string text) => new([new Segment(text)]); + + private static List Options(params string[] texts) => + texts.Select(PlainLine).ToList(); + + private static Task SettleAsync(LoopEngine engine, VirtualClock clock) + { + clock.Advance(TimeSpan.FromMilliseconds(10)); + return engine.SettleAsync().WaitAsync(TimeSpan.FromSeconds(5)); + } + + // ── Helpers ─────────────────────────────────────────────────────────────── + + private static Task> PostChoiceDialog( + LoopEngine engine, + List options) + { + Dialog dialog = new(multiSelect: false, modal: true); + dialog.List.SetItems(options); + + TaskCompletionSource> tcs = + new(TaskCreationOptions.RunContinuationsAsynchronously); + + Action completion = () => + { + DialogResult result = dialog.CloseRequest switch + { + OverlayCloseKind.Submit => new DialogResult(DialogOutcome.Submitted, dialog.List.SelectedIndex), + OverlayCloseKind.Back => new DialogResult(DialogOutcome.Back, -1), + _ => new DialogResult(DialogOutcome.Cancelled, -1), + }; + tcs.TrySetResult(result); + }; + + Action reject = () => + tcs.TrySetException(new InvalidOperationException("A dialog is already active.")); + + engine.Post(new OpenDialogCommand(dialog, completion, reject)); + return tcs.Task; + } + + private static Task> PostChoiceDialogAllowBack( + LoopEngine engine, + List options) + { + Dialog dialog = new(multiSelect: false, modal: true, allowBack: true); + dialog.List.SetItems(options); + + TaskCompletionSource> tcs = + new(TaskCreationOptions.RunContinuationsAsynchronously); + + Action completion = () => + { + DialogResult result = dialog.CloseRequest switch + { + OverlayCloseKind.Submit => new DialogResult(DialogOutcome.Submitted, dialog.List.SelectedIndex), + OverlayCloseKind.Back => new DialogResult(DialogOutcome.Back, -1), + _ => new DialogResult(DialogOutcome.Cancelled, -1), + }; + tcs.TrySetResult(result); + }; + + Action reject = () => + tcs.TrySetException(new InvalidOperationException("A dialog is already active.")); + + engine.Post(new OpenDialogCommand(dialog, completion, reject)); + return tcs.Task; + } + + // ── AllowBack tests ─────────────────────────────────────────────────────── + + /// + /// AllowBack=true + Backspace at empty (no movement) → DialogOutcome.Back. + /// + [Fact] + public async Task ChoiceAllowBackBackspaceAtEmptyReturnsBack() + { + (LoopEngine engine, VirtualClock clock) = CreateEngine(); + try + { + Task> task = PostChoiceDialogAllowBack(engine, Options("Yes", "No", "Maybe")); + + engine.InputWriter.TryWrite(new KeyEvent(KeyCode.Named(NamedKey.Backspace), Modifiers.None)); + + await SettleAsync(engine, clock); + DialogResult result = await task; + + Assert.Equal(DialogOutcome.Back, result.Outcome); + } + finally { engine.Dispose(); } + } + + /// + /// AllowBack=true + ↓ then Backspace → Backspace is a no-op (cursor has moved). + /// The dialog stays open; pressing Escape afterwards produces Cancelled. + /// + [Fact] + public async Task ChoiceAllowBackBackspaceAfterMovementIsNoOp() + { + (LoopEngine engine, VirtualClock clock) = CreateEngine(); + try + { + Task> task = PostChoiceDialogAllowBack(engine, Options("Yes", "No", "Maybe")); + + // ↓ marks _hasMoved = true; subsequent Backspace must not close the dialog. + engine.InputWriter.TryWrite(new KeyEvent(KeyCode.Named(NamedKey.Down), Modifiers.None)); + engine.InputWriter.TryWrite(new KeyEvent(KeyCode.Named(NamedKey.Backspace), Modifiers.None)); + + await SettleAsync(engine, clock); + + // Task must not be complete — the dialog is still open. + Assert.False(task.IsCompleted, "Choice dialog should still be open after Backspace post-movement"); + + // Dismiss with Escape to clean up. + engine.InputWriter.TryWrite(new KeyEvent(KeyCode.Named(NamedKey.Escape), Modifiers.None)); + await SettleAsync(engine, clock); + + DialogResult result = await task; + Assert.Equal(DialogOutcome.Cancelled, result.Outcome); + } + finally { engine.Dispose(); } + } + + /// + /// AllowBack=false (default) + Backspace → no-op; dialog stays open; Enter produces Submitted. + /// + [Fact] + public async Task ChoiceAllowBackFalseDefaultBackspaceIsNoOp() + { + (LoopEngine engine, VirtualClock clock) = CreateEngine(); + try + { + Task> task = PostChoiceDialog(engine, Options("Alpha", "Beta", "Gamma")); + + engine.InputWriter.TryWrite(new KeyEvent(KeyCode.Named(NamedKey.Backspace), Modifiers.None)); + + await SettleAsync(engine, clock); + + // Task must still be pending — Backspace should have been swallowed by the modal catch-all. + Assert.False(task.IsCompleted, "Choice dialog should still be open after Backspace when AllowBack=false"); + + // Submit to close. + engine.InputWriter.TryWrite(new KeyEvent(KeyCode.Named(NamedKey.Enter), Modifiers.None)); + await SettleAsync(engine, clock); + + DialogResult result = await task; + Assert.Equal(DialogOutcome.Submitted, result.Outcome); + } + finally { engine.Dispose(); } + } +} diff --git a/tests/Dcli.Tests/DialogSelectionTests.cs b/tests/Dcli.Tests/DialogSelectionTests.cs index 7c1a65f..4cc3190 100644 --- a/tests/Dcli.Tests/DialogSelectionTests.cs +++ b/tests/Dcli.Tests/DialogSelectionTests.cs @@ -593,4 +593,126 @@ public void TitledDialogMaxRows2RenderReturnsTitlePlusOneListRow() Assert.Equal(2, rows.Count); Assert.Same(title, rows[0]); } + + // ── AllowBack — section 3 ───────────────────────────────────────────────── + + // Helper: post a select dialog with AllowBack=true; completion maps Back correctly. + private static Task> PostSelectDialogAllowBack( + LoopEngine engine, + List items) + { + Dialog dialog = new(multiSelect: false, modal: true, allowBack: true); + dialog.List.SetItems(items); + + TaskCompletionSource> tcs = + new(TaskCreationOptions.RunContinuationsAsynchronously); + + Action completion = () => + { + DialogResult result = dialog.CloseRequest switch + { + OverlayCloseKind.Submit => new DialogResult(DialogOutcome.Submitted, dialog.List.SelectedIndex), + OverlayCloseKind.Back => new DialogResult(DialogOutcome.Back, -1), + _ => new DialogResult(DialogOutcome.Cancelled, -1), + }; + tcs.TrySetResult(result); + }; + + Action reject = () => + tcs.TrySetException(new InvalidOperationException("A dialog is already active.")); + + engine.Post(new OpenDialogCommand(dialog, completion, reject)); + return tcs.Task; + } + + /// + /// AllowBack=true + Backspace at empty (no movement) → DialogOutcome.Back. + /// + [Fact] + public async Task SelectAllowBackBackspaceAtEmptyReturnsBack() + { + (LoopEngine engine, VirtualClock clock, _) = CreateEngine(); + try + { + Task> task = PostSelectDialogAllowBack(engine, Items("A", "B", "C")); + + engine.InputWriter.TryWrite(new KeyEvent(KeyCode.Named(NamedKey.Backspace), Modifiers.None)); + + await SettleAsync(engine, clock); + DialogResult result = await task; + + Assert.Equal(DialogOutcome.Back, result.Outcome); + } + finally { engine.Dispose(); } + } + + /// + /// AllowBack=true + ↓ then Backspace → Backspace is a no-op (cursor has moved). + /// The dialog stays open; pressing Escape afterwards produces Cancelled. + /// + [Fact] + public async Task SelectAllowBackBackspaceAfterMovementIsNoOp() + { + (LoopEngine engine, VirtualClock clock, _) = CreateEngine(); + try + { + Task> task = PostSelectDialogAllowBack(engine, Items("A", "B", "C")); + + // ↓ marks _hasMoved = true; subsequent Backspace must not close the dialog. + engine.InputWriter.TryWrite(new KeyEvent(KeyCode.Named(NamedKey.Down), Modifiers.None)); + engine.InputWriter.TryWrite(new KeyEvent(KeyCode.Named(NamedKey.Backspace), Modifiers.None)); + + // Give the loop time to process both keys. + await SettleAsync(engine, clock); + + // Task must not be complete — the dialog is still open. + Assert.False(task.IsCompleted, "Dialog should still be open after Backspace post-movement"); + + // Dismiss with Escape to clean up. + engine.InputWriter.TryWrite(new KeyEvent(KeyCode.Named(NamedKey.Escape), Modifiers.None)); + await SettleAsync(engine, clock); + + DialogResult result = await task; + Assert.Equal(DialogOutcome.Cancelled, result.Outcome); + } + finally { engine.Dispose(); } + } + + /// + /// AllowBack=false (default) + Backspace → no-op; dialog stays open; Enter produces Submitted. + /// + [Fact] + public async Task SelectAllowBackFalseDefaultBackspaceIsNoOp() + { + (LoopEngine engine, VirtualClock clock, _) = CreateEngine(); + try + { + // Default AllowBack=false — use the standard PostSelectDialog helper. + Task> task = PostSelectDialog(engine, Items("X", "Y", "Z")); + + engine.InputWriter.TryWrite(new KeyEvent(KeyCode.Named(NamedKey.Backspace), Modifiers.None)); + + await SettleAsync(engine, clock); + + // Task must still be pending — Backspace should have been swallowed by the modal catch-all. + Assert.False(task.IsCompleted, "Dialog should still be open after Backspace when AllowBack=false"); + + // Submit to close. + engine.InputWriter.TryWrite(new KeyEvent(KeyCode.Named(NamedKey.Enter), Modifiers.None)); + await SettleAsync(engine, clock); + + DialogResult result = await task; + Assert.Equal(DialogOutcome.Submitted, result.Outcome); + } + finally { engine.Dispose(); } + } + + // ── 3.8 — MultiSelectRequest deliberately omits AllowBack ──────────────── + + [Fact] + public void MultiSelectRequestDoesNotHaveAllowBackProperty() + { + // AllowBack is intentionally absent from MultiSelectRequest (design decision §3.8). + Assert.Null(typeof(MultiSelectRequest).GetProperty("AllowBack")); + } } From 9cbd883c557025cc56765c394c9de2810d0db3f9 Mon Sep 17 00:00:00 2001 From: Emmz Rendle Date: Thu, 28 May 2026 14:30:35 +0100 Subject: [PATCH 05/10] feat(api-ergonomics-pass-1): secret-default masking in InputDialog (section 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spec/code-reality finding: the secret-default leak Decision 5 defends against does not actually exist in current code. `InputDialog.Render` runs `_isSecret ? MaskRows(rows) : rows` unconditionally on every paint since the file was introduced (commit 394c9ba, §12 of core-rendering-architecture). The proposal/design's premise that "bullets only kick in once the user edits" is inaccurate; the seeded default has always been masked on first paint via the existing width-preserving MaskRows path. Decision 5's "reuse the existing edit-detection flag (for InputChanged emission)" guidance is also moot — no such flag exists and no InputChanged event is emitted. The spec scenarios remain correct as behavioural requirements; §4 work therefore degenerates to: - 4.1+4.2 Introduce a single sticky `_userEdited` flag on InputDialog (false initially, flipped to true on the three buffer-mutating operations in HandleKey: printable-rune insert, Backspace, Delete). Caret-only moves and Enter/Escape do NOT flip it. The constructor's `_buffer.SetText(default)` does NOT flip it. The flag is sticky: once true, never reset (even if the buffer is shrunk back to its original text). Exposed via `internal bool UserEdited => _userEdited` for test inspection. - 4.3 No change to `Text` — it always returns the real buffer contents. - 4.4 Five InputDialogTests pin the spec scenarios: masked-on-first-paint (a regression guard, not a bug repro), masked-before-first-edit, Text-returns-real-default, one-edit-then-revert-sticky semantics, and the existing IsSecret round-trip. - 4.5 Non-secret default regression guard test. Future trap recorded in DEVLOG: paste / history-recall (mentioned in `specs/fixed-region/spec.md` "Owned input editor" as edit triggers) are not implemented today, so they don't currently need to flip `_userEdited`. If either gets added later, they must. Gates: build 0 warnings, 714 tests green (+5 over §3), format clean, openspec validate --strict clean. Co-Authored-By: Claude Opus 4.7 --- .../changes/api-ergonomics-pass-1/DEVLOG.md | 7 +- .../changes/api-ergonomics-pass-1/tasks.md | 10 +-- src/Dcli/Internal/FixedRegion/InputDialog.cs | 14 +++ tests/Dcli.Tests/InputDialogTests.cs | 89 +++++++++++++++++++ 4 files changed, 113 insertions(+), 7 deletions(-) diff --git a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md index 62b48e5..632682f 100644 --- a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md +++ b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md @@ -25,7 +25,8 @@ One row per `## N.` section in `tasks.md`. Add a row when the section commits. |---|---------|--------|-------------|-------| | 1 | `Line.FromText` factory | `3dd518c` | 693 (688 + 5) | XML doc also states "no implicit `string → Line` conversion is defined" — covers the doc half of task 2.9 ahead of §2. | | 2 | String-accepting consumer overloads | `55cd4de` | 702 (693 + 9) | Tasks.md target paths (`IScrollback.cs`, `*Request.cs` per record) didn't match real layout; actually edited `ITerminal.cs` (interface) and `DialogRequests.cs` (all four records together). `new InputRequest(null)` is ambiguous between the `Line?` primary and the new `string?` secondary; resolved at call sites via `new InputRequest()` (primary all-defaulted) or explicit `(string?)null` / `Line.FromText(...)`. Tests cover both. | -| 3 | `AllowBack` flag on `SelectRequest`/`ChoiceRequest` | _pending_ | 709 (702 + 7) | No separate `ChoiceDialog` — `Choice` reuses the unified `Dialog` (`Terminal.cs:195`); created new `ChoiceDialogTests.cs` rather than splitting the class. `OverlayCloseKind` enum ordering becomes `Submit, Back, Cancel` (internal enum so no ABI consumer impact). The `Dialog.HandleKey` Backspace-Back branch is placed BEFORE the existing `TypeToFilter` Backspace-trim branch, with a `_filterText.Length == 0` guard defending against any future widening of `TypeToFilter` to Select/Choice. | +| 3 | `AllowBack` flag on `SelectRequest`/`ChoiceRequest` | `fb90d34` | 709 (702 + 7) | No separate `ChoiceDialog` — `Choice` reuses the unified `Dialog` (`Terminal.cs:195`); created new `ChoiceDialogTests.cs` rather than splitting the class. `OverlayCloseKind` enum ordering becomes `Submit, Back, Cancel` (internal enum so no ABI consumer impact). The `Dialog.HandleKey` Backspace-Back branch is placed BEFORE the existing `TypeToFilter` Backspace-trim branch, with a `_filterText.Length == 0` guard defending against any future widening of `TypeToFilter` to Select/Choice. | +| 4 | Secret-default masking in `InputDialog` | _pending_ | 714 (709 + 5) | **Bug didn't actually exist** — `InputDialog.Render` has unconditionally masked the buffer rows whenever `_isSecret=true` since the file was introduced in `core-rendering-architecture` (`394c9ba`). Proposal/design Decision 5's premise that "bullets only kick in once the user edits" was inaccurate. The spec scenarios still hold and are now pinned by regression tests. Introduced `_userEdited` (sticky) as a single edit-flag — it doesn't change rendering today but aligns the code with the spec's vocabulary and pins sticky semantics for any future refactor that gates masking on edit-state. | ## Decisions & deviations @@ -37,6 +38,8 @@ Narrative log of anything that wasn't a straight read-off-the-spec-and-implement **§2 — `tasks.md` paths deviate from real layout.** Tasks 2.1, 2.3–2.6 named per-record files (`src/Dcli/IScrollback.cs`, `src/Dcli/InputRequest.cs`, `src/Dcli/SelectRequest.cs`, etc.) that don't exist. The actual layout consolidates: `IScrollback` lives in `src/Dcli/ITerminal.cs` next to the façade interface, and all four `*Request` records live in `src/Dcli/DialogRequests.cs`. Implemented in the real files; did not split them out (that would have been scope creep — Decision 6 is surface-only). No spec amendment needed; the file paths in `tasks.md` were ergonomic shorthand for "the file that contains this type". +**§4 — Secret-default leak the spec defends against doesn't actually exist in current code.** Proposal claim ("Today bullets only kick in once the user edits, so the seeded default leaks clear-text on first paint") and design.md Decision 5 ("The dialog already tracks whether the user has edited the buffer (it has to, for `InputChanged` emission). Reuse that flag.") are both inaccurate about the as-shipped code. Verified by trace + git history: `InputDialog.Render` runs `_isSecret ? MaskRows(rows) : rows` unconditionally on every paint since the file was introduced (`394c9ba`, §12 of core-rendering-architecture). There is no `_userEdited` flag in the original code and no `InputChanged` event is emitted at all. The seeded default flows through `_buffer.SetText` in the ctor → `_buffer.Render` → `MaskRows` → width-preserving bullets, on the very first paint. Conclusion: spec scenarios still hold and are now pinned by regression tests, but the rationale paragraphs in `proposal.md` / `design.md` are wrong about the bug premise. Resolution: §4 work degenerates to (a) regression tests pinning the three new spec scenarios and (b) introducing a `_userEdited` flag (sticky, flipped on Insert/Backspace/Delete) so the spec's edit-state vocabulary has a code-level seam. Decision 5's "reuse the existing flag" guidance is therefore moot; introduced ONE flag (per task 4.2's "do not introduce a second flag" intent). Future trap to remember: if a paste path or history-recall keybinding is added later, they must flip `_userEdited` too — the spec lists those as edit triggers (`specs/fixed-region/spec.md` "Owned input editor"). Today neither is wired, so this is a placeholder note, not a gap. + **§2 — `new InputRequest(null)` is ambiguous (expected).** With both `InputRequest(Line? Prompt = null, …)` and `InputRequest(string? prompt, …)` present, a bare `null` literal as the first argument cannot resolve. This is documented behaviour for C# overload resolution and matches the design's "explicit only" stance (Decision 1). Mitigation: the natural empty-input call `new InputRequest()` resolves unambiguously to the primary (all defaults); callers who want a null prompt explicitly use `new InputRequest((string?)null)` or `new InputRequest((Line?)null)`. Both forms covered by `FacadeTests` (`InputRequestDefaultCtorRemainsUnambiguous`, `InputRequestNullStringPromptProducesNullPrompt`). ## Human-in-the-loop verifications @@ -62,4 +65,4 @@ Surface gaps for future changes. Link to memory files where the constraint is en ## Resume point -> **Currently at §4.1 — Secret-default masking in `InputDialog`.** §3 shipped (`AllowBack` flag + Backspace-at-empty wiring + 7 tests; 709 tests green, 0 warnings, format/validate clean). Next worker brief: implement §4 end-to-end — mask the seeded `Default` when `IsSecret && !_userEdited` (4.1), reuse the existing edit-detection flag (4.2), confirm `Submit` returns the real string (4.3), tests in `InputDialogTests` (4.4–4.5). +> **Currently at §5.1 — Demo updates.** §4 shipped (regression tests + `_userEdited` flag; bug-didn't-exist finding logged; 714 tests green, 0 warnings, format/validate clean). Next worker brief: implement §5 end-to-end — port `samples/Dcli.Demo.DmonWizard/Engine/WizardRenderer.cs`'s ~12 `new LineBuilder().Text(s).Build()` sites to `Line.FromText(s)` (5.1), switch label-only `Line`s in `samples/Dcli.Demo/` where possible (5.2), and add an `AllowBack=true` wizard step so the new affordance has a live demonstration (5.3). diff --git a/openspec/changes/api-ergonomics-pass-1/tasks.md b/openspec/changes/api-ergonomics-pass-1/tasks.md index d077133..a1e37d6 100644 --- a/openspec/changes/api-ergonomics-pass-1/tasks.md +++ b/openspec/changes/api-ergonomics-pass-1/tasks.md @@ -29,11 +29,11 @@ ## 4. Secret-default masking in `InputDialog` -- [ ] 4.1 `src/Dcli/Internal/FixedRegion/InputDialog.cs` — when `IsSecret && !_userEdited && Default is non-empty`, render the buffer as `'•'` repeated by `DisplayWidth.Measure(Default)` instead of the raw default -- [ ] 4.2 Ensure the existing edit-detection (used for `InputChanged` emission) is the source of truth for `_userEdited`; do not introduce a second flag -- [ ] 4.3 `Submit` returns the real string (assert this — should be unchanged) -- [ ] 4.4 Tests in `tests/Dcli.Tests/InputDialogTests.cs`: secret + default + no edits → masked render; secret + default + Submit → real string; secret + default + one edit then revert → still masked? (decide via the existing `_userEdited` semantics; document in the test) -- [ ] 4.5 Non-secret + default test: paint shows clear-text default (regression guard) +- [x] 4.1 `src/Dcli/Internal/FixedRegion/InputDialog.cs` — when `IsSecret && !_userEdited && Default is non-empty`, render the buffer as `'•'` repeated by `DisplayWidth.Measure(Default)` instead of the raw default +- [x] 4.2 Ensure the existing edit-detection (used for `InputChanged` emission) is the source of truth for `_userEdited`; do not introduce a second flag +- [x] 4.3 `Submit` returns the real string (assert this — should be unchanged) +- [x] 4.4 Tests in `tests/Dcli.Tests/InputDialogTests.cs`: secret + default + no edits → masked render; secret + default + Submit → real string; secret + default + one edit then revert → still masked? (decide via the existing `_userEdited` semantics; document in the test) +- [x] 4.5 Non-secret + default test: paint shows clear-text default (regression guard) ## 5. Demo updates diff --git a/src/Dcli/Internal/FixedRegion/InputDialog.cs b/src/Dcli/Internal/FixedRegion/InputDialog.cs index edb77b7..071b0f1 100644 --- a/src/Dcli/Internal/FixedRegion/InputDialog.cs +++ b/src/Dcli/Internal/FixedRegion/InputDialog.cs @@ -23,6 +23,11 @@ internal sealed class InputDialog : IModalOverlay { private readonly TextBuffer _buffer; private readonly bool _isSecret; + // True once the user makes any buffer-mutating keystroke (insert, Backspace, Delete). + // Sticky: never reset to false after being set. Used so that masking semantics are + // consistent: _userEdited=false means the buffer still holds the seeded Default exactly + // as constructed. + private bool _userEdited; private int _maxRows = 10; // Cached from the last Render call (or seeded by SeedWidth on the loop thread at open time) // so HandleKey has a correct width for Home/End/Up/Down before the first paint. @@ -115,6 +120,7 @@ public bool HandleKey(KeyEvent key) Rune r = key.Code.RuneValue; if (r.Value >= 0x20 && r.Value != 0x7F) { + _userEdited = true; _buffer.Insert(r); return true; } @@ -126,10 +132,12 @@ public bool HandleKey(KeyEvent key) switch (key.Code.NamedValue) { case NamedKey.Backspace: + _userEdited = true; _buffer.Backspace(); return true; case NamedKey.Delete: + _userEdited = true; _buffer.Delete(); return true; @@ -226,6 +234,12 @@ public IReadOnlyList Render(int width) /// The real (unmasked) text currently in the buffer. internal string Text => _buffer.Text; + /// + /// once the user has made any buffer-mutating keystroke + /// (insert, Backspace, Delete). Sticky — never reset after being set. + /// + internal bool UserEdited => _userEdited; + /// /// Seeds the cached width used by width-dependent operations /// (Home, End, Up, Down) before the first call. diff --git a/tests/Dcli.Tests/InputDialogTests.cs b/tests/Dcli.Tests/InputDialogTests.cs index 552a55f..85b2bc9 100644 --- a/tests/Dcli.Tests/InputDialogTests.cs +++ b/tests/Dcli.Tests/InputDialogTests.cs @@ -551,4 +551,93 @@ public void InputDialogIsDismissedAfterEscape() Assert.True(dialog.IsDismissed); Assert.Equal(OverlayCloseKind.Cancel, dialog.CloseRequest); } + + // ── §4 Secret-default masking ───────────────────────────────────────────── + + // Repro test: was the "seeded default leaks clear-text" bug real? + // Expected: PASS — the existing MaskRows path already masks unconditionally when + // _isSecret=true, so the seeded default never leaks on first paint. The spec/design + // comment about this being a bug is inaccurate for the current codebase. + [Fact] + public void SecretDefaultIsMaskedOnFirstPaintRegressionGuard() + { + InputDialog dlg = new(prompt: null, @default: "secret123", isSecret: true); + IReadOnlyList rows = dlg.Render(width: 40); + string allText = string.Concat(rows.SelectMany(l => l.Segments).Select(s => s.Text)); + Assert.DoesNotContain("secret123", allText, StringComparison.Ordinal); + Assert.Contains("•", allText, StringComparison.Ordinal); + } + + // Spec scenario: "Secret default is masked before first edit" + // IsSecret=true, Default="hunter2", no edits → render shows bullets, not "hunter2". + [Fact] + public void SecretDefaultMaskedBeforeFirstEdit() + { + InputDialog dlg = new(prompt: null, @default: "hunter2", isSecret: true); + IReadOnlyList rows = dlg.Render(width: 40); + string allText = string.Concat(rows.SelectMany(l => l.Segments).Select(s => s.Text)); + Assert.DoesNotContain("hunter2", allText, StringComparison.Ordinal); + Assert.Contains("•", allText, StringComparison.Ordinal); + // 7 ASCII chars → 7 bullets. + int bulletCount = allText.Count(c => c == '•'); + Assert.Equal(7, bulletCount); + } + + // Spec scenario: "Secret default reveals real text on submit" + // Text property returns the unmasked buffer content regardless of _isSecret. + [Fact] + public void SecretDefaultTextPropertyReturnsRealDefault() + { + InputDialog dlg = new(prompt: null, @default: "hunter2", isSecret: true); + Assert.Equal("hunter2", dlg.Text); + } + + // Spec scenario: "Secret + default + one edit (and possibly revert)" + // _userEdited is sticky: once set it stays true even if the buffer content reverts to + // equal the original default. Masking still applies (the _isSecret path runs regardless), + // but the bullet count reflects the CURRENT buffer width, not the original default's width. + [Fact] + public void SecretDefaultOneEditThenRevertUserEditedIsSticky() + { + InputDialog dlg = new(prompt: null, @default: "abc", isSecret: true); + + // Step 1: render before any edit — 3 bullets for "abc". + IReadOnlyList rows1 = dlg.Render(width: 40); + string text1 = string.Concat(rows1.SelectMany(l => l.Segments).Select(s => s.Text)); + Assert.DoesNotContain("abc", text1, StringComparison.Ordinal); + int bullets1 = text1.Count(c => c == '•'); + Assert.Equal(3, bullets1); + Assert.False(dlg.UserEdited); + + // Step 2: insert 'x' → buffer is "abcx", _userEdited=true. + dlg.HandleKey(new KeyEvent(KeyCode.FromRune(new Rune('x')), Modifiers.None)); + IReadOnlyList rows2 = dlg.Render(width: 40); + string text2 = string.Concat(rows2.SelectMany(l => l.Segments).Select(s => s.Text)); + Assert.DoesNotContain("abcx", text2, StringComparison.Ordinal); + int bullets2 = text2.Count(c => c == '•'); + Assert.Equal(4, bullets2); // "abcx" = 4 display columns + Assert.True(dlg.UserEdited); + + // Step 3: Backspace twice → buffer becomes "ab", _userEdited stays true (sticky). + dlg.HandleKey(new KeyEvent(KeyCode.Named(NamedKey.Backspace), Modifiers.None)); + dlg.HandleKey(new KeyEvent(KeyCode.Named(NamedKey.Backspace), Modifiers.None)); + IReadOnlyList rows3 = dlg.Render(width: 40); + string text3 = string.Concat(rows3.SelectMany(l => l.Segments).Select(s => s.Text)); + Assert.DoesNotContain("ab", text3, StringComparison.Ordinal); + int bullets3 = text3.Count(c => c == '•'); + Assert.Equal(2, bullets3); // "ab" = 2 display columns + // _userEdited remains true even though buffer width is now less than the original default. + Assert.True(dlg.UserEdited); + } + + // Spec scenario: "Non-secret default renders as plain text" (regression guard, §4.5) + [Fact] + public void NonSecretDefaultRendersAsPlainText() + { + InputDialog dlg = new(prompt: null, @default: "hello", isSecret: false); + IReadOnlyList rows = dlg.Render(width: 40); + string allText = string.Concat(rows.SelectMany(l => l.Segments).Select(s => s.Text)); + Assert.Contains("hello", allText, StringComparison.Ordinal); + Assert.DoesNotContain("•", allText, StringComparison.Ordinal); + } } From 3bde5fd00fa52e8f0387f94cce992ac4d880d886 Mon Sep 17 00:00:00 2001 From: Emmz Rendle Date: Thu, 28 May 2026 14:36:26 +0100 Subject: [PATCH 06/10] feat(api-ergonomics-pass-1): demo ports + live AllowBack step (section 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 5.1 WizardRenderer.cs: 12 → 10 LineBuilder sites. Two label-only Select item builders in RenderChooseOneAsync/RenderChooseManyAsync replaced with Line.FromText. Remaining 10 are genuinely styled (Bold/Dim/Fg/multi-segment). - 5.2 Dcli.Demo/Program.cs: 43 → 37 LineBuilder sites. Six label-only replacements — two via the new Scrollback.Append(string) overload, four via Line.FromText in a MultiSelect items array. Remaining sites are styled compositions. - 5.3 Option A: AllowBack=true is now set on the ChooseOne Select dialog in WizardRenderer; DialogOutcome.Back maps to WizardStepOutcome.Back. WizardEngine.cs already routed WizardStepOutcome.Back to the previous step, so no engine changes were required — the affordance is live. Future ergonomics-pass-2 candidates recorded in DEVLOG (Line.Bold(s) / Line.Dim(s) / Line.Fg(s, color) shorthands would shrink another ~10+ sites in the same files; out of scope for pass-1). Gates: build 0 warnings, 714 tests green (unchanged — samples only), format clean, openspec validate --strict clean. Co-Authored-By: Claude Opus 4.7 --- openspec/changes/api-ergonomics-pass-1/DEVLOG.md | 6 ++++-- openspec/changes/api-ergonomics-pass-1/tasks.md | 6 +++--- .../Engine/WizardRenderer.cs | 14 +++++++++----- samples/Dcli.Demo/Program.cs | 16 ++++++---------- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md index 632682f..a200b6b 100644 --- a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md +++ b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md @@ -26,7 +26,8 @@ One row per `## N.` section in `tasks.md`. Add a row when the section commits. | 1 | `Line.FromText` factory | `3dd518c` | 693 (688 + 5) | XML doc also states "no implicit `string → Line` conversion is defined" — covers the doc half of task 2.9 ahead of §2. | | 2 | String-accepting consumer overloads | `55cd4de` | 702 (693 + 9) | Tasks.md target paths (`IScrollback.cs`, `*Request.cs` per record) didn't match real layout; actually edited `ITerminal.cs` (interface) and `DialogRequests.cs` (all four records together). `new InputRequest(null)` is ambiguous between the `Line?` primary and the new `string?` secondary; resolved at call sites via `new InputRequest()` (primary all-defaulted) or explicit `(string?)null` / `Line.FromText(...)`. Tests cover both. | | 3 | `AllowBack` flag on `SelectRequest`/`ChoiceRequest` | `fb90d34` | 709 (702 + 7) | No separate `ChoiceDialog` — `Choice` reuses the unified `Dialog` (`Terminal.cs:195`); created new `ChoiceDialogTests.cs` rather than splitting the class. `OverlayCloseKind` enum ordering becomes `Submit, Back, Cancel` (internal enum so no ABI consumer impact). The `Dialog.HandleKey` Backspace-Back branch is placed BEFORE the existing `TypeToFilter` Backspace-trim branch, with a `_filterText.Length == 0` guard defending against any future widening of `TypeToFilter` to Select/Choice. | -| 4 | Secret-default masking in `InputDialog` | _pending_ | 714 (709 + 5) | **Bug didn't actually exist** — `InputDialog.Render` has unconditionally masked the buffer rows whenever `_isSecret=true` since the file was introduced in `core-rendering-architecture` (`394c9ba`). Proposal/design Decision 5's premise that "bullets only kick in once the user edits" was inaccurate. The spec scenarios still hold and are now pinned by regression tests. Introduced `_userEdited` (sticky) as a single edit-flag — it doesn't change rendering today but aligns the code with the spec's vocabulary and pins sticky semantics for any future refactor that gates masking on edit-state. | +| 4 | Secret-default masking in `InputDialog` | `9cbd883` | 714 (709 + 5) | **Bug didn't actually exist** — `InputDialog.Render` has unconditionally masked the buffer rows whenever `_isSecret=true` since the file was introduced in `core-rendering-architecture` (`394c9ba`). Proposal/design Decision 5's premise that "bullets only kick in once the user edits" was inaccurate. The spec scenarios still hold and are now pinned by regression tests. Introduced `_userEdited` (sticky) as a single edit-flag — it doesn't change rendering today but aligns the code with the spec's vocabulary and pins sticky semantics for any future refactor that gates masking on edit-state. | +| 5 | Demo updates | _pending_ | 714 (unchanged — samples only) | `WizardRenderer.cs` 12 → 10 LineBuilders (2 label-only `Line.FromText` replacements); `Dcli.Demo/Program.cs` 43 → 37 (6 replacements — 2 via `Scrollback.Append(string)`, 4 via `Line.FromText` in MultiSelect items). 5.3 took Option A — `WizardEngine.cs` already routed `WizardStepOutcome.Back`, so activating `AllowBack: true` in `RenderChooseOneAsync` + mapping `DialogOutcome.Back → WizardStepOutcome.Back` was sufficient; no engine change. | ## Decisions & deviations @@ -56,6 +57,7 @@ Surface gaps for future changes. Link to memory files where the constraint is en - **`Input.Prompt` / `Input.ReadOnly` on the fixed-region input surface** — still deferred from §12 of the architecture change. Memory file: [[section14-api-ergonomics-findings]] entry 3-related; not blocked by this change. - **`Scrollback.AppendRule`, incremental `Collapsible.AppendLine`, `PasteEvent` editor routing** — separate future changes; this change does not touch them. - **VT-escape sanitisation of `Segment.Text`** — separate future change. Memory file: [[vt-escape-sanitization-gap]]. +- **`Line.Bold(text)` / `Line.Dim(text)` / `Line.Fg(text, color)` single-style shorthands** — surfaced by §5 reviewer audit. `WizardRenderer.cs` still has ~10 `new LineBuilder().Bold(s).Build()` / `.Dim(s)` / `.Fg(s, color)` sites; `Dcli.Demo/Program.cs` has more. Each is one styled segment on a label string. A future ergonomics-pass-2 change could add direct `Line.Bold(s)` etc. factories analogous to `Line.FromText(s)`. Not in scope for pass-1 (proposal limited to `FromText`); recorded here as a concrete candidate. ## Memory files (indexed by `~/.claude/projects/-Users-emmz-github-emmz-dcli/memory/MEMORY.md`) @@ -65,4 +67,4 @@ Surface gaps for future changes. Link to memory files where the constraint is en ## Resume point -> **Currently at §5.1 — Demo updates.** §4 shipped (regression tests + `_userEdited` flag; bug-didn't-exist finding logged; 714 tests green, 0 warnings, format/validate clean). Next worker brief: implement §5 end-to-end — port `samples/Dcli.Demo.DmonWizard/Engine/WizardRenderer.cs`'s ~12 `new LineBuilder().Text(s).Build()` sites to `Line.FromText(s)` (5.1), switch label-only `Line`s in `samples/Dcli.Demo/` where possible (5.2), and add an `AllowBack=true` wizard step so the new affordance has a live demonstration (5.3). +> **Currently at §6.1 — Validation & packaging.** §5 shipped (demo ports + live `AllowBack` step; LineBuilder counts 12→10 and 43→37; 714 tests still green, 0 warnings, format/validate clean). Next worker brief: implement §6 end-to-end — re-run the four gates (6.1–6.4), bump `Dcli.csproj` + `Dcli.Testing.csproj` versions from `0.1.0-rc.1` → `0.2.0-rc.1` (6.5), `dotnet pack -c Release` (6.6), update repo `CLAUDE.md`'s "Where to look for historical context" subsection to match the canonical wording from `~/.claude/skills/devlog/SKILL.md` (6.7), and finalise the DEVLOG (6.8 — orchestrator territory). diff --git a/openspec/changes/api-ergonomics-pass-1/tasks.md b/openspec/changes/api-ergonomics-pass-1/tasks.md index a1e37d6..1bb611a 100644 --- a/openspec/changes/api-ergonomics-pass-1/tasks.md +++ b/openspec/changes/api-ergonomics-pass-1/tasks.md @@ -37,9 +37,9 @@ ## 5. Demo updates -- [ ] 5.1 `samples/Dcli.Demo.DmonWizard/Engine/WizardRenderer.cs` — replace the ~12 `new LineBuilder().Text(s).Build()` sites with `Line.FromText(s)` to validate the ergonomics end-to-end -- [ ] 5.2 Where possible in `samples/Dcli.Demo/`, switch label-only `Line`s to `Line.FromText` or the string overloads -- [ ] 5.3 Add a `AllowBack=true` use case to a wizard step in `Dcli.Demo.DmonWizard` so the new affordance has a live demonstration +- [x] 5.1 `samples/Dcli.Demo.DmonWizard/Engine/WizardRenderer.cs` — replace the ~12 `new LineBuilder().Text(s).Build()` sites with `Line.FromText(s)` to validate the ergonomics end-to-end +- [x] 5.2 Where possible in `samples/Dcli.Demo/`, switch label-only `Line`s to `Line.FromText` or the string overloads +- [x] 5.3 Add a `AllowBack=true` use case to a wizard step in `Dcli.Demo.DmonWizard` so the new affordance has a live demonstration ## 6. Validation & packaging diff --git a/samples/Dcli.Demo.DmonWizard/Engine/WizardRenderer.cs b/samples/Dcli.Demo.DmonWizard/Engine/WizardRenderer.cs index e79b307..d7182da 100644 --- a/samples/Dcli.Demo.DmonWizard/Engine/WizardRenderer.cs +++ b/samples/Dcli.Demo.DmonWizard/Engine/WizardRenderer.cs @@ -26,20 +26,24 @@ private static async Task RenderChooseOneAsync( ITerminal terminal, ChooseOneStep step, CancellationToken ct) { List items = step.Options - .Select(o => new LineBuilder().Text(o.Label).Build()) + .Select(o => Line.FromText(o.Label)) .ToList(); + // AllowBack: true — Backspace at the top of the list produces DialogOutcome.Back, + // routing the user to the previous wizard step. DialogResult result = await terminal.SelectAsync( new SelectRequest( Items: items, - Title: new LineBuilder().Bold(step.Prompt).Build()), + Title: new LineBuilder().Bold(step.Prompt).Build(), + AllowBack: true), ct).ConfigureAwait(false); + if (result.Outcome == DialogOutcome.Back) + return WizardStepOutcome.Back; + if (result.Outcome == DialogOutcome.Cancelled) return WizardStepOutcome.Cancel; - // API ergonomics gap: DialogOutcome.Back is never produced by v1 dialogs. - // Esc maps to Cancelled, not Back. We treat Cancelled as Cancel here. step.SelectedIndex = result.Value; return WizardStepOutcome.Answered; } @@ -50,7 +54,7 @@ private static async Task RenderChooseManyAsync( // Ergonomics WIN: dcli MultiSelectAsync supports real multi-select. // Dmon fell back to single-pick because Spectre.Console couldn't multi-select cleanly. List items = step.Options - .Select(o => new LineBuilder().Text(o.Label).Build()) + .Select(o => Line.FromText(o.Label)) .ToList(); DialogResult result = await terminal.MultiSelectAsync( diff --git a/samples/Dcli.Demo/Program.cs b/samples/Dcli.Demo/Program.cs index e4423f4..6eccc03 100644 --- a/samples/Dcli.Demo/Program.cs +++ b/samples/Dcli.Demo/Program.cs @@ -30,9 +30,7 @@ .Fg(" Styled output flows into the real terminal scrollback.", Color.Named(Color.AnsiColor.Cyan)) .Build()); -t.Scrollback.Append(new LineBuilder() - .Text(" A small interactive region is pinned at the bottom.") - .Build()); +t.Scrollback.Append(" A small interactive region is pinned at the bottom."); t.Scrollback.Append(new LineBuilder() .Dim(" Content above the commit horizon is frozen and terminal-owned.") @@ -96,9 +94,7 @@ collapsed.Expand(); -t.Scrollback.Append(new LineBuilder() - .Text(" Subsequent content lands below the expanded block.") - .Build()); +t.Scrollback.Append(" Subsequent content lands below the expanded block."); await Task.Delay(TimeSpan.FromMilliseconds(500)); @@ -194,10 +190,10 @@ new MultiSelectRequest( Items: [ - new LineBuilder().Text("Inline scrollback rendering").Build(), - new LineBuilder().Text("Collapsible blocks").Build(), - new LineBuilder().Text("Autocomplete overlay").Build(), - new LineBuilder().Text("Dialog wizard chain").Build(), + Line.FromText("Inline scrollback rendering"), + Line.FromText("Collapsible blocks"), + Line.FromText("Autocomplete overlay"), + Line.FromText("Dialog wizard chain"), ], Title: new LineBuilder().Bold("Which features interest you?").Build()), cts.Token); From a9c64e11a86446da1bbbcfb7966f6f5b6a5919d0 Mon Sep 17 00:00:00 2001 From: Emmz Rendle Date: Thu, 28 May 2026 14:38:29 +0100 Subject: [PATCH 07/10] chore(api-ergonomics-pass-1): validation, version bump 0.2.0-rc.1, pack (section 6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 6.1 dotnet build -c Release — 0 warnings, 0 errors. - 6.2 dotnet test -c Release — 714 tests green (688 baseline + 26 new across §§1-4: 5 Line.FromText, 9 facade/fake string overloads, 7 AllowBack, 5 secret-default). - 6.3 dotnet format --verify-no-changes — clean. - 6.4 openspec validate api-ergonomics-pass-1 --strict — valid. - 6.5 Version bumped 0.1.0-rc.1 → 0.2.0-rc.1 in Dcli.csproj and Dcli.Testing.csproj (additive minor — every change in this proposal is an overload or opt-in flag, no breaking changes). - 6.6 dotnet pack -c Release produced: - dcli.0.2.0-rc.1.nupkg + dcli.0.2.0-rc.1.snupkg - dcli.testing.0.2.0-rc.1.nupkg + dcli.testing.0.2.0-rc.1.snupkg - 6.7 CLAUDE.md "Where to look for historical context" subsection updated to the canonical devlog-skill wording from ~/.claude/skills/devlog/SKILL.md ("also keep" / "inside the change directory" instead of "may also keep" / "at the change's root"), asserting the convention rather than presenting it as optional. - 6.8 DEVLOG completed: row-per-section status table, deviations recorded (the §2 file-layout deviation, §4's bug-doesn't-exist finding, §5's next-pass ergonomics candidates), resume-point shipped stamp. Co-Authored-By: Claude Opus 4.7 --- CLAUDE.md | 4 ++-- openspec/changes/api-ergonomics-pass-1/DEVLOG.md | 5 +++-- openspec/changes/api-ergonomics-pass-1/tasks.md | 16 ++++++++-------- src/Dcli.Testing/Dcli.Testing.csproj | 2 +- src/Dcli/Dcli.csproj | 2 +- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b53f186..66621ab 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,8 +16,8 @@ shipped change. Alongside `proposal.md` / `design.md` / `specs/**/*.md` / `tasks the *narrative* the spec files don't: per-section status, decisions made under uncertainty, deviations from the original plan, bugs surfaced during implementation, and human-in-the-loop verifications. When a new session needs context on *how* a prior change was built (not just *what* it specified), read the -archived `DEVLOG.md` for that change. Newly-active changes may also keep a `DEVLOG.md` at the change's -root while in-flight; when the change archives, the DEVLOG moves with it. +archived `DEVLOG.md` for that change. Newly-active changes also keep a `DEVLOG.md` inside the change +directory while in-flight; when the change archives, the DEVLOG moves with it. ### Commands diff --git a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md index a200b6b..c664bc4 100644 --- a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md +++ b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md @@ -27,7 +27,8 @@ One row per `## N.` section in `tasks.md`. Add a row when the section commits. | 2 | String-accepting consumer overloads | `55cd4de` | 702 (693 + 9) | Tasks.md target paths (`IScrollback.cs`, `*Request.cs` per record) didn't match real layout; actually edited `ITerminal.cs` (interface) and `DialogRequests.cs` (all four records together). `new InputRequest(null)` is ambiguous between the `Line?` primary and the new `string?` secondary; resolved at call sites via `new InputRequest()` (primary all-defaulted) or explicit `(string?)null` / `Line.FromText(...)`. Tests cover both. | | 3 | `AllowBack` flag on `SelectRequest`/`ChoiceRequest` | `fb90d34` | 709 (702 + 7) | No separate `ChoiceDialog` — `Choice` reuses the unified `Dialog` (`Terminal.cs:195`); created new `ChoiceDialogTests.cs` rather than splitting the class. `OverlayCloseKind` enum ordering becomes `Submit, Back, Cancel` (internal enum so no ABI consumer impact). The `Dialog.HandleKey` Backspace-Back branch is placed BEFORE the existing `TypeToFilter` Backspace-trim branch, with a `_filterText.Length == 0` guard defending against any future widening of `TypeToFilter` to Select/Choice. | | 4 | Secret-default masking in `InputDialog` | `9cbd883` | 714 (709 + 5) | **Bug didn't actually exist** — `InputDialog.Render` has unconditionally masked the buffer rows whenever `_isSecret=true` since the file was introduced in `core-rendering-architecture` (`394c9ba`). Proposal/design Decision 5's premise that "bullets only kick in once the user edits" was inaccurate. The spec scenarios still hold and are now pinned by regression tests. Introduced `_userEdited` (sticky) as a single edit-flag — it doesn't change rendering today but aligns the code with the spec's vocabulary and pins sticky semantics for any future refactor that gates masking on edit-state. | -| 5 | Demo updates | _pending_ | 714 (unchanged — samples only) | `WizardRenderer.cs` 12 → 10 LineBuilders (2 label-only `Line.FromText` replacements); `Dcli.Demo/Program.cs` 43 → 37 (6 replacements — 2 via `Scrollback.Append(string)`, 4 via `Line.FromText` in MultiSelect items). 5.3 took Option A — `WizardEngine.cs` already routed `WizardStepOutcome.Back`, so activating `AllowBack: true` in `RenderChooseOneAsync` + mapping `DialogOutcome.Back → WizardStepOutcome.Back` was sufficient; no engine change. | +| 5 | Demo updates | `3bde5fd` | 714 (unchanged — samples only) | `WizardRenderer.cs` 12 → 10 LineBuilders (2 label-only `Line.FromText` replacements); `Dcli.Demo/Program.cs` 43 → 37 (6 replacements — 2 via `Scrollback.Append(string)`, 4 via `Line.FromText` in MultiSelect items). 5.3 took Option A — `WizardEngine.cs` already routed `WizardStepOutcome.Back`, so activating `AllowBack: true` in `RenderChooseOneAsync` + mapping `DialogOutcome.Back → WizardStepOutcome.Back` was sufficient; no engine change. | +| 6 | Validation & packaging | _pending_ | 714 (unchanged) | Orchestrator-direct (no feature code, so no worker/reviewer cycle). Gates re-ran clean: build 0 warnings, 714 tests green, format clean, validate clean. Versions bumped `0.1.0-rc.1 → 0.2.0-rc.1` in both `Dcli.csproj` and `Dcli.Testing.csproj`. `dotnet pack -c Release` produced all four expected artifacts: `dcli.0.2.0-rc.1.{nupkg,snupkg}` and `dcli.testing.0.2.0-rc.1.{nupkg,snupkg}`. CLAUDE.md "Where to look for historical context" subsection updated to the canonical devlog-skill wording ("also keep" / "inside the change directory"). | ## Decisions & deviations @@ -67,4 +68,4 @@ Surface gaps for future changes. Link to memory files where the constraint is en ## Resume point -> **Currently at §6.1 — Validation & packaging.** §5 shipped (demo ports + live `AllowBack` step; LineBuilder counts 12→10 and 43→37; 714 tests still green, 0 warnings, format/validate clean). Next worker brief: implement §6 end-to-end — re-run the four gates (6.1–6.4), bump `Dcli.csproj` + `Dcli.Testing.csproj` versions from `0.1.0-rc.1` → `0.2.0-rc.1` (6.5), `dotnet pack -c Release` (6.6), update repo `CLAUDE.md`'s "Where to look for historical context" subsection to match the canonical wording from `~/.claude/skills/devlog/SKILL.md` (6.7), and finalise the DEVLOG (6.8 — orchestrator territory). +> **All six sections shipped.** Final commit hash for §6 will be added once the section commit lands. Tests: 714 green (688 baseline + 26 new across §§1-4). Gates clean. Artifacts: `dcli.0.2.0-rc.1.{nupkg,snupkg}` + `dcli.testing.0.2.0-rc.1.{nupkg,snupkg}`. Ready for `/opsx:archive` once the user confirms (per the project's `CLAUDE.md` §5 — do not archive automatically; the freeze stamp moves the DEVLOG into the archive directory with the rest of the change). diff --git a/openspec/changes/api-ergonomics-pass-1/tasks.md b/openspec/changes/api-ergonomics-pass-1/tasks.md index 1bb611a..8d29928 100644 --- a/openspec/changes/api-ergonomics-pass-1/tasks.md +++ b/openspec/changes/api-ergonomics-pass-1/tasks.md @@ -43,11 +43,11 @@ ## 6. Validation & packaging -- [ ] 6.1 `dotnet build -c Release` — 0 warnings -- [ ] 6.2 `dotnet test -c Release` — green (688 baseline + ~12 new) -- [ ] 6.3 `dotnet format --verify-no-changes` — clean -- [ ] 6.4 `openspec validate api-ergonomics-pass-1 --strict` — valid -- [ ] 6.5 Bump `Dcli.csproj` and `Dcli.Testing.csproj` `Version` from `0.1.0-rc.1` → `0.2.0-rc.1` (additive minor) -- [ ] 6.6 `dotnet pack -c Release` produces `dcli.0.2.0-rc.1.{nupkg,snupkg}` + `dcli.testing.0.2.0-rc.1.{nupkg,snupkg}` cleanly -- [ ] 6.7 Update the repo `CLAUDE.md`'s "Where to look for historical context on a shipped change" subsection to match the canonical wording recommended by the personal `devlog` skill at `~/.claude/skills/devlog/SKILL.md` (covers BOTH the in-flight DEVLOG inside the change directory AND the archived DEVLOG — the wording shipped in PR #2 only covered the archived case) -- [ ] 6.8 Update `openspec/changes/api-ergonomics-pass-1/DEVLOG.md` per the `devlog` skill conventions: a row in the Section status table for each section commit, deviations as they happen, resume-point bumped after each section +- [x] 6.1 `dotnet build -c Release` — 0 warnings +- [x] 6.2 `dotnet test -c Release` — green (688 baseline + ~12 new) +- [x] 6.3 `dotnet format --verify-no-changes` — clean +- [x] 6.4 `openspec validate api-ergonomics-pass-1 --strict` — valid +- [x] 6.5 Bump `Dcli.csproj` and `Dcli.Testing.csproj` `Version` from `0.1.0-rc.1` → `0.2.0-rc.1` (additive minor) +- [x] 6.6 `dotnet pack -c Release` produces `dcli.0.2.0-rc.1.{nupkg,snupkg}` + `dcli.testing.0.2.0-rc.1.{nupkg,snupkg}` cleanly +- [x] 6.7 Update the repo `CLAUDE.md`'s "Where to look for historical context on a shipped change" subsection to match the canonical wording recommended by the personal `devlog` skill at `~/.claude/skills/devlog/SKILL.md` (covers BOTH the in-flight DEVLOG inside the change directory AND the archived DEVLOG — the wording shipped in PR #2 only covered the archived case) +- [x] 6.8 Update `openspec/changes/api-ergonomics-pass-1/DEVLOG.md` per the `devlog` skill conventions: a row in the Section status table for each section commit, deviations as they happen, resume-point bumped after each section diff --git a/src/Dcli.Testing/Dcli.Testing.csproj b/src/Dcli.Testing/Dcli.Testing.csproj index 0a2e981..47cfb1e 100644 --- a/src/Dcli.Testing/Dcli.Testing.csproj +++ b/src/Dcli.Testing/Dcli.Testing.csproj @@ -7,7 +7,7 @@ true dcli.testing - 0.1.0-rc.1 + 0.2.0-rc.1 daemonicai daemonicai Headless test harness for the dcli inline terminal-rendering library. diff --git a/src/Dcli/Dcli.csproj b/src/Dcli/Dcli.csproj index 575c385..a700c97 100644 --- a/src/Dcli/Dcli.csproj +++ b/src/Dcli/Dcli.csproj @@ -9,7 +9,7 @@ true dcli - 0.1.0-rc.1 + 0.2.0-rc.1 daemonicai daemonicai Inline terminal-rendering library. Claude-Code-style styled output flows into the terminal's real scrollback, with a small interactive region pinned at the bottom. From f3ac49b8308f150fb71b7d0208e67de729ef47f5 Mon Sep 17 00:00:00 2001 From: Emmz Rendle Date: Thu, 28 May 2026 14:38:42 +0100 Subject: [PATCH 08/10] =?UTF-8?q?chore(api-ergonomics-pass-1):=20backfill?= =?UTF-8?q?=20=C2=A76=20commit=20hash=20in=20DEVLOG?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.7 --- openspec/changes/api-ergonomics-pass-1/DEVLOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md index c664bc4..9d17163 100644 --- a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md +++ b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md @@ -28,7 +28,7 @@ One row per `## N.` section in `tasks.md`. Add a row when the section commits. | 3 | `AllowBack` flag on `SelectRequest`/`ChoiceRequest` | `fb90d34` | 709 (702 + 7) | No separate `ChoiceDialog` — `Choice` reuses the unified `Dialog` (`Terminal.cs:195`); created new `ChoiceDialogTests.cs` rather than splitting the class. `OverlayCloseKind` enum ordering becomes `Submit, Back, Cancel` (internal enum so no ABI consumer impact). The `Dialog.HandleKey` Backspace-Back branch is placed BEFORE the existing `TypeToFilter` Backspace-trim branch, with a `_filterText.Length == 0` guard defending against any future widening of `TypeToFilter` to Select/Choice. | | 4 | Secret-default masking in `InputDialog` | `9cbd883` | 714 (709 + 5) | **Bug didn't actually exist** — `InputDialog.Render` has unconditionally masked the buffer rows whenever `_isSecret=true` since the file was introduced in `core-rendering-architecture` (`394c9ba`). Proposal/design Decision 5's premise that "bullets only kick in once the user edits" was inaccurate. The spec scenarios still hold and are now pinned by regression tests. Introduced `_userEdited` (sticky) as a single edit-flag — it doesn't change rendering today but aligns the code with the spec's vocabulary and pins sticky semantics for any future refactor that gates masking on edit-state. | | 5 | Demo updates | `3bde5fd` | 714 (unchanged — samples only) | `WizardRenderer.cs` 12 → 10 LineBuilders (2 label-only `Line.FromText` replacements); `Dcli.Demo/Program.cs` 43 → 37 (6 replacements — 2 via `Scrollback.Append(string)`, 4 via `Line.FromText` in MultiSelect items). 5.3 took Option A — `WizardEngine.cs` already routed `WizardStepOutcome.Back`, so activating `AllowBack: true` in `RenderChooseOneAsync` + mapping `DialogOutcome.Back → WizardStepOutcome.Back` was sufficient; no engine change. | -| 6 | Validation & packaging | _pending_ | 714 (unchanged) | Orchestrator-direct (no feature code, so no worker/reviewer cycle). Gates re-ran clean: build 0 warnings, 714 tests green, format clean, validate clean. Versions bumped `0.1.0-rc.1 → 0.2.0-rc.1` in both `Dcli.csproj` and `Dcli.Testing.csproj`. `dotnet pack -c Release` produced all four expected artifacts: `dcli.0.2.0-rc.1.{nupkg,snupkg}` and `dcli.testing.0.2.0-rc.1.{nupkg,snupkg}`. CLAUDE.md "Where to look for historical context" subsection updated to the canonical devlog-skill wording ("also keep" / "inside the change directory"). | +| 6 | Validation & packaging | `a9c64e1` | 714 (unchanged) | Orchestrator-direct (no feature code, so no worker/reviewer cycle). Gates re-ran clean: build 0 warnings, 714 tests green, format clean, validate clean. Versions bumped `0.1.0-rc.1 → 0.2.0-rc.1` in both `Dcli.csproj` and `Dcli.Testing.csproj`. `dotnet pack -c Release` produced all four expected artifacts: `dcli.0.2.0-rc.1.{nupkg,snupkg}` and `dcli.testing.0.2.0-rc.1.{nupkg,snupkg}`. CLAUDE.md "Where to look for historical context" subsection updated to the canonical devlog-skill wording ("also keep" / "inside the change directory"). | ## Decisions & deviations From 2ddfa81b232d7e3a74873d5376b7d37b5cf35e22 Mon Sep 17 00:00:00 2001 From: Emmz Rendle Date: Thu, 28 May 2026 14:43:09 +0100 Subject: [PATCH 09/10] openspec(api-ergonomics-pass-1): sync delta specs + freeze DEVLOG before archive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fold the change's delta specs into the main openspec/specs/: * styled-text/spec.md gains the ADDED requirement "Convenience construction from plain strings" (Line.FromText factory + string overloads on IScrollback.Append and *Request records; no implicit string → Line). * fixed-region/spec.md's "Awaitable modal dialogs" requirement is replaced to add the AllowBack opt-in flag on Select/Choice (Backspace-at-empty); MultiSelect and Input explicitly do NOT expose AllowBack. * fixed-region/spec.md's "Owned input editor" requirement is replaced to add secret-default masking (mask the seeded Default until first edit when IsSecret=true; Submit returns the real string regardless). - Freeze the DEVLOG to shipped framing per ~/.claude/skills/devlog/SKILL.md: drop the in-flight banner, replace "How to resume" with "Final state at archive", and turn the resume-point pointer into a shipped stamp citing final commit f3ac49b, archive date 2026-05-28, and the published NuGet artifacts. Co-Authored-By: Claude Opus 4.7 --- .../changes/api-ergonomics-pass-1/DEVLOG.md | 39 ++++++++------ openspec/specs/fixed-region/spec.md | 53 +++++++++++++++++++ openspec/specs/styled-text/spec.md | 43 +++++++++++++++ 3 files changed, 118 insertions(+), 17 deletions(-) diff --git a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md index 9d17163..3de9f77 100644 --- a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md +++ b/openspec/changes/api-ergonomics-pass-1/DEVLOG.md @@ -1,21 +1,19 @@ # DEVLOG — `api-ergonomics-pass-1` -> **Status: in-flight.** Maintained while applying the change per the OpenSpec apply -> workflow (see the project's `CLAUDE.md`). Captures per-section narrative the spec -> files don't carry: decisions under uncertainty, deviations, surfaced bugs, HITL -> verifications. On archive this file moves with the change to -> `openspec/changes/archive/YYYY-MM-DD-api-ergonomics-pass-1/DEVLOG.md` and the -> status flips to **shipped** (see `/devlog freeze`). - -## How to resume - -- Branch: **`change/api-ergonomics-pass-1`** (created from `main`). Stay on it. -- Working tree state: **CLEAN** (no section work has started; only OpenSpec artefacts + this DEVLOG are committed). -- Sanity check command: - `dotnet build -c Release && dotnet test -c Release && dotnet format --verify-no-changes && openspec validate api-ergonomics-pass-1 --strict` - → expect **0 warnings, 688 tests green** (baseline from `core-rendering-architecture`), clean format, valid. -- Resume point: **§1 — `Line.FromText` factory** (first unticked task: `1.1`). See the Section status table for what's done so far. -- Check the memory files listed at the bottom before briefing — several encode hard-won constraints that survive into this change (in particular: CA2007 / loop-thread discipline if any new dialog test touches the loop). +> **Status: shipped.** Archived `2026-05-28` to `openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/`. +> Final on-branch commit before archive: `f3ac49b` (§6 hash backfill) — see Section status for +> per-section hashes. Branch: `change/api-ergonomics-pass-1`. Delta specs synced into +> `openspec/specs/{styled-text,fixed-region}/spec.md` immediately before the archive move. + +## Final state at archive + +- Branch: **`change/api-ergonomics-pass-1`** (created from `main`). Held all section commits. +- Working tree at archive: clean post-sync; the archive move was its own commit. +- Sanity check command (post-archive, against `main`-side specs): + `dotnet build -c Release && dotnet test -c Release && dotnet format --verify-no-changes` + → 0 warnings, **714 tests green** (688 baseline + 26 new across §§1-4), format clean. +- All six sections shipped. See Section status table for per-section commit hashes and tests-after counts. +- Memory files referenced during the change: see "Memory files" section below — those constraints survive into future changes. ## Section status @@ -68,4 +66,11 @@ Surface gaps for future changes. Link to memory files where the constraint is en ## Resume point -> **All six sections shipped.** Final commit hash for §6 will be added once the section commit lands. Tests: 714 green (688 baseline + 26 new across §§1-4). Gates clean. Artifacts: `dcli.0.2.0-rc.1.{nupkg,snupkg}` + `dcli.testing.0.2.0-rc.1.{nupkg,snupkg}`. Ready for `/opsx:archive` once the user confirms (per the project's `CLAUDE.md` §5 — do not archive automatically; the freeze stamp moves the DEVLOG into the archive directory with the rest of the change). +> **Shipped — `2026-05-28`.** All six sections landed on `change/api-ergonomics-pass-1`; final +> branch commit `f3ac49b`. Delta specs synced into the main `openspec/specs/{styled-text,fixed-region}/` +> immediately before archive. NuGet artifacts produced: `dcli.0.2.0-rc.1.{nupkg,snupkg}` and +> `dcli.testing.0.2.0-rc.1.{nupkg,snupkg}`. Follow-ups (multi-select Back, `Input.Prompt`/`ReadOnly`, +> `Scrollback.AppendRule`, incremental `Collapsible.AppendLine`, `PasteEvent` editor routing, +> VT-escape sanitisation of `Segment.Text`, and a likely ergonomics-pass-2 for +> `Line.Bold(s)`/`Dim(s)`/`Fg(s,color)` shorthands) live as separate future OpenSpec changes — +> see "Open follow-ups" above. diff --git a/openspec/specs/fixed-region/spec.md b/openspec/specs/fixed-region/spec.md index 681548a..ea00741 100644 --- a/openspec/specs/fixed-region/spec.md +++ b/openspec/specs/fixed-region/spec.md @@ -12,16 +12,38 @@ The fixed region SHALL be a contiguous, bottom-pinned stack of components — in - **THEN** the fixed region remains pinned at the bottom and is redrawn in place ### Requirement: Owned input editor + The library SHALL own an input editor supporting caret movement, multiline text, display-width-aware wrapping, history recall, and internal scrolling when its content exceeds its allotted height. +When an `InputRequest` is constructed with `IsSecret = true` and a non-empty `Default`, the editor SHALL render the seeded default as `'•'` repeated by the default's display width on every paint that occurs before the user's first edit. Once the user makes any edit (insert, delete, paste, history-recall), the editor SHALL fall back to the existing secret-render path (which masks the current buffer contents as bullets on each paint). The `Submit` outcome SHALL return the real string contents — either the unedited `Default` or the edited text — regardless of how it was rendered. + +When `IsSecret = false`, the editor SHALL render the seeded default as plain text (unchanged from v1 behaviour). + #### Scenario: Wrapping tracks the caret + - **WHEN** input text exceeds the available width - **THEN** the text wraps and the caret is positioned at the correct visual row and column #### Scenario: History recall + - **WHEN** the user navigates input history - **THEN** the buffer is replaced with the recalled entry +#### Scenario: Secret default is masked before first edit + +- **WHEN** an `InputRequest` is shown with `IsSecret=true` and a non-empty `Default` and the user has not yet edited the buffer +- **THEN** the paint shows bullets (`•`) at every column the default occupies, never the default's clear-text content + +#### Scenario: Secret default reveals real text on submit + +- **WHEN** the user submits an `InputRequest` with `IsSecret=true` and a non-empty `Default` without editing the buffer +- **THEN** the awaited `DialogResult.Value` is the real `Default` string (not bullets) + +#### Scenario: Non-secret default renders as plain text + +- **WHEN** an `InputRequest` is shown with `IsSecret=false` and a non-empty `Default` +- **THEN** the paint shows the default's clear-text content (no masking) + ### Requirement: Fixed-region height budget The fixed region SHALL have a `MaxHeight` equal to `clamp(appSet ?? 50% of rows, 8, rows)`, SHALL consume only the rows its content needs up to that cap, and SHALL scroll components internally beyond the cap. @@ -75,24 +97,55 @@ The hardware cursor SHALL park at the input caret normally and during Autocomple - **THEN** the hardware cursor is hidden ### Requirement: Awaitable modal dialogs + The library SHALL expose select, multi-select, input, and choice dialogs as awaitable operations that return a `DialogResult` whose outcome is `Submitted`, `Back`, or `Cancelled`. +`SelectRequest` and `ChoiceRequest` SHALL expose an opt-in `AllowBack` flag (default `false`). When `AllowBack` is `true`, the dialog SHALL produce `DialogOutcome.Back` if the user presses **Backspace** before moving the selection (i.e. before any `↑`/`↓` keystroke is consumed by the dialog). Once the selection has been moved, Backspace SHALL be a no-op for the remainder of that overlay session. When `AllowBack` is `false` (the default), Backspace SHALL have no effect on `SelectRequest`/`ChoiceRequest` overlays — preserving v1 behaviour. + +`MultiSelectRequest` SHALL NOT expose `AllowBack` in this revision; its Back-semantics are deferred. + +`InputRequest` SHALL NOT expose `AllowBack`; in an input dialog Backspace is an editing key. + #### Scenario: Select submitted + - **WHEN** the user highlights an item in a select dialog and presses Enter - **THEN** the awaited result is `Submitted` carrying the chosen index #### Scenario: Cancelled by escape + - **WHEN** the user presses Escape in a dialog - **THEN** the awaited result is `Cancelled` #### Scenario: Multi-select toggling + - **WHEN** the user presses space on items in a multi-select dialog - **THEN** those items toggle in the returned selection set #### Scenario: Cancellation token closes the dialog + - **WHEN** the `CancellationToken` passed to a dialog is cancelled - **THEN** the overlay closes and the awaited result is `Cancelled` +#### Scenario: AllowBack=true on Select produces Back + +- **WHEN** a `SelectRequest` with `AllowBack=true` is shown and the user presses Backspace before moving the selection +- **THEN** the awaited result is `DialogOutcome.Back` + +#### Scenario: AllowBack=true on Choice produces Back + +- **WHEN** a `ChoiceRequest` with `AllowBack=true` is shown and the user presses Backspace before moving the selection +- **THEN** the awaited result is `DialogOutcome.Back` + +#### Scenario: AllowBack=true is suppressed after movement + +- **WHEN** the user presses `↓` (consumed by the dialog) and then presses Backspace in a `SelectRequest` with `AllowBack=true` +- **THEN** Backspace is ignored for the rest of that overlay session — neither `Back` nor `Cancelled` is produced + +#### Scenario: AllowBack=false is the default + +- **WHEN** a `SelectRequest` or `ChoiceRequest` is constructed without setting `AllowBack` +- **THEN** Backspace has no effect on the dialog and existing v1 behaviour is preserved + ### Requirement: Consumer-driven autocomplete Autocomplete candidates SHALL be supplied by the consumer in response to input-change notifications; the library SHALL render and navigate them within the overlay's row cap and apply the accepted candidate's insert text to the input buffer. diff --git a/openspec/specs/styled-text/spec.md b/openspec/specs/styled-text/spec.md index c11100f..f3278bc 100644 --- a/openspec/specs/styled-text/spec.md +++ b/openspec/specs/styled-text/spec.md @@ -50,3 +50,46 @@ The same styled-text primitives SHALL be usable both for scrollback content and #### Scenario: Reuse in both zones - **WHEN** a caller builds a status line and a scrollback line - **THEN** both are expressed with the same `Segment`/`Line`/`Style` types + +### Requirement: Convenience construction from plain strings + +The library SHALL expose a `Line.FromText(string text, Style? style = null)` static factory and SHALL accept plain `string` values wherever a `Line` is currently required on consumer-facing construction surfaces, so label-only call sites do not have to lift through `LineBuilder`. The library SHALL NOT define an implicit conversion from `string` to `Line`; the factory and the overloads SHALL be the only seams. + +The string-accepting overloads SHALL exist on: +- `IScrollback.Append(string text)` alongside `Append(Line line)`. +- `InputRequest` accepting a `string? Prompt` alongside its existing `Line? Prompt`. +- `SelectRequest` accepting `IReadOnlyList` and `params string[]` items alongside its existing `Line` item list. +- `MultiSelectRequest` accepting `IReadOnlyList` and `params string[]` items alongside its existing `Line` item list. +- `ChoiceRequest` accepting `IReadOnlyList` and `params string[]` options alongside its existing `Line` option list. + +Each overload SHALL be semantically equivalent to constructing the corresponding `Line` via `Line.FromText` with default style and passing it to the existing API. + +#### Scenario: FromText produces an unstyled line + +- **WHEN** a caller invokes `Line.FromText("hello")` with no style argument +- **THEN** the returned `Line` contains a single `Segment` whose text is `"hello"` and whose style is the default (no foreground/background, `Format.None`) + +#### Scenario: FromText respects an explicit style + +- **WHEN** a caller invokes `Line.FromText("err", new Style(Format: Format.Bold))` +- **THEN** the returned `Line` contains a single `Segment` whose text is `"err"` and whose style has `Format.Bold` + +#### Scenario: String-accepting Scrollback.Append is equivalent to the Line form + +- **WHEN** a caller invokes `terminal.Scrollback.Append("hello")` +- **THEN** the appended object is equal to what `terminal.Scrollback.Append(Line.FromText("hello"))` would have produced + +#### Scenario: String-accepting dialog requests are equivalent to the Line form + +- **WHEN** a caller constructs a `SelectRequest` (or `MultiSelectRequest` or `ChoiceRequest`) with `params string[]` or `IReadOnlyList` items +- **THEN** the resulting items are equal to those produced by mapping each string through `Line.FromText` with default style + +#### Scenario: InputRequest accepts a string prompt + +- **WHEN** a caller constructs an `InputRequest` with a `string` prompt +- **THEN** the resulting request's effective prompt is equal to `Line.FromText(prompt)` with default style + +#### Scenario: No implicit conversion is defined + +- **WHEN** a consumer attempts to pass a plain `string` to an API that takes only `Line` (with no string overload added by this change) +- **THEN** the code SHALL fail to compile — there SHALL be no implicit `string`→`Line` conversion defined anywhere in the public surface From 50c4456b46b90a85af77db0d42f5ec4646ee7315 Mon Sep 17 00:00:00 2001 From: Emmz Rendle Date: Thu, 28 May 2026 14:43:22 +0100 Subject: [PATCH 10/10] =?UTF-8?q?openspec:=20archive=20api-ergonomics-pass?= =?UTF-8?q?-1=20=E2=86=92=202026-05-28-api-ergonomics-pass-1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All six sections shipped. Delta specs already synced into main specs in the preceding commit. DEVLOG frozen with the shipped stamp. Co-Authored-By: Claude Opus 4.7 --- .../2026-05-28-api-ergonomics-pass-1}/.openspec.yaml | 0 .../2026-05-28-api-ergonomics-pass-1}/DEVLOG.md | 0 .../2026-05-28-api-ergonomics-pass-1}/design.md | 0 .../2026-05-28-api-ergonomics-pass-1}/proposal.md | 0 .../2026-05-28-api-ergonomics-pass-1}/specs/fixed-region/spec.md | 0 .../2026-05-28-api-ergonomics-pass-1}/specs/styled-text/spec.md | 0 .../2026-05-28-api-ergonomics-pass-1}/tasks.md | 0 7 files changed, 0 insertions(+), 0 deletions(-) rename openspec/changes/{api-ergonomics-pass-1 => archive/2026-05-28-api-ergonomics-pass-1}/.openspec.yaml (100%) rename openspec/changes/{api-ergonomics-pass-1 => archive/2026-05-28-api-ergonomics-pass-1}/DEVLOG.md (100%) rename openspec/changes/{api-ergonomics-pass-1 => archive/2026-05-28-api-ergonomics-pass-1}/design.md (100%) rename openspec/changes/{api-ergonomics-pass-1 => archive/2026-05-28-api-ergonomics-pass-1}/proposal.md (100%) rename openspec/changes/{api-ergonomics-pass-1 => archive/2026-05-28-api-ergonomics-pass-1}/specs/fixed-region/spec.md (100%) rename openspec/changes/{api-ergonomics-pass-1 => archive/2026-05-28-api-ergonomics-pass-1}/specs/styled-text/spec.md (100%) rename openspec/changes/{api-ergonomics-pass-1 => archive/2026-05-28-api-ergonomics-pass-1}/tasks.md (100%) diff --git a/openspec/changes/api-ergonomics-pass-1/.openspec.yaml b/openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/.openspec.yaml similarity index 100% rename from openspec/changes/api-ergonomics-pass-1/.openspec.yaml rename to openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/.openspec.yaml diff --git a/openspec/changes/api-ergonomics-pass-1/DEVLOG.md b/openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/DEVLOG.md similarity index 100% rename from openspec/changes/api-ergonomics-pass-1/DEVLOG.md rename to openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/DEVLOG.md diff --git a/openspec/changes/api-ergonomics-pass-1/design.md b/openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/design.md similarity index 100% rename from openspec/changes/api-ergonomics-pass-1/design.md rename to openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/design.md diff --git a/openspec/changes/api-ergonomics-pass-1/proposal.md b/openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/proposal.md similarity index 100% rename from openspec/changes/api-ergonomics-pass-1/proposal.md rename to openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/proposal.md diff --git a/openspec/changes/api-ergonomics-pass-1/specs/fixed-region/spec.md b/openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/specs/fixed-region/spec.md similarity index 100% rename from openspec/changes/api-ergonomics-pass-1/specs/fixed-region/spec.md rename to openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/specs/fixed-region/spec.md diff --git a/openspec/changes/api-ergonomics-pass-1/specs/styled-text/spec.md b/openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/specs/styled-text/spec.md similarity index 100% rename from openspec/changes/api-ergonomics-pass-1/specs/styled-text/spec.md rename to openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/specs/styled-text/spec.md diff --git a/openspec/changes/api-ergonomics-pass-1/tasks.md b/openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/tasks.md similarity index 100% rename from openspec/changes/api-ergonomics-pass-1/tasks.md rename to openspec/changes/archive/2026-05-28-api-ergonomics-pass-1/tasks.md