diff --git a/docs/memory/cli/index.md b/docs/memory/cli/index.md index eaa7c11..15913db 100644 --- a/docs/memory/cli/index.md +++ b/docs/memory/cli/index.md @@ -8,6 +8,7 @@ description: "CLI source structure (cmd/idea + internal/idea + version wiring), | File | Description | Last Updated | |------|-------------|-------------| | [edit](edit.md) | `idea edit` two-form contract: inline replacement (two-arg form) vs. $EDITOR round-trip (`edit `) — editor resolution chain, temp-file mechanics, edge/exit semantics, and the fake-editor test seam | 2026-06-12 | -| [prune](prune.md) | Bulk-remove subcommand (`idea prune`): dry-run-by-default/`--force` contract, stdout/stderr channel split, exit codes, the deliberate non-archival design, and the `removeIdeaAt` seam shared with `Rm` | 2026-06-12 | -| [structure](structure.md) | Source tree layout (cmd/idea + internal/idea), root command factory, command aliases vs. the bare-text shorthand, backlog line lifecycle (lenient read / canonical write incl. the escaped-text convention for multiline ideas and the explicit `idea fmt` canonicalizer with bare-checkbox adoption), help-dump contract, and version stamping | 2026-06-12 | +| [list](list.md) | `idea list`/`ls` rendering contract: TTY-aware rune-safe text truncation, the `--full` flag, the optional `[id...]` positional filter, ANSI color (NO_COLOR-gated), and the pipe contract that keeps piped output canonical | 2026-06-13 | +| [prune](prune.md) | Bulk-remove subcommand (`idea prune`): the TTY × `--force` decision matrix (pipe dry-run vs. interactive `[y/N]` confirm), the leading stderr count header, TTY-aware truncation/color/`--full` listing, stdout/stderr channel split, exit codes, the deliberate non-archival design, and the `removeIdeaAt` seam shared with `Rm` | 2026-06-13 | +| [structure](structure.md) | Source tree layout (cmd/idea + internal/idea), root command factory, command aliases vs. the bare-text shorthand, backlog line lifecycle (lenient read / canonical write incl. the escaped-text convention for multiline ideas and the explicit `idea fmt` canonicalizer with bare-checkbox adoption), the TTY/width/color/truncation seam (internal/idea/term.go) + shared printIdeaLines render path, the golang.org/x/term direct dependency, help-dump contract, and version stamping | 2026-06-13 | | [update](update.md) | Self-update subcommand (`idea update`): Homebrew-backed upgrade flow, non-brew install fallback hint, and the `--skip-brew-update` flag | 2026-06-10 | diff --git a/docs/memory/cli/list.md b/docs/memory/cli/list.md new file mode 100644 index 0000000..58de7b5 --- /dev/null +++ b/docs/memory/cli/list.md @@ -0,0 +1,65 @@ +--- +description: "`idea list`/`ls` rendering contract: TTY-aware rune-safe text truncation, the `--full` flag, the optional `[id...]` positional filter, ANSI color (NO_COLOR-gated), and the pipe contract that keeps piped output canonical" +--- + +# `idea list` / `ls` Subcommand + +`idea list` (alias `ls`) lists ideas from the backlog. The cobra wrapper lives at `src/cmd/idea/list.go` (`listCmd()` factory); the TTY/width/color/truncation logic lives in `src/internal/idea/term.go` (Constitution IV seam). Open ideas show by default; `--all/-a` adds done ideas, `--done` shows only done, `--json` emits the structured records, `--sort` (`date`|`id`) and `--reverse` order them. The `ls` alias is documented in `structure.md` (§ Command aliases). The display features below were added by `260613-kfcl-tty-aware-output-rendering`. + +The originating DX pain: ideas in this project are frequently paragraph-length, so on a terminal they soft-wrap into many visual rows and short ideas drown between long ones — and the scannable `[id] date:` anchor is buried. + +## TTY-aware rendering (truncation + color) + +All display rendering is **TTY-gated** so piped output stays full and canonical (Constitution VI). The single render path is `printIdeaLines` in `cmd/idea/output.go` (see `structure.md` § Shared TTY-aware render path), shared with `prune`: + +- **On a terminal** (stdout is a TTY): each idea renders via `idea.DisplayListLine(i, width, full, color)` — text truncated to the terminal width (unless `--full`), prefix dimmed and a done `[x]` greened (unless `NO_COLOR`/non-TTY). +- **Piped or redirected** (non-TTY): full canonical `FormatLine` output, regardless of `--full` — `--full` is meaningful only on a TTY. +- **`--json`** is unaffected in all cases: structured records (`id`, `date`, `status`, `text`) are emitted unchanged. The display features are display-only — `FormatLine`/`DisplayLine`, the parser, the backlog format, and the `--json` schema are all untouched. + +### Truncation (`DisplayListLine` / `truncateText`) + +`DisplayListLine` builds the canonical escaped line shape but clips only the **text portion**: + +- The `- [done] [id] date: ` prefix is **NEVER** truncated — it is the scannable anchor. The available text width is `width - len([]rune(prefix))`. +- Truncation is **rune-safe**: `truncateText` operates on `[]rune`, never byte slices, so multibyte (CJK/emoji) text is never cut mid-rune. Wide-glyph display-width awareness is an explicit non-goal — rune-count against columns is the floor. +- A single-rune ellipsis `…` (U+2026, the `ellipsis` const) is appended when text is clipped. +- A **multiline** idea (escaped text containing a literal `\n` escape) is always clipped at the first newline with `…` — regardless of width — so a rendered list line is always exactly one physical row. +- **Degenerate width**: when the available text width is non-positive (prefix alone fills/exceeds the terminal) the text reduces to just `…`; when `avail <= 1` only the ellipsis is emitted. The prefix is still never clipped. + +### `--full` flag + +`--full` (boolean, default false) disables truncation on a TTY: full text is shown (still colored). It has no effect when piped (output is already full canonical there). `prune` carries the same flag for symmetry (see `prune.md`). + +### Color (NO_COLOR-gated) + +When `idea.UseColor(os.Stdout)` is true (TTY **and** `NO_COLOR` unset — presence disables color regardless of value per the NO_COLOR spec), `DisplayListLine` dims the `- `/`[id] date:` spans (ANSI faint `\033[2m`) and greens a done `[x]` checkbox (`\033[32m`). Color is applied **after** truncation so the width math counts visible runes, never escape bytes (see `structure.md` § term.go seam). The checkbox is rebuilt as its own span between two dim spans so the id/date stay faint while a done `[x]` stays green. + +## Optional `[id...]` positional filter + +`idea list`/`ls` accepts zero-or-more positional ID arguments (`Use: "list [id...]"`). The behavior: + +| Argument | Behavior | +|----------|----------| +| (none) | List all ideas matching the active filter (`--all`/`--done`) + sort. | +| Well-formed IDs present in the backlog | List only those ideas, still respecting filter/`--sort`/`--reverse`/truncation/color. | +| Well-formed but **absent** ID (`zzzz`) | `warning: no idea with ID "zzzz"` on **stderr** (one line per missing ID), and the matched survivors are still listed (warn-and-list-the-rest — pipe-friendly stdout posture). | +| **Malformed** ID (not `[a-z0-9]{4}`) | Usage error via `idea.ValidateID` in the cobra `Args` validator — the command never runs. | + +The split is deliberate: a malformed argument is a *usage mistake* (caught up front by `Args`), a well-formed-but-absent ID is a *not-found* condition (warn + continue). The filter lives in the `filterByIDs(cmd, ideas, args)` helper in `list.go`. `idea show ` remains the single-idea full-detail command; `ls --full` overlapping `show` is accepted mild redundancy, not a conflict. + +## Help text + +`Long` documents the truncation/`--full`/`[id...]` behavior and the pipe contract; `Short` stays the byte-stable one-liner (repo convention, `structure.md` § Command help text). The help-dump JSON schema is unchanged — the list node's `text` updates automatically since it reproduces `-h` output (including cobra's `Aliases: list, ls` line). + +## Tests + +- `src/cmd/idea/main_test.go` — `TestList_IDFilter` (filter to listed IDs; unknown-ID stderr warning naming the missing ID with survivors listed; malformed-ID usage error) and `TestList_PipedOutputIsCanonical` (piped `ls` / `ls --full` is byte-identical to the `FormatLine` listing — no ANSI, no `…`), via the existing `buildBinary`/`setupGitRepo`/`writeRepoBacklog`/`runSplit` helpers. +- `src/internal/idea/term_test.go` — `DisplayListLine`/`truncateText` rune-safety, prefix-never-truncated, ellipsis presence, multiline-at-first-newline, `full` bypasses truncation, and color-applied-after-truncation (see `structure.md` for the term-seam test list). + +## Cross-references + +- Source-tree placement, the `ls` alias and bare-text namespace rule, the `term.go` TTY/width/color/truncation seam, and the shared `printIdeaLines` render path: `structure.md`. +- The same TTY-aware rendering applied to the prune dry-run, plus the count header and interactive confirm: `prune.md`. +- Command table: `../../specs/overview.md` (note: the overview still describes the pre-change `list` row). +- Constitution Principles IV (logic in `internal/idea`) and VI (machine-parseable stdout): `fab/project/constitution.md`. +- Originating change: `260613-kfcl-tty-aware-output-rendering`. diff --git a/docs/memory/cli/prune.md b/docs/memory/cli/prune.md index 1798f55..80e59b3 100644 --- a/docs/memory/cli/prune.md +++ b/docs/memory/cli/prune.md @@ -1,28 +1,35 @@ --- -description: "Bulk-remove subcommand (`idea prune`): dry-run-by-default/`--force` contract, stdout/stderr channel split, exit codes, the deliberate non-archival design, and the `removeIdeaAt` seam shared with `Rm`" +description: "Bulk-remove subcommand (`idea prune`): the TTY × `--force` decision matrix (pipe dry-run vs. interactive `[y/N]` confirm), the leading stderr count header, TTY-aware truncation/color/`--full` listing, stdout/stderr channel split, exit codes, the deliberate non-archival design, and the `removeIdeaAt` seam shared with `Rm`" --- # `idea prune` Subcommand -`idea prune` bulk-removes all done (`[x]`) ideas from the backlog in one pass. The cobra wrapper lives at `src/cmd/idea/prune.go` (`pruneCmd()` factory, modeled on `rmCmd()`); the behavior lives in `Prune` in `src/internal/idea/idea.go`. The split follows Constitution Principle IV — the `RunE` body contains only `resolveFile()` + `idea.Prune` orchestration and output formatting. Added by `260612-drc1-add-prune-subcommand`. +`idea prune` bulk-removes all done (`[x]`) ideas from the backlog in one pass. The cobra wrapper lives at `src/cmd/idea/prune.go` (`pruneCmd()` factory, modeled on `rmCmd()`); the behavior lives in `Prune` in `src/internal/idea/idea.go`. The split follows Constitution Principle IV — the `RunE` body contains only `resolveFile()` + `idea.Prune` orchestration and output formatting. Added by `260612-drc1-add-prune-subcommand`; the TTY-aware count header, interactive confirm, and truncation/color/`--full` rendering were added by `260613-kfcl-tty-aware-output-rendering`. It fills the bulk-removal gap: `rm` is deliberately single-item (`cobra.ExactArgs(1)` + `RequireSingle`'s single-match-or-refuse contract), and `list` merely hides done items by default — neither clears accumulated done lines. -## Dry-run-by-default / `--force` contract +## TTY-aware decision matrix (`--force` × TTY) -The bare invocation is a **free dry run** — it mirrors `rm`'s `--force` safety convention but makes the refusal a useful preview that succeeds. The only local flag is `--force`; the command takes no positional args (`cobra.NoArgs`), inherits the persistent `--file`/`--main` from root (Constitution III), and defines no alias. +The two local flags are `--force` and `--full`; the command takes no positional args (`cobra.NoArgs`), inherits the persistent `--file`/`--main` from root (Constitution III), and defines no alias. Behavior is gated on **stdout being a TTY** and `--force` — the bare invocation is a **free dry run** on a pipe but becomes an **interactive confirm** on a terminal (since `260613-kfcl`), so the un-buryable `[y/N]` prompt is the last line at the cursor instead of a hint that scrolls off-screen. -| Invocation | File state | stdout | stderr | Exit | -|------------|-----------|--------|--------|------| -| `idea prune` (done items exist) | **untouched** | one line per done idea via `FormatLine`, in file order | `Re-run with --force to confirm.` | 0 | -| `idea prune --force` (done items exist) | all `[x]` idea lines removed; survivors canonically rewritten via `SaveFile` | `Pruned N done idea(s).` — count only, no per-line listing | backfill notice when count > 0 | 0 | -| either form (no done items) | untouched — no save | `No done ideas to prune.` | — | 0 | +| stdout TTY? | `--force`? | File state | stdout | stderr | Exit | +|---|---|---|---|---|---| +| any | Yes | all `[x]` lines removed; survivors canonically rewritten via `SaveFile` | `Pruned N done idea(s).` — count only | backfill notice when count > 0 | 0 | +| No | No | **untouched** (dry run) | one line per done idea via `FormatLine`, file order | `N done idea(s) would be pruned` header, then `Re-run with --force to confirm.` trailing hint | 0 | +| Yes | No | removed **iff** the user answers `y`/`yes`; otherwise untouched | the listed (truncated/colored) removable lines, then `Pruned N done idea(s).` on confirm | `N done idea(s) would be pruned` header, the listed lines' channel is stdout, then `Prune N done idea(s)? [y/N] ` prompt; `Aborted — no ideas removed.` on a non-`y` answer | 0 | +| any | (no done items) | untouched — no save | `No done ideas to prune.` | — | 0 | -Per-line listing is **reserved for the dry run**; `--force` output stays quiet for scripting (a user-decided DX trade-off from the intake discussion). After a `--force` run the wrapper calls `printBackfillNotice(cmd, backfilled)` before printing the count, so any `note: stamped today's date on N previously-dateless item(s)` advisory goes to stderr exactly as in `done`/`rm`/`edit`. +**Count header (feature B).** Before listing, `idea prune` (without `--force`) prints `N done idea(s) would be pruned` to **stderr** — the primary signal, printed *before* the list so a human sees the action first regardless of list length. The header carries no call-to-action clause; the action is the interactive prompt (TTY) or the trailing `Re-run with --force to confirm.` hint (non-TTY) — splitting count from action avoids a contradictory "re-run with --force" line right above a `[y/N]` prompt. + +**Interactive confirm (feature E).** On a TTY without `--force`, after the header + list, `confirmPrune(cmd, n)` writes `Prune N done idea(s)? [y/N] ` to stderr and reads one line from `cmd.InOrStdin()`; deletion proceeds only on `y`/`yes` (case-insensitive, `strings.TrimSpace`'d), and **any other input — including bare Enter and EOF — aborts** with `Aborted — no ideas removed.` and no file change. On confirm the deletion runs through the same force path (`idea.Prune(path, true)`), so the file outcome is identical to `--force`. The prompt is **never** shown on a non-TTY (it would hang a pipe) — the non-TTY no-force path falls back to the classic dry run and keeps the trailing `Re-run with --force to confirm.` hint. In TTY mode the prompt **replaces** that trailing hint. + +**TTY-aware listing.** The removable-line listing goes through the shared `printIdeaLines` render path (`cmd/idea/output.go`; see `structure.md` and `list.md`): truncated to width and colored on a TTY (unless `--full`), full canonical `FormatLine` when piped — so the dry run stays pipe-friendly (e.g. `idea prune | wc -l`) and a `prune --force`'s count-only output is unchanged. `--full` (boolean) disables truncation on a TTY, mirroring `list` for symmetry. + +Per-line listing is **reserved for the non-force paths**; `--force` output stays quiet for scripting (a user-decided DX trade-off from the intake discussion). After a `--force` run — and after a confirmed interactive delete — the wrapper calls `printBackfillNotice(cmd, backfilled)` before printing the count, so any `note: stamped today's date on N previously-dateless item(s)` advisory goes to stderr exactly as in `done`/`rm`/`edit`. ## Output channels -stdout carries only the machine-readable result (Constitution VI): the dry run's stdout is exactly the removable lines — pipe-friendly, e.g. `idea prune | wc -l`. The confirm hint is advisory, so it goes to **stderr** via `cmd.ErrOrStderr()`. This is the second deliberate stderr routing in the CLI, after the backfill notice (see `structure.md` § Backfill stderr notice). +stdout carries only the machine-readable result (Constitution VI): the dry run's / pre-confirm's stdout is exactly the removable lines — pipe-friendly, e.g. `idea prune | wc -l`. Everything advisory — the count header, the `[y/N]` prompt, the abort message, the trailing force hint, and the backfill notice — goes to **stderr** via `cmd.ErrOrStderr()`, so `2>/dev/null` suppresses all of it while stdout stays exactly the removable lines. This continues the deliberate advisory-to-stderr channel policy established by the backfill notice (see `structure.md` § Backfill stderr notice). (The count messages `No done ideas to prune.` / `Pruned N done idea(s).` use `fmt.Println`/`fmt.Printf` to the process stdout, matching the original prune wrapper.) ## Exit codes @@ -57,9 +64,11 @@ v1 prunes **all** done items. `prune --before YYYY-MM-DD` (prune only old done i - `src/internal/idea/prune_test.go` — `TestPrune`, table-driven against `t.TempDir()` (Constitution V): mixed-file force removes only `[x]` lines; dry run returns done ideas with the file byte-identical; all-open no-op in both modes (byte-identical proves no save); non-idea lines verbatim through force; dateless surviving open item backfilled on the force save with the count reflected. `TestPrune_MissingFile` pins the error path in both modes. - `src/cmd/idea/main_test.go` — `TestPrune_CLIOutputContract`, subprocess table asserting the exact stdout/stderr split (dry-run listing + stderr hint, force count-only, empty-case message), exit 0 on every path, and resulting backlog content, via the existing `buildBinary`/`setupGitRepo`/`writeRepoBacklog`/`runSplit`/`readRepoBacklog` helpers. +- `src/cmd/idea/main_test.go` — `TestPrune_CountHeaderAndDecisionMatrix` (the `N done idea(s) would be pruned` stderr header text + the non-TTY decision-matrix rows: No/No dry-run fallback with the trailing hint, No/Yes immediate delete, asserting piped stdout stays canonical), `TestConfirmPrune` (the in-process `confirmPrune` y/yes/Y/YES/with-spaces confirm vs. n/no/bare-Enter/EOF/garbage abort), and `TestPrune_ConfirmedDeleteAndAbort` (a `y`/`yes` answer deletes exactly like `--force`; an `n`/EOF answer leaves the backlog byte-identical) — added by `260613-kfcl-tty-aware-output-rendering`. ## Cross-references -- Source-tree placement, root command factory registration, display-semantics table, and the bare-text namespace claim of the `prune` verb: `structure.md`. -- Backlog line format contract the prune rewrite preserves: `../../specs/backlog-format.md`; command table: `../../specs/overview.md`. +- Source-tree placement, root command factory registration, display-semantics table, the `term.go` TTY/width/color/truncation seam, the shared `printIdeaLines` render path, and the bare-text namespace claim of the `prune` verb: `structure.md`. +- The same TTY-aware truncation/`--full`/color rendering as it applies to `idea list`/`ls`: `list.md`. +- Backlog line format contract the prune rewrite preserves: `../../specs/backlog-format.md`; command table: `../../specs/overview.md` (note: the overview still describes the pre-change `prune` dry-run row). - Constitution Principles I (one-file plain-text backlog), III (cobra factories, persistent flags on root), IV (logic in `internal/idea`), VI (machine-parseable stdout): `fab/project/constitution.md`. diff --git a/docs/memory/cli/structure.md b/docs/memory/cli/structure.md index 5ee3505..0a57f55 100644 --- a/docs/memory/cli/structure.md +++ b/docs/memory/cli/structure.md @@ -1,5 +1,5 @@ --- -description: "Source tree layout (cmd/idea + internal/idea), root command factory, command aliases vs. the bare-text shorthand, backlog line lifecycle (lenient read / canonical write incl. the escaped-text convention for multiline ideas and the explicit `idea fmt` canonicalizer with bare-checkbox adoption), help-dump contract, and version stamping" +description: "Source tree layout (cmd/idea + internal/idea), root command factory, command aliases vs. the bare-text shorthand, backlog line lifecycle (lenient read / canonical write incl. the escaped-text convention for multiline ideas and the explicit `idea fmt` canonicalizer with bare-checkbox adoption), the TTY/width/color/truncation seam (internal/idea/term.go) + shared printIdeaLines render path, the golang.org/x/term direct dependency, help-dump contract, and version stamping" --- # CLI Source Structure @@ -16,6 +16,7 @@ src/ idea/ # cobra entry point; one file per subcommand main.go # newRootCmd() factory + main() add.go list.go show.go done.go reopen.go edit.go rm.go prune.go fmt.go resolve.go update.go shell_init.go + output.go # printIdeaLines: the single TTY-aware list/prune render path help_dump.go # hidden "help-dump" subcommand (CLI tree → JSON) main_test.go fmt_test.go shell_init_test.go help_dump_test.go internal/ @@ -26,12 +27,14 @@ src/ editor_test.go fmt.go # idea fmt: Fmt/FmtResult + bare-checkbox adoption fmt_test.go + term.go # TTY/width/color/truncation seam (DisplayListLine) + term_test.go prune_test.go update.go update_test.go ``` -The module path is `github.com/sahil87/idea` (matches the GitHub repo URL). Direct dependencies are limited to `github.com/spf13/cobra` plus the standard library, per the constitution's dependency-discipline principle. +The module path is `github.com/sahil87/idea` (matches the GitHub repo URL). Direct dependencies are `github.com/spf13/cobra` plus `golang.org/x/term` (the **first** non-stdlib/cobra direct dependency, added by `260613-kfcl-tty-aware-output-rendering` for terminal isatty + width detection — stdlib has no width primitive). The constitution's Dependency Discipline principle requires per-change justification rather than a hard cap; `x/term` was justified in that change's intake. It is pinned at `v0.27.0` (with `golang.org/x/sys v0.28.0` indirect) specifically to keep the `go 1.22` directive unchanged — a newer `x/term` would have bumped the directive to 1.25 (Build Reproducibility). ## Why this shape @@ -106,16 +109,18 @@ The helpers are exported from `internal/idea` and built on package-level `string **In-memory real, on-disk escaped.** `ParseLine` applies `UnescapeText` to the captured text group — *after* the Shape B precision guard, which evaluates the raw on-disk text. `Idea.Text` therefore always holds the **real** text (raw newlines, raw backslashes) while the file holds the escaped form. Consequences: `MarshalJSON` needed no change (JSON encodes the newlines itself, so `--json` output carries real newlines in `text`), and `Match`/query semantics operate on the real text the user typed. -**Display semantics per command:** +**Display semantics per command.** `idea list`/`ls` and the `idea prune` dry-run/pre-confirm listing are **TTY-aware** (since `260613-kfcl-tty-aware-output-rendering`): on a terminal they truncate text to the width and color the prefix/checkbox via `DisplayListLine`; when piped or redirected they emit the full canonical `FormatLine` regardless, preserving the line-per-record pipe contract (Constitution VI). Both route through the single `printIdeaLines` helper (see § Shared TTY-aware render path). `--json`, `show`, and all confirmations are unchanged. | Output | Form | |--------|------| -| `idea list` (incl. `--done`, `--all`) | escaped one line per idea via `FormatLine` — the line-per-record guarantee for external pipelines | +| `idea list` / `ls` (incl. `--done`, `--all`, `[id...]`) — **TTY** | one line per idea via `DisplayListLine`: text truncated to width with `…` (unless `--full`), `[id] date:` prefix dimmed, done `[x]` greened (unless `NO_COLOR`) — see `list.md` | +| `idea list` / `ls` — **piped/redirected** | escaped one line per idea via `FormatLine` (no truncation, no ANSI) regardless of `--full` — the line-per-record guarantee for external pipelines | | `idea show` (plain) | `DisplayLine` — real newlines; continuation lines render below the `- [x] [id] date: ` prefix line | -| `list --json` / `show --json` | real newlines in the `text` field (unchanged `MarshalJSON`) | +| `list --json` / `show --json` | real newlines in the `text` field (unchanged `MarshalJSON`); display features never touch JSON | | confirmations — `Added:` (via `idea.EscapeText` in `cmd/idea/add.go`); `Updated:`/`Done:`/`Removed:`/`Reopened:` (via `FormatLine`) | escaped single line — stdout stays machine-parseable (Constitution VI) | -| `idea prune` (dry run) | escaped one line per removable done idea via `FormatLine` on stdout (pipe-friendly, e.g. `idea prune \| wc -l`); the confirm hint `Re-run with --force to confirm.` goes to **stderr** | -| `idea prune --force` confirmation | `Pruned N done idea(s).` — count only, no per-line listing (per-line is reserved for the dry run; see `prune.md`) | +| `idea prune` (dry run / pre-confirm) — **TTY** | removable done ideas via `DisplayListLine` (truncated/colored unless `--full`) on stdout; a `N done idea(s) would be pruned` count header + a `Prune N done idea(s)? [y/N]` confirm prompt on **stderr** — see `prune.md` | +| `idea prune` (dry run) — **piped/redirected** | escaped one line per removable done idea via `FormatLine` on stdout (pipe-friendly, e.g. `idea prune \| wc -l`); the count header + `Re-run with --force to confirm.` hint go to **stderr**; never prompts | +| `idea prune --force` confirmation | `Pruned N done idea(s).` — count only, no per-line listing (per-line is reserved for the non-force paths; see `prune.md`) | **Legacy backslash policy (pre-convention files).** Lines written before the escape convention may contain literal backslashes: @@ -168,6 +173,27 @@ with two guards evaluated on the **whitespace-trimmed** captured text — trimme **Uniqueness blind spot (consistent, accepted).** Adopted-ID uniqueness checks parsed ideas only — a 4-char bracket inside an unparseable line is invisible to it, exactly as it is to `Add`'s `checkIDCollision`. +## TTY/width/color/truncation seam (`internal/idea/term.go`) + +`internal/idea/term.go` (added by `260613-kfcl-tty-aware-output-rendering`) is the single home of all terminal-aware rendering logic — the Constitution IV seam for the display features. `cmd/` only asks it for a decision (is this a TTY? how wide? use color?) and for a ready-to-print display line; `FormatLine`/`DisplayLine` stay the machine/canonical renderers and are intentionally untouched (display-only change). The exported surface: + +| Symbol | Role | +|--------|------| +| `IsTTY(f *os.File) bool` | `term.IsTerminal(int(f.Fd()))`; nil-safe (returns false). The single gate every display change keys on so piped/redirected output stays canonical (Constitution VI). | +| `TermWidth(f *os.File) int` | Resolution order `term.GetSize` → `$COLUMNS` (when it parses to a positive int) → the `defaultTermWidth` const (80). | +| `UseColor(f *os.File) bool` | True only when `f` is a TTY **and** `NO_COLOR` is unset. Uses `os.LookupEnv` presence (not truthiness) per the NO_COLOR spec — any value, including empty, disables color. | +| `DisplayListLine(i Idea, width int, full, color bool) string` | The rune-safe display-line builder (below). Has exactly **one** caller: `printIdeaLines` in `cmd/idea/output.go`. | + +Internal helpers: `truncateText(text string, avail int)` (rune-safe `[]rune` clip with the `ellipsis` U+2026 const, multiline-at-first-`\n`), `dimPrefix`/`greenCheck` (ANSI wrappers). ANSI codes are named consts (`ansiReset`/`ansiFaint`/`ansiGreen`), and 80 is the named `defaultTermWidth` const — no magic numbers/strings (code-quality Anti-Patterns). + +**Width is a parameter, not read inside the builder**, so tests inject it rather than allocating a real PTY (Constitution V). **Color is applied AFTER truncation** so the width math counts visible runes, never escape bytes. The `- [done] [id] date: ` prefix is never truncated; when colored, the checkbox is its own (green-when-done) span between two dim spans so the id/date stay faint. Full truncation/color contract: `list.md`. + +`internal/idea/term_test.go` is table-driven against the seam (Constitution V): `TermWidth` fallback (GetSize-fail via the non-TTY path + `$COLUMNS` set/unset/invalid → 80), `UseColor`/color helpers honoring `NO_COLOR`, and `DisplayListLine` (multibyte rune-safety, prefix-never-truncated, ellipsis presence, multiline-at-first-newline, `full` bypasses truncation, color applied after truncation). + +### Shared TTY-aware render path (`cmd/idea/output.go`) + +`printIdeaLines(out io.Writer, ideas []idea.Idea, full bool)` in `cmd/idea/output.go` is the **single** TTY-aware render path, shared by `list.go` and `prune.go` (extracted during this change's review rework so the render loop is not duplicated — code-quality Anti-Patterns, A-017). It keys the TTY/width/color decision on `os.Stdout` (the real destination) while writing to `out` (normally `os.Stdout` in production, a buffer under test): on a TTY each idea renders via `idea.DisplayListLine(i, width, full, color)`; when piped it emits `idea.FormatLine(i)` regardless of `full`. The Constitution IV split holds — `output.go` only *picks the mode*; all truncation/color logic lives in `term.go`. + ## Command help text (`Short` vs `Long`) Every subcommand sets an enriched cobra `Long` describing what it does, its key flags, the worktree-vs-`--main` resolution (for backlog-touching commands), and a short example. `Short` stays the terse one-liner used by the `Available Commands` sidebar and the `idea -h` root listing — it is a public, byte-stable string; depth goes in `Long` only. The convention was applied repo-wide by `260602-s73u-enrich-command-long-help` (the 8 backlog/update commands; `main.go` / `shell_init.go` already carried `Long`). @@ -245,6 +271,7 @@ This wiring is required because `idea` is released independently: - Release pipeline that consumes this layout (build path, version stamping, Homebrew formula); shll.ai pulls the command reference via `idea help-dump` (the release no longer publishes it): `../release/pipeline.md`. - Self-update subcommand built on top of the Homebrew tap (`update.go` / `internal/idea/update.go`): `update.md`. -- Bulk-remove subcommand (`prune.go` / `idea.Prune`): dry-run/`--force` contract, output channels, and the deliberate non-archival design: `prune.md`. +- `idea list`/`ls` TTY-aware rendering contract — truncation, `--full`, the `[id...]` filter, and color (`list.go` / `internal/idea/term.go`): `list.md`. +- Bulk-remove subcommand (`prune.go` / `idea.Prune`): dry-run/`--force` contract, the count header + interactive `[y/N]` confirm, output channels, and the deliberate non-archival design: `prune.md`. - `edit` subcommand two-form contract and the `$VISUAL`/`$EDITOR`/`vi` temp-file round trip (`edit.go` / `internal/idea/editor.go`): `edit.md`. - Constitution principles III and IV: `fab/project/constitution.md`. diff --git a/fab/changes/260613-kfcl-tty-aware-output-rendering/.history.jsonl b/fab/changes/260613-kfcl-tty-aware-output-rendering/.history.jsonl new file mode 100644 index 0000000..5bd12f4 --- /dev/null +++ b/fab/changes/260613-kfcl-tty-aware-output-rendering/.history.jsonl @@ -0,0 +1,15 @@ +{"action":"enter","driver":"fab-new","event":"stage-transition","stage":"intake","ts":"2026-06-13T07:46:43Z"} +{"args":"DX improvements to idea CLI output rendering: TTY-aware truncation (+--full, +ls id filter), prune count header, color/styling, interactive prune confirm; golang.org/x/term dependency","cmd":"fab-new","event":"command","ts":"2026-06-13T07:46:43Z"} +{"delta":"+0.0","event":"confidence","score":0,"trigger":"calc-score","ts":"2026-06-13T07:48:04Z"} +{"delta":"+0.0","event":"confidence","score":0,"trigger":"calc-score","ts":"2026-06-13T07:48:09Z"} +{"delta":"+0.0","event":"confidence","score":0,"trigger":"calc-score","ts":"2026-06-13T07:48:20Z"} +{"delta":"+3.5","event":"confidence","score":3.5,"trigger":"calc-score","ts":"2026-06-13T07:49:19Z"} +{"delta":"+0.0","event":"confidence","score":3.5,"trigger":"calc-score","ts":"2026-06-13T07:49:27Z"} +{"cmd":"fab-fff","event":"command","ts":"2026-06-13T07:52:16Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"apply","ts":"2026-06-13T07:52:31Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"review","ts":"2026-06-13T08:02:22Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"hydrate","ts":"2026-06-13T08:11:26Z"} +{"event":"review","result":"passed","ts":"2026-06-13T08:11:26Z"} +{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"ship","ts":"2026-06-13T08:16:53Z"} +{"action":"enter","driver":"git-pr","event":"stage-transition","stage":"review-pr","ts":"2026-06-13T08:18:38Z"} +{"event":"review","result":"passed","ts":"2026-06-13T08:26:21Z"} diff --git a/fab/changes/260613-kfcl-tty-aware-output-rendering/.status.yaml b/fab/changes/260613-kfcl-tty-aware-output-rendering/.status.yaml new file mode 100644 index 0000000..2a7fcc2 --- /dev/null +++ b/fab/changes/260613-kfcl-tty-aware-output-rendering/.status.yaml @@ -0,0 +1,51 @@ +id: kfcl +name: 260613-kfcl-tty-aware-output-rendering +created: 2026-06-13T07:46:43Z +created_by: sahil-noon +change_type: feat +issues: [] +progress: + intake: done + apply: done + review: done + hydrate: done + ship: done + review-pr: done +plan: + generated: true + task_count: 10 + acceptance_count: 20 + acceptance_completed: 0 +confidence: + certain: 8 + confident: 5 + tentative: 0 + unresolved: 0 + score: 3.5 + fuzzy: true + dimensions: + signal: 87.8 + reversibility: 83.1 + competence: 87.2 + disambiguation: 83.8 +stage_metrics: + intake: {started_at: "2026-06-13T07:46:43Z", driver: fab-new, iterations: 1, completed_at: "2026-06-13T07:52:31Z"} + apply: {started_at: "2026-06-13T07:52:31Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-13T08:02:22Z"} + review: {started_at: "2026-06-13T08:02:22Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-13T08:11:26Z"} + hydrate: {started_at: "2026-06-13T08:11:26Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-13T08:16:53Z"} + ship: {started_at: "2026-06-13T08:16:53Z", driver: fab-fff, iterations: 1, completed_at: "2026-06-13T08:18:38Z"} + review-pr: {started_at: "2026-06-13T08:18:38Z", driver: git-pr, iterations: 1, completed_at: "2026-06-13T08:26:21Z"} +prs: + - https://github.com/sahil87/idea/pull/20 +true_impact: + added: 0 + deleted: 0 + net: 0 + excluding: + added: 0 + deleted: 0 + net: 0 + computed_at: "2026-06-13T08:16:53Z" + computed_at_stage: hydrate +# true_impact: lazily created on first apply-finish (no placeholder here). +last_updated: 2026-06-13T08:26:21Z diff --git a/fab/changes/260613-kfcl-tty-aware-output-rendering/intake.md b/fab/changes/260613-kfcl-tty-aware-output-rendering/intake.md new file mode 100644 index 0000000..6c6cceb --- /dev/null +++ b/fab/changes/260613-kfcl-tty-aware-output-rendering/intake.md @@ -0,0 +1,150 @@ +# Intake: TTY-Aware Output Rendering + +**Change**: 260613-kfcl-tty-aware-output-rendering +**Created**: 2026-06-13 + +## Origin + + + +> Conversational `/fab-discuss` → `/fab-new`. The trigger was a concrete DX pain point reported by the user: running `idea prune` dumps the full (often multi-hundred-word) text of every done idea, and the critical action prompt — `Re-run with --force to confirm.` — gets buried at the bottom of the terminal, off-screen. The same wall-of-text problem afflicts `idea ls`: short one-line ideas are lost between paragraph-length ones. + +The discussion enumerated a DX backlog (labelled A–G) and triaged it. This change bundles the four features the user explicitly selected to build now — **A** (truncation + `--full` + `ls [id...]` filter), **B** (prune count header), **D** (color), **E** (interactive prune confirm). Two items were explicitly deferred: + +- **G (section grouping via `## H2` headers, read + write side)** → backlogged to the **main** worktree as idea `ykwp` with full hand-off detail. +- **C (paging / `$PAGER`)** → decided against; a `--limit` cap may be revisited later if long lists remain painful after truncation. + +Key decisions reached in conversation: +- The pipe-friendliness contract (Constitution VI; `prune.go:44` comment) is sacred — every display change MUST be TTY-gated so piped output stays full/canonical and machine-parseable. +- `golang.org/x/term` is the accepted dependency for isatty + terminal width (user confirmed "1 agreed"), justified under Constitution Dependency Discipline because stdlib has no isatty/width primitive. + +## Why + + + +1. **Problem**: `idea ls` and `idea prune` (dry-run) print each idea's full untruncated text via `FormatLine`. Ideas in this project are frequently paragraph-length specs (e.g. `a36m`, `72oq` in the live backlog run >300 words each). On a TTY these soft-wrap into 20+ visual rows apiece, so: (a) the `prune` confirm hint scrolls off-screen and is missed; (b) `ls` is unscannable — short ideas drown between long ones; (c) the 4-char `[id]`/`date` prefix the user actually scans for is visually buried. + +2. **Consequence if unfixed**: The reported failure mode persists — users miss the `--force` instruction and assume `prune` did nothing, or cannot find a specific idea in a long `ls`. The tool's whole reason for existing ("reduce friction over hand-editing a markdown file", per Constitution rationale) is undermined when reading the list is itself high-friction. + +3. **Why this approach over alternatives**: + - **Truncation over paging (C)**: paging requires `$PAGER`/`less` TTY-handoff plumbing that is fiddly in Go and overkill for backlogs of dozens (not thousands) of lines. Truncation + a count header + an id filter dissolve most of the need; a `--limit` cap is a cheaper future fallback than a real pager. + - **Interactive confirm (E) over keeping the two-step `--force` dance**: the user's framing keeps `--force` working for scripts while making the interactive prompt the *last* line at the cursor — inherently un-buryable, which is the most direct fix for the reported pain. The original objection (breaking `--force` muscle memory) is resolved because `--force` still skips the prompt. + - **`golang.org/x/term` over stdlib-only TTY detection**: stdlib can detect a char device (`os.Stdout.Stat()`) but has no terminal-width primitive; `$COLUMNS`/`tput cols` are hacky and not always available. `x/term` is the idiomatic quasi-official Go package and gives both isatty and width cleanly. + +## What Changes + + + +### A. TTY-aware truncation, `--full` flag, and `ls [id...]` filter + +**Truncation** (applies to `idea ls` and `idea prune` dry-run output): +- When stdout is a **TTY** and `--full` is not set, truncate each idea's **text portion** so the rendered line fits the terminal width, appending a single-character ellipsis `…` (U+2026) when truncated. +- The `[id] date:` prefix is the scannable anchor and MUST NEVER be truncated — only the trailing text is clipped. +- Multiline ideas (text containing escaped `\n`): truncate at the first newline regardless of width (so display is always one physical row), with the `…` indicating more follows. +- Truncation MUST be **rune-safe** — never cut mid-rune (use `[]rune` slicing or `utf8`-aware logic, not byte slicing). Width-awareness for wide glyphs (CJK/emoji) is a non-goal; rune-count fitting against terminal columns is the floor. +- When stdout is **NOT a TTY** (piped/redirected), emit full canonical text regardless of `--full` — preserves the pipe-friendliness contract. `--full` is only meaningful on a TTY. + +**`--full` flag**: add to `idea ls` (and it implicitly disables truncation in `prune` dry-run if applicable — see Open Questions). Disables truncation, emits full text on a TTY. + +**`ls [id...]` positional filter**: `idea list`/`idea ls` gains an optional variadic positional argument of idea IDs. When provided, only ideas whose ID is in the set are listed (still list-formatted, still respects `--sort`/`--reverse`/truncation/color). Invalid/unknown IDs: see Open Questions. `idea show ` remains the full-detail single-idea command; `ls --full` overlapping with `show` is acceptable mild redundancy, not a conflict. + +Example (TTY, 80-col terminal): +``` +$ idea ls +- [ ] [oibl] 2026-03-14: unable to scroll terminal on mobile +- [ ] [rkx4] 2026-03-20: The text input dialog - show buttons at the bottom of t… +$ idea ls --full +- [ ] [rkx4] 2026-03-20: The text input dialog - show buttons at the bottom of that dialog in a section that represent 'most used commands' ...(full text)... +$ idea ls oibl rkx4 # filter to two ideas +``` + +### B. `idea prune` dry-run count summary header (stderr) + +Before listing the prunable ideas, print a one-line summary to **stderr**: +``` +2 done idea(s) would be pruned — re-run with --force to confirm. +``` +- Goes to **stderr** (like the existing hint and the backfill notice), so `2>/dev/null` suppresses it consistently and **stdout still carries exactly the removable lines** (pipe-friendly, Constitution VI). +- Printed **before** the list so the action is the first thing a human sees, regardless of list length. +- Replaces / subsumes the trailing `Re-run with --force to confirm.` line, OR complements it — the header is the primary signal. (The trailing hint may be kept or dropped; leading header is the fix.) + +### D. Color / styling (TTY-gated, `NO_COLOR`-aware) + +When stdout is a **TTY** AND the `NO_COLOR` env var is **unset**: +- Dim (ANSI faint / `\033[2m`) the `[id] date:` prefix so the eye lands on the text. +- Color the done checkbox `[x]` green. +- Open `[ ]` checkboxes and the idea text render in the default color. + +When stdout is **NOT a TTY** OR `NO_COLOR` is set: emit plain, uncolored output (current behavior). Applies to `idea ls` and `idea prune` dry-run. Color and truncation are independent but share the same TTY gate. + +### E. Interactive confirm for `idea prune` + +Decision matrix for `idea prune`: + +| stdout TTY? | `--force`? | Behavior | +|---|---|---| +| Yes | No | List prunable ideas (with B's header + A's truncation + D's color), then prompt `Prune N done idea(s)? [y/N] ` on stderr; read a line from stdin; delete only on `y`/`yes` (case-insensitive); anything else aborts with no change. | +| Yes | Yes | Skip the prompt entirely; delete immediately (current `--force` behavior). | +| No | No | Do NOT prompt (would hang a pipe). Fall back to today's dry-run: list removable lines on stdout + stderr hint. | +| No | Yes | Delete immediately (current behavior). | + +The prompt is the last line written, at the cursor — inherently visible. `--force` preserves script behavior unchanged. + +### Shared internal helper + +A small new internal seam (e.g. in `internal/idea`, or a tiny `internal/idea/term.go`) provides: `IsTTY(f *os.File) bool`, `TermWidth(f *os.File) int` (fallback width when undetectable — see Open Questions), and styling helpers (dim/green honoring `NO_COLOR`). `golang.org/x/term` is the single new direct dependency. Per Constitution IV, all logic lives in `internal/idea`; `cmd/` only wires flags and chooses TTY vs. plain rendering by asking the helper. + +## Affected Memory + + + +- `cli/list`: (modify) `idea ls` gains TTY-aware truncation, `--full` flag, optional `[id...]` positional filter, and color output. +- `cli/prune`: (modify) `idea prune` dry-run gains a leading stderr count header and TTY-gated interactive `[y/N]` confirm; non-TTY/`--force` paths unchanged. +- `cli/index` (or the relevant cli sub-domain index): (modify) note the new `golang.org/x/term` direct dependency and the TTY/width/color rendering seam. + + + +## Impact + + + +- **`src/cmd/idea/list.go`**: add `--full` flag and `[id...]` positional arg (`Args` changes from none to variadic id validation); branch rendering between TTY (truncate+color) and plain. +- **`src/cmd/idea/prune.go`**: add leading stderr count header; add TTY-gated `[y/N]` prompt; keep `--force` and non-TTY paths intact; preserve stdout = removable lines. +- **`src/internal/idea/idea.go`**: `FormatLine`/`DisplayLine` unchanged (machine/canonical); add a new display/truncation/styling helper consumed by the cmd layer. +- **New**: small `internal/idea` TTY/width/color seam wrapping `golang.org/x/term`. +- **Dependencies**: `go.mod`/`go.sum` gain `golang.org/x/term` (first non-stdlib/cobra direct dep) — justified per Constitution Dependency Discipline. +- **Tests**: table-driven tests for truncation (rune-safety, multiline-at-first-newline, prefix-never-truncated, ellipsis), id filter, count-header text, and the prune decision matrix. TTY-dependent behavior is tested by injecting the TTY/width decision (the helper seam) rather than allocating a real PTY — keeps tests `t.TempDir()`-style and deterministic per Constitution V. +- **No change** to: the backlog file format, the parser, `FormatLine` output, the `--json` schema (Constitution VI untouched — display-only change). + +## Open Questions + + + +- **Fallback terminal width** when `x/term.GetSize` fails (e.g. width 0 / not a real terminal but isatty true): default to 80 columns? Honor `$COLUMNS` first? +- **Unknown IDs in `ls [id...]`**: error out, or silently skip with a stderr warning, or print nothing? (Lean: warn-and-skip on stderr, list the rest — consistent with the pipe-friendly stdout posture.) +- **Does `--full` belong on `prune`** too, or only `ls`? The `prune` dry-run reuses the same rendering; a `--full` on prune would let users see full done-idea text before confirming. (Lean: add to both for symmetry, cheap.) +- **Truncation reserve for color codes**: ANSI codes are zero-width but counted as bytes — ensure width math counts *visible* runes, not escape sequences (apply color AFTER truncation, or measure pre-color). +- **Keep or drop the trailing `Re-run with --force to confirm.`** line once the leading header (B) exists? (Lean: drop it in TTY mode where the interactive prompt replaces it; keep it in non-TTY no-force fallback.) + +## Assumptions + + + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Certain | Use `golang.org/x/term` for isatty + terminal width; first non-stdlib/cobra direct dep, justified per Dependency Discipline | User explicitly confirmed ("1 agreed"); stdlib has no width primitive; quasi-official package | S:95 R:80 A:90 D:90 | +| 2 | Certain | All display changes (truncate, color, confirm) are TTY-gated; piped output stays full/canonical to preserve Constitution VI pipe contract | Discussed and agreed as the recurring guardrail; constitution-mandated | S:95 R:75 A:95 D:90 | +| 3 | Certain | Logic lives in a new `internal/idea` TTY/width/color seam; `cmd/` only wires flags and picks rendering mode | Constitution IV mandates logic in internal/idea; matches existing structure | S:90 R:80 A:95 D:85 | +| 4 | Confident | Truncate only the text portion, never the `[id] date:` prefix; truncate multiline at first `\n`; rune-safe | Discussed explicitly; prefix is the scannable anchor, rune-safety is the correctness floor — minor latitude in ellipsis char/exact column budget | S:90 R:80 A:88 D:82 | +| 5 | Certain | B's count summary goes to stderr, printed before the list; stdout stays exactly the removable lines | Explicitly agreed in discussion + mandated by Constitution VI pipe contract; no alternative | S:92 R:85 A:92 D:88 | +| 6 | Certain | E: prompt only when TTY && !force; never prompt on non-TTY (would hang a pipe); --force always skips prompt | User authored this exact decision matrix in conversation; nothing left to decide | S:95 R:80 A:90 D:90 | +| 7 | Confident | D: dim `[id] date:` prefix, green `[x]`; gate on TTY && unset NO_COLOR | Discussed; NO_COLOR is the standard opt-out — exact ANSI shades have minor latitude | S:88 R:85 A:88 D:80 | +| 8 | Certain | Add `--full` to `ls`; add optional `[id...]` positional filter to `ls`; `show` stays the single-idea full-detail command | User explicitly requested both in conversation; `show` already exists so roles are fixed | S:92 R:80 A:88 D:85 | +| 9 | Certain | Skip paging/`$PAGER` (item C); a `--limit` cap may come later if needed | User explicitly decided against paging in conversation | S:95 R:90 A:88 D:90 | +| 10 | Certain | Section grouping (item G) is out of scope — deferred to main backlog `ykwp` (read + write side) | User explicitly instructed to backlog G separately and scope this change to A/B/D/E | S:95 R:90 A:90 D:92 | +| 11 | Confident | Unknown IDs in `ls [id...]` → warn on stderr and list the rest (rather than hard error) | Clear front-runner: consistent with the pipe-friendly stdout posture; trivially reversible | S:70 R:85 A:75 D:70 | +| 12 | Confident | Fallback terminal width = 80 cols (honoring `$COLUMNS` if set) when width is undetectable | 80 cols is the universal terminal default and `$COLUMNS`-first is the standard convention | S:75 R:85 A:80 D:75 | +| 13 | Confident | Add `--full` to `prune` dry-run too (symmetry with `ls`), and drop the trailing force-hint in TTY mode where the prompt replaces it | Cheap symmetric choice with an obvious front-runner; trivially reversible | S:70 R:85 A:75 D:72 | + +13 assumptions (8 certain, 5 confident, 0 tentative, 0 unresolved). diff --git a/fab/changes/260613-kfcl-tty-aware-output-rendering/plan.md b/fab/changes/260613-kfcl-tty-aware-output-rendering/plan.md new file mode 100644 index 0000000..9a44ea7 --- /dev/null +++ b/fab/changes/260613-kfcl-tty-aware-output-rendering/plan.md @@ -0,0 +1,196 @@ +# Plan: TTY-Aware Output Rendering + +**Change**: 260613-kfcl-tty-aware-output-rendering +**Intake**: `intake.md` + +## Requirements + + + +### Terminal Seam: TTY / Width / Color Detection + +#### R1: TTY detection +The package SHALL expose `IsTTY(f *os.File) bool` returning whether the given file is a terminal, via `golang.org/x/term.IsTerminal`. + +- **GIVEN** stdout connected to an interactive terminal +- **WHEN** `IsTTY(os.Stdout)` is called +- **THEN** it returns true +- **AND** when stdout is a pipe or regular file it returns false + +#### R2: Terminal width resolution with fallback +The package SHALL expose `TermWidth(f *os.File) int` returning the terminal column count: `golang.org/x/term.GetSize` first; if that fails or yields a non-positive width, honor `$COLUMNS` when it parses to a positive integer; otherwise fall back to 80. + +- **GIVEN** `GetSize` cannot determine width (returns error or width ≤ 0) +- **WHEN** `TermWidth` is called and `$COLUMNS` is set to a positive integer +- **THEN** the `$COLUMNS` value is returned +- **AND** when `$COLUMNS` is unset or invalid, 80 is returned + +#### R3: Color helpers honoring NO_COLOR +The package SHALL expose styling helpers that dim the `[id] date:` prefix (ANSI faint `\033[2m`) and color a done `[x]` checkbox green, and a predicate `UseColor(f *os.File) bool` that is true only when `f` is a TTY AND `NO_COLOR` is unset. Color codes SHALL be applied only after width-based truncation so width math counts visible runes, never escape bytes. + +- **GIVEN** stdout is a TTY and `NO_COLOR` is unset +- **WHEN** `UseColor(os.Stdout)` is consulted +- **THEN** it returns true and styling helpers wrap text in ANSI codes +- **AND** when `NO_COLOR` is set (to any value, including empty) or stdout is not a TTY, `UseColor` returns false and helpers return their input unchanged + +### List Rendering: Truncation, --full, and Display + +#### R4: Rune-safe text truncation that never clips the prefix +The package SHALL expose a display-line builder that, given an idea and a target width, renders the canonical escaped line but truncates only the **text portion** to fit the width, appending `…` (U+2026) when clipped. The `- [x] [id] date: ` prefix SHALL NEVER be truncated. Truncation SHALL be rune-safe (operate on `[]rune`, never byte slices). When the escaped text contains a literal `\n` escape sequence (multiline idea), the text SHALL be truncated at the first such newline regardless of width, with `…` appended. + +- **GIVEN** an idea whose escaped text exceeds the available width +- **WHEN** the display line is built at that width +- **THEN** the prefix is intact and the text is clipped to fit with a trailing `…` +- **AND** truncation falls on a rune boundary (no broken multibyte sequence) +- **AND** a multiline idea (escaped text containing `\n`) is clipped at the first `\n` with `…` even if it would otherwise fit + +#### R5: `--full` flag and TTY gate for `idea list`/`ls` +`idea list` (and its `ls` alias) SHALL accept a `--full` boolean flag. On a TTY with `--full` unset, each listed idea SHALL be truncated per R4 and colored per R3. With `--full` set on a TTY, full text SHALL be emitted (colored per R3, no truncation). When stdout is NOT a TTY, full canonical `FormatLine` output SHALL be emitted regardless of `--full` (no truncation, no color), preserving the pipe contract. `--json` output SHALL be unchanged in all cases. + +- **GIVEN** stdout is a TTY and the list contains an over-wide idea +- **WHEN** `idea ls` runs without `--full` +- **THEN** the idea's text is truncated with `…` and the prefix is dimmed +- **AND** with `--full` the full text is shown (no `…`) +- **AND** when stdout is piped, output equals the current `FormatLine` output (no truncation, no ANSI) for both `--full` and default + +#### R6: Optional `[id...]` positional filter for `idea list`/`ls` +`idea list`/`ls` SHALL accept zero-or-more positional ID arguments. Each argument MUST be a 4-char lowercase alphanumeric ID (validated via `idea.ValidateID`); a malformed argument is a usage error. When IDs are supplied, only ideas whose ID is in the set are listed, still respecting `--sort`/`--reverse`/truncation/color and the active filter. Unknown (well-formed but absent) IDs SHALL produce a stderr warning naming each missing ID, and the remaining matched ideas SHALL still be listed. When all supplied IDs are unknown, the warning is emitted and the normal empty-result message path applies. + +- **GIVEN** a backlog containing ideas `oibl` and `rkx4` +- **WHEN** `idea ls oibl rkx4` runs +- **THEN** only those two ideas are listed (filter/sort/reverse still applied) +- **AND** `idea ls oibl zzzz` warns on stderr that `zzzz` was not found and lists `oibl` +- **AND** `idea ls BADID` (malformed) fails with a usage/validation error + +### Prune Rendering: Count Header, Color, Interactive Confirm + +#### R7: Dry-run count summary header on stderr +`idea prune` without `--force` SHALL, before listing prunable ideas, print to **stderr**: `N done idea(s) would be pruned`. stdout SHALL still carry exactly the removable lines (truncated/colored on a TTY per R4/R3; full canonical `FormatLine` when piped). The header is printed before the list so it is the first thing a human sees. + +- **GIVEN** two done ideas exist and stdout is a pipe +- **WHEN** `idea prune` runs +- **THEN** stderr begins with `2 done idea(s) would be pruned` and stdout carries exactly the two `FormatLine` lines + +#### R8: Interactive confirm gated on TTY && !force +`idea prune` SHALL prompt for confirmation only when stdout is a TTY AND `--force` is unset. The prompt `Prune N done idea(s)? [y/N] ` SHALL be written to stderr after the count header and list; a line is read from stdin and deletion proceeds only on `y`/`yes` (case-insensitive, trimmed); any other input aborts with no change and an abort message on stderr. When stdout is a TTY in interactive mode, the trailing `Re-run with --force to confirm.` hint SHALL be dropped (the prompt replaces it). When stdout is NOT a TTY and `--force` is unset, the command SHALL NOT prompt and SHALL fall back to the dry run (removable lines on stdout + the trailing `Re-run with --force to confirm.` hint on stderr, in addition to the R7 header). `--force` (TTY or not) SHALL delete immediately with the existing count-only output. + +- **GIVEN** stdout is a TTY, `--force` unset, two done ideas +- **WHEN** the prompt fires and the user enters `y` +- **THEN** the two done ideas are removed (same as `--force`) and `Pruned 2 done idea(s).` prints +- **AND** any non-`y`/`yes` answer aborts with no file change and an abort message +- **AND** when stdout is piped and `--force` unset, no prompt is shown and the trailing hint is emitted (current dry-run fallback) + +#### R9: `--full` on `idea prune` for symmetry +`idea prune` SHALL accept a `--full` boolean flag with the same TTY-gated semantics as R5 applied to its dry-run / pre-confirm listing. + +- **GIVEN** stdout is a TTY with an over-wide done idea +- **WHEN** `idea prune --full` runs +- **THEN** the listed done idea shows full text (no `…`); piped output is unaffected by `--full` + +### Non-Goals + +- Section grouping via `## H2` headers (item G) — deferred to backlog `ykwp`. +- Paging / `$PAGER` (item C) — decided against. +- Wide-glyph (CJK/emoji) display-width awareness — rune-count against columns is the floor. +- Changing `FormatLine`/`DisplayLine`, the parser, the backlog format, or the `--json` schema. + +### Design Decisions + +1. **New `internal/idea/term.go` seam**: TTY/width/color/truncation logic lives in `internal/idea`; `cmd/` only asks the seam for the decision. — *Why*: Constitution IV. — *Rejected*: inline logic in `cmd/` (untestable without a PTY, violates IV). +2. **Width injectable for tests**: the display-line builder takes width as a parameter (no real PTY in tests). — *Why*: Constitution V (deterministic, real-temp-dir tests). — *Rejected*: allocating a PTY (flaky, platform-dependent). +3. **Color applied after truncation**: truncate plain text, then wrap with ANSI. — *Why*: width math must count visible runes, not escape bytes (intake open question). +4. **`golang.org/x/term`**: first non-stdlib/cobra direct dep. — *Why*: stdlib has no width primitive; recorded in intake, Constitution Dependency Discipline. — *Rejected*: `os.Stat` char-device check + `$COLUMNS`/`tput` hacks (no clean width). + +## Tasks + +### Phase 1: Setup + +- [x] T001 Add `golang.org/x/term` as a direct dependency: `cd src && go get golang.org/x/term`, then `go mod tidy` (after T002 imports it so tidy keeps it); pin the `go` directive at 1.22 (no incidental toolchain bump) + +### Phase 2: Core Implementation (internal/idea seam) + +- [x] T002 Create `src/internal/idea/term.go` with `IsTTY(f *os.File) bool` (wraps `term.IsTerminal`) and `TermWidth(f *os.File) int` (`term.GetSize` → `$COLUMNS` → 80) +- [x] T003 Add color helpers to `src/internal/idea/term.go`: `UseColor(f *os.File) bool` (TTY && NO_COLOR unset), `dimPrefix(s string) string` (ANSI faint), `greenCheck(s string) string`; package-level ANSI consts +- [x] T004 Add the rune-safe display-line builder to `src/internal/idea/term.go`: `DisplayListLine(i Idea, width int, full, color bool) string` — builds the escaped canonical line, truncates only the text portion at the first `\n` escape or to width with `…`, never clips the prefix, applies color after truncation + +### Phase 3: Integration & Edge Cases (cmd wiring) + +- [x] T005 Wire `src/cmd/idea/list.go`: add `--full` flag; change `Args` to a validator allowing zero-or-more 4-char IDs; filter ideas by the supplied ID set with a stderr warning for unknown IDs; render each idea via `idea.DisplayListLine` using `IsTTY`/`TermWidth`/`UseColor(os.Stdout)`; `--json` and non-TTY paths unchanged +- [x] T006 Wire `src/cmd/idea/prune.go`: add `--full` flag; print the R7 stderr count header before the listing; render dry-run/pre-confirm lines via `idea.DisplayListLine`; implement the R8 TTY&&!force interactive `[y/N]` prompt (read from `cmd.InOrStdin()`, prompt on stderr) calling `idea.Prune(path, true)` on confirm; drop the trailing hint in TTY mode, keep it in the non-TTY no-force fallback; `--force` path unchanged except header + +### Phase 4: Tests + +- [x] T007 [P] Add `src/internal/idea/term_test.go`: table-driven tests for `TermWidth` fallback (GetSize-fail injected via the non-TTY path + `$COLUMNS` set/unset/invalid), `UseColor`/color helpers honoring `NO_COLOR`, and `DisplayListLine` (multibyte rune-safety, prefix-never-truncated, ellipsis presence, multiline-at-first-newline, `full` bypasses truncation, color applied after truncation) +- [x] T008 [P] Extend `src/cmd/idea/main_test.go`: `ls [id...]` filter incl. unknown-id stderr warning and malformed-id error; prune count-header text and the non-TTY decision-matrix rows (No/No fallback hint, No/Yes immediate delete); assert piped output stays canonical (no ANSI, no truncation) + +### Phase 5: Review Rework + +- [x] T009 Extract the duplicated TTY-aware render loop (`list.go` RunE branch + `prune.go` `printPruneLines`) into one shared `cmd/idea/output.go` helper `printIdeaLines(out io.Writer, ideas []idea.Idea, full bool)`; call it from both sites; remove `printPruneLines` (review should-fix #1) +- [x] T010 Add in-process tests for the interactive confirm (review should-fix #2): `TestConfirmPrune` (y/yes/Y/YES/spaces confirm; n/no/bare-enter/EOF/garbage abort) and `TestPrune_ConfirmedDeleteAndAbort` (y/yes delete like `--force`; n/EOF leave the backlog byte-identical), in `src/cmd/idea/main_test.go` + +## Execution Order + +- T002 blocks T003, T004 (same file, shared package decls) +- T002–T004 block T005, T006 (cmd wiring consumes the seam) +- T007, T008 follow their respective implementation tasks; [P] relative to each other +- T009, T010 are review-rework follow-ups (post-implementation) + +## Acceptance + +### Functional Completeness + +- [ ] A-001 R1: `IsTTY` correctly reports terminal vs. non-terminal for a given `*os.File` +- [ ] A-002 R2: `TermWidth` returns `GetSize` width when available, else `$COLUMNS`, else 80 +- [ ] A-003 R3: color helpers dim the prefix and green the `[x]`; `UseColor` is true only on a TTY with `NO_COLOR` unset +- [ ] A-004 R4: the display-line builder truncates only the text portion rune-safely with `…`, never the prefix +- [ ] A-005 R5: `idea ls --full` shows full text on a TTY; default truncates; piped output is unchanged canonical +- [ ] A-006 R6: `idea ls oibl rkx4` lists only those ideas; unknown IDs warn on stderr; malformed IDs error +- [ ] A-007 R7: `idea prune` dry run prints the `N done idea(s) would be pruned` header on stderr with removable lines on stdout +- [ ] A-008 R8: prune prompts only when TTY && !force; non-TTY no-force falls back to dry run with the trailing hint; `--force` deletes immediately. Confirm logic and confirmed-delete are test-backed by `TestConfirmPrune` + `TestPrune_ConfirmedDeleteAndAbort` +- [ ] A-009 R9: `idea prune --full` shows full done-idea text on a TTY; piped output unaffected + +### Behavioral Correctness + +- [ ] A-010 R5: a piped `idea ls` / `idea ls --full` emits byte-identical output to today's `FormatLine` listing (no ANSI, no `…`) +- [ ] A-011 R8: an aborted interactive prune (`n`/EOF answer) leaves the backlog file byte-identical, proven by `TestPrune_ConfirmedDeleteAndAbort` + +### Scenario Coverage + +- [ ] A-012 R4: multiline idea (escaped `\n` in text) renders as one physical row truncated at the first newline with `…` +- [ ] A-013 R6: an unknown-ID warning is exercised by a test asserting the stderr message and the listed survivors + +### Edge Cases & Error Handling + +- [ ] A-014 R4: truncation on multibyte (non-ASCII) text falls on a rune boundary (no broken UTF-8), covered by a test +- [ ] A-015 R2: `TermWidth` handles `$COLUMNS` unset and invalid (`$COLUMNS=abc`) by returning 80 + +### Code Quality + +- [ ] A-016 Pattern consistency: new code follows the naming, error-handling, and cobra-factory patterns of surrounding `cmd/idea` and `internal/idea` files +- [ ] A-017 No unnecessary duplication: reuses `FormatLine`/`EscapeText`/`ValidateID` and the existing `printBackfillNotice`/`resolveFile` helpers rather than reimplementing them; the TTY-aware render loop is a single shared `printIdeaLines` helper (`cmd/idea/output.go`) called by both `list` and `prune` (no duplicated render path) +- [ ] A-018 Readability over cleverness: the truncation helper is straightforward `[]rune` slicing, not byte arithmetic (code-quality.md Principles) +- [ ] A-019 No magic strings/numbers: the 80-col fallback and ANSI codes are named constants (code-quality.md Anti-Patterns) +- [ ] A-020 Logic placement: all TTY/width/color/truncation logic lives in `internal/idea`; `cmd/` only wires flags and picks the rendering mode (Constitution IV) + +## Notes + +- Check items as you review: `- [x]` +- All acceptance items must pass before `/fab-continue` (hydrate) + +## Assumptions + +| # | Grade | Decision | Rationale | Scores | +|---|-------|----------|-----------|--------| +| 1 | Certain | New `internal/idea/term.go` holds TTY/width/color/truncation; `cmd/` only wires flags | Constitution IV mandate; intake Design decision | S:92 R:80 A:95 D:88 | +| 2 | Certain | Width is a parameter to the display-line builder so tests inject it (no real PTY) | Constitution V; intake explicitly requires injectable seam | S:90 R:82 A:95 D:90 | +| 3 | Certain | `TermWidth` order: `GetSize` → `$COLUMNS` → 80 | Intake assumption #12, explicit | S:85 R:85 A:90 D:85 | +| 4 | Certain | Color applied after truncation so width counts visible runes | Intake open-question resolution, explicit | S:88 R:80 A:92 D:88 | +| 5 | Confident | Count header text: `N done idea(s) would be pruned` (drop the trailing `— re-run...` clause from the header since the interactive prompt / fallback hint carries the call-to-action) | Intake shows `N done idea(s) would be pruned — re-run with --force to confirm.` but assumption #13 drops the force-hint in TTY mode where the prompt fires; splitting the header (count) from the action (prompt or trailing hint) avoids a contradictory "re-run with --force" line right above a `[y/N]` prompt. Reversible wording choice. | S:70 R:88 A:78 D:72 | +| 6 | Confident | Malformed `ls` ID args (not `[a-z0-9]{4}`) are a usage error via `ValidateID`; only well-formed-but-absent IDs get the warn-and-list-rest treatment | Intake assumption #11 covers "unknown IDs" (warn+skip); malformed vs. unknown is a natural split — a typo'd long string is a usage mistake, a 4-char absent ID is "not found". Reversible. | S:68 R:85 A:78 D:70 | +| 7 | Confident | Add `--full` to both `ls` and `prune` | Intake assumption #13 (symmetry, cheap) | S:72 R:85 A:80 D:75 | +| 8 | Confident | `NO_COLOR` set to ANY value (incl. empty string) disables color, per the NO_COLOR spec (presence, not truthiness) | Standard NO_COLOR convention; intake says "unset" gate. Reversible. | S:78 R:85 A:82 D:80 | + +8 assumptions (4 certain, 4 confident, 0 tentative). diff --git a/src/cmd/idea/list.go b/src/cmd/idea/list.go index 40059b4..b660e37 100644 --- a/src/cmd/idea/list.go +++ b/src/cmd/idea/list.go @@ -10,25 +10,40 @@ import ( ) func listCmd() *cobra.Command { - var all, done, jsonOut, reverse bool + var all, done, jsonOut, reverse, full bool var sortField string cmd := &cobra.Command{ - Use: "list", + Use: "list [id...]", Aliases: []string{"ls"}, Short: "List ideas from the backlog", Long: `List ideas from the current worktree's backlog (fab/backlog.md). Open ideas are shown by default. Use --all/-a to include done ideas, or --done -to show only completed ones. --json emits the structured records (id, date, -status, text) for piping into other tools. --sort accepts "date" (default) or -"id", and --reverse flips the order. As with every backlog command, --main -targets the main worktree's backlog and --file / IDEAS_FILE point elsewhere -(see "idea --help"). +to show only completed ones. Pass one or more 4-char IDs to list only those +ideas. --json emits the structured records (id, date, status, text) for piping +into other tools. --sort accepts "date" (default) or "id", and --reverse flips +the order. + +On a terminal, long idea text is truncated to fit the width (the [id] date: +prefix is never clipped) and the prefix is dimmed; --full shows the complete +text. When the output is piped or redirected, full canonical lines are emitted +regardless of --full so downstream tools see machine-parseable records. As with +every backlog command, --main targets the main worktree's backlog and +--file / IDEAS_FILE point elsewhere (see "idea --help"). idea list idea list --all --sort id + idea ls a7k2 b3c9 --full idea list --json`, + Args: func(cmd *cobra.Command, args []string) error { + for _, a := range args { + if err := idea.ValidateID(a); err != nil { + return err + } + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { path, err := resolveFile() if err != nil { @@ -56,6 +71,13 @@ targets the main worktree's backlog and --file / IDEAS_FILE point elsewhere return err } + // Optional [id...] positional filter: keep only ideas whose ID was + // requested, warn on stderr for any well-formed-but-absent ID, and + // still list the rest (pipe-friendly posture — Constitution VI). + if len(args) > 0 { + ideas = filterByIDs(cmd, ideas, args) + } + if len(ideas) == 0 { if jsonOut { fmt.Println("[]") @@ -71,9 +93,10 @@ targets the main worktree's backlog and --file / IDEAS_FILE point elsewhere return enc.Encode(ideas) } - for _, i := range ideas { - fmt.Println(idea.FormatLine(i)) - } + // Display-only rendering is TTY-gated (see printIdeaLines): on a + // terminal truncate to width (unless --full) and color; when piped + // emit full canonical FormatLine output so the pipe contract holds. + printIdeaLines(cmd.OutOrStdout(), ideas, full) return nil }, } @@ -83,6 +106,35 @@ targets the main worktree's backlog and --file / IDEAS_FILE point elsewhere cmd.Flags().BoolVar(&jsonOut, "json", false, "Output as JSON") cmd.Flags().StringVar(&sortField, "sort", "date", "Sort by field (id or date)") cmd.Flags().BoolVar(&reverse, "reverse", false, "Reverse sort order") + cmd.Flags().BoolVar(&full, "full", false, "Show full idea text on a terminal (no truncation)") return cmd } + +// filterByIDs returns the subset of ideas whose ID is in wantIDs, preserving +// the input order. Any requested ID with no match is reported once on stderr +// (warn-and-list-the-rest), consistent with the pipe-friendly stdout posture. +func filterByIDs(cmd *cobra.Command, ideas []idea.Idea, wantIDs []string) []idea.Idea { + want := make(map[string]bool, len(wantIDs)) + for _, id := range wantIDs { + want[id] = true + } + + var result []idea.Idea + found := make(map[string]bool, len(wantIDs)) + for _, i := range ideas { + if want[i.ID] { + result = append(result, i) + found[i.ID] = true + } + } + + warned := make(map[string]bool, len(wantIDs)) + for _, id := range wantIDs { + if !found[id] && !warned[id] { + fmt.Fprintf(cmd.ErrOrStderr(), "warning: no idea with ID %q\n", id) + warned[id] = true + } + } + return result +} diff --git a/src/cmd/idea/main_test.go b/src/cmd/idea/main_test.go index 37bfe82..c4bd895 100644 --- a/src/cmd/idea/main_test.go +++ b/src/cmd/idea/main_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/sahil87/idea/internal/idea" ) // buildBinary builds the idea binary to a temp location for integration tests. @@ -405,11 +407,13 @@ func TestPrune_CLIOutputContract(t *testing.T) { wantBacklog string // "" means unchanged }{ { - name: "dry run lists done ideas on stdout and hints on stderr", + name: "dry run lists done ideas on stdout, count header + hint on stderr", backlog: mixed, args: []string{"prune"}, wantStdout: "- [x] [d0ne] 2026-06-02: prune me\n- [x] [d1ne] 2026-06-03: prune me too\n", - wantStderr: "Re-run with --force to confirm.\n", + // Non-TTY (piped) path: the leading count header (feature B) plus + // the classic trailing fallback hint, in that order. + wantStderr: "2 done idea(s) would be pruned\nRe-run with --force to confirm.\n", }, { name: "force prints count only and removes the done lines", @@ -710,3 +714,286 @@ func TestEdit_MultilineSingleLineConfirmation(t *testing.T) { t.Errorf("edit confirmation:\ngot: %q\nwant: %q", stdout, want) } } + +// TestList_IDFilter covers the optional [id...] positional filter: listing only +// the requested IDs, the warn-and-list-the-rest behavior for well-formed-but- +// absent IDs, and the usage error for a malformed ID. The subprocess pipes its +// output, so list emits canonical FormatLine lines (the non-TTY path) — which +// also pins the pipe contract (no ANSI, no truncation). +func TestList_IDFilter(t *testing.T) { + bin := buildBinary(t) + + const seed = "# Backlog\n\n" + + "- [ ] [ab12] 2026-06-01: first idea\n" + + "- [ ] [cd34] 2026-06-02: second idea\n" + + "- [ ] [ef56] 2026-06-03: third idea\n" + + tests := []struct { + name string + args []string + wantStdout string + wantStderr []string // required stderr substrings + noStdout []string // forbidden stdout substrings + wantErr bool + }{ + { + name: "filter to a subset by id", + args: []string{"ls", "ab12", "ef56", "--sort", "id"}, + wantStdout: "- [ ] [ab12] 2026-06-01: first idea\n- [ ] [ef56] 2026-06-03: third idea\n", + noStdout: []string{"cd34"}, + }, + { + name: "unknown id warns on stderr and lists the rest", + args: []string{"ls", "ab12", "zzzz"}, + wantStdout: "- [ ] [ab12] 2026-06-01: first idea\n", + wantStderr: []string{`no idea with ID "zzzz"`}, + noStdout: []string{"zzzz"}, + }, + { + name: "all-unknown ids warn and fall through to empty message", + args: []string{"ls", "yyyy", "zzzz"}, + wantStdout: "No ideas found.\n", + wantStderr: []string{`no idea with ID "yyyy"`, `no idea with ID "zzzz"`}, + }, + { + name: "malformed id is a usage error", + args: []string{"ls", "TOOLONG"}, + wantErr: true, + wantStderr: []string{ + "invalid ID", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repo := setupGitRepo(t) + writeRepoBacklog(t, repo, seed) + + stdout, stderr, err := runSplit(t, bin, repo, tt.args...) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got none\nstdout=%q stderr=%q", stdout, stderr) + } + } else if err != nil { + t.Fatalf("%v failed: %v\nstdout=%q stderr=%q", tt.args, err, stdout, stderr) + } + + if tt.wantStdout != "" && stdout != tt.wantStdout { + t.Errorf("stdout = %q, want %q", stdout, tt.wantStdout) + } + for _, sub := range tt.wantStderr { + if !strings.Contains(stderr, sub) { + t.Errorf("stderr %q missing %q", stderr, sub) + } + } + for _, sub := range tt.noStdout { + if strings.Contains(stdout, sub) { + t.Errorf("stdout %q should not contain %q", stdout, sub) + } + } + }) + } +} + +// TestList_PipedOutputIsCanonical pins the pipe contract: piped `ls` and +// `ls --full` emit byte-identical canonical FormatLine output (no ANSI, no +// ellipsis truncation) regardless of --full, since the subprocess is not a TTY. +func TestList_PipedOutputIsCanonical(t *testing.T) { + bin := buildBinary(t) + repo := setupGitRepo(t) + // A long idea that WOULD truncate on a narrow terminal. + long := strings.Repeat("x", 300) + writeRepoBacklog(t, repo, "- [ ] [ab12] 2026-06-01: "+long+"\n") + + plain, _, err := runSplit(t, bin, repo, "ls") + if err != nil { + t.Fatalf("ls failed: %v", err) + } + full, _, err := runSplit(t, bin, repo, "ls", "--full") + if err != nil { + t.Fatalf("ls --full failed: %v", err) + } + want := "- [ ] [ab12] 2026-06-01: " + long + "\n" + if plain != want { + t.Errorf("piped ls stdout:\ngot: %q\nwant: %q", plain, want) + } + if full != want { + t.Errorf("piped ls --full stdout:\ngot: %q\nwant: %q", full, want) + } + if strings.Contains(plain, "\033[") || strings.Contains(plain, "…") { + t.Errorf("piped output leaked ANSI/ellipsis: %q", plain) + } +} + +// TestPrune_CountHeaderAndDecisionMatrix covers feature B (the leading stderr +// count header) and the non-TTY rows of the feature E decision matrix. The +// subprocess pipes stdout (never a TTY), so: no-force prints the header + the +// removable lines on stdout + the trailing fallback hint and never prompts; +// --force deletes immediately. The interactive TTY prompt is verified at the +// seam level (internal/idea term tests) since it requires a real terminal. +func TestPrune_CountHeaderAndDecisionMatrix(t *testing.T) { + bin := buildBinary(t) + + mixed := "# Backlog\n\n" + + "- [ ] [op3n] 2026-06-01: keep me\n" + + "- [x] [d0ne] 2026-06-02: prune me\n" + + "- [x] [d1ne] 2026-06-03: prune me too\n" + + tests := []struct { + name string + args []string + wantStdout string + wantStderr []string // required stderr substrings (in addition to header) + noStderr []string // forbidden stderr substrings + wantBacklog string // "" means unchanged + }{ + { + name: "non-tty no-force: header + lines on stdout + fallback hint, no prompt", + args: []string{"prune"}, + wantStdout: "- [x] [d0ne] 2026-06-02: prune me\n- [x] [d1ne] 2026-06-03: prune me too\n", + wantStderr: []string{"2 done idea(s) would be pruned", "Re-run with --force to confirm."}, + noStderr: []string{"[y/N]"}, + }, + { + name: "non-tty force: deletes immediately, count only", + args: []string{"prune", "--force"}, + wantStdout: "Pruned 2 done idea(s).\n", + noStderr: []string{"would be pruned", "[y/N]"}, + wantBacklog: "# Backlog\n\n- [ ] [op3n] 2026-06-01: keep me\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repo := setupGitRepo(t) + writeRepoBacklog(t, repo, mixed) + + stdout, stderr, err := runSplit(t, bin, repo, tt.args...) + if err != nil { + t.Fatalf("%v failed: %v\nstdout=%q stderr=%q", tt.args, err, stdout, stderr) + } + if stdout != tt.wantStdout { + t.Errorf("stdout = %q, want %q", stdout, tt.wantStdout) + } + for _, sub := range tt.wantStderr { + if !strings.Contains(stderr, sub) { + t.Errorf("stderr %q missing %q", stderr, sub) + } + } + for _, sub := range tt.noStderr { + if strings.Contains(stderr, sub) { + t.Errorf("stderr %q should not contain %q", stderr, sub) + } + } + + want := tt.wantBacklog + if want == "" { + want = mixed + } + if got := readRepoBacklog(t, repo); got != want { + t.Errorf("backlog:\ngot:\n%s\nwant:\n%s", got, want) + } + }) + } +} + +// TestConfirmPrune covers the [y/N] confirm logic in-process (feature E). It +// reads cmd.InOrStdin(), so it is unit-testable without a PTY by feeding a +// buffer: only "y"/"yes" (case-insensitive, trimmed) confirm; everything else — +// including bare Enter and EOF (no input) — aborts. The TTY gating that decides +// whether confirmPrune is even reached is exercised by the seam tests; here we +// pin the decision itself. +func TestConfirmPrune(t *testing.T) { + tests := []struct { + name string + input string + want bool + }{ + {name: "y confirms", input: "y\n", want: true}, + {name: "yes confirms", input: "yes\n", want: true}, + {name: "uppercase Y confirms", input: "Y\n", want: true}, + {name: "YES confirms", input: "YES\n", want: true}, + {name: "y with surrounding spaces confirms", input: " y \n", want: true}, + {name: "n aborts", input: "n\n", want: false}, + {name: "no aborts", input: "no\n", want: false}, + {name: "bare enter aborts", input: "\n", want: false}, + {name: "EOF (no input) aborts", input: "", want: false}, + {name: "garbage aborts", input: "yep\n", want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := newRootCmd() + cmd.SetIn(strings.NewReader(tt.input)) + cmd.SetErr(&bytes.Buffer{}) // swallow the prompt + if got := confirmPrune(cmd, 2); got != tt.want { + t.Errorf("confirmPrune(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} + +// TestPrune_ConfirmedDeleteAndAbort proves the two file-state outcomes of the +// interactive confirm (closing acceptance A-008's TTY rows and A-011 with real +// tests rather than code inspection): a "y"/"yes" answer deletes exactly like +// --force, while any abort answer leaves the backlog byte-identical. It mirrors +// the prune RunE guard — idea.Prune(path, true) runs only when confirmPrune +// returns true — using the real internal/idea seam, so the confirm logic and +// the resulting write (or no-write) are tested together without a PTY. +func TestPrune_ConfirmedDeleteAndAbort(t *testing.T) { + const mixed = "# Backlog\n\n" + + "- [ ] [op3n] 2026-06-01: keep me\n" + + "- [x] [d0ne] 2026-06-02: prune me\n" + + "- [x] [d1ne] 2026-06-03: prune me too\n" + const pruned = "# Backlog\n\n- [ ] [op3n] 2026-06-01: keep me\n" + + tests := []struct { + name string + input string + wantConfirm bool + wantBacklog string + }{ + {name: "y deletes the done ideas", input: "y\n", wantConfirm: true, wantBacklog: pruned}, + {name: "yes deletes the done ideas", input: "yes\n", wantConfirm: true, wantBacklog: pruned}, + {name: "n leaves the backlog byte-identical", input: "n\n", wantConfirm: false, wantBacklog: mixed}, + {name: "EOF leaves the backlog byte-identical", input: "", wantConfirm: false, wantBacklog: mixed}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "backlog.md") + if err := os.WriteFile(path, []byte(mixed), 0644); err != nil { + t.Fatal(err) + } + + cmd := newRootCmd() + cmd.SetIn(strings.NewReader(tt.input)) + cmd.SetErr(&bytes.Buffer{}) + + // Mirror the prune RunE guard exactly: dry run first (never writes), + // then delete only on a confirmed answer. + if _, _, err := idea.Prune(path, false); err != nil { + t.Fatalf("dry-run prune: %v", err) + } + confirmed := confirmPrune(cmd, 2) + if confirmed != tt.wantConfirm { + t.Fatalf("confirmPrune = %v, want %v", confirmed, tt.wantConfirm) + } + if confirmed { + if _, _, err := idea.Prune(path, true); err != nil { + t.Fatalf("confirmed prune: %v", err) + } + } + + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != tt.wantBacklog { + t.Errorf("backlog:\ngot:\n%s\nwant:\n%s", got, tt.wantBacklog) + } + }) + } +} diff --git a/src/cmd/idea/output.go b/src/cmd/idea/output.go new file mode 100644 index 0000000..fbfba8d --- /dev/null +++ b/src/cmd/idea/output.go @@ -0,0 +1,35 @@ +package main + +import ( + "fmt" + "io" + "os" + + "github.com/sahil87/idea/internal/idea" +) + +// printIdeaLines renders a slice of ideas one-per-line to out, TTY-aware. It is +// the single home of the list/prune rendering policy (Constitution IV's split +// keeps the truncation/color logic in internal/idea; this just picks the mode): +// when stdout is a terminal each line is truncated to the width and colored +// (unless full); when piped or redirected, full canonical FormatLine output is +// emitted regardless of full, preserving the machine-parseable pipe contract +// (Constitution VI). +// +// The TTY/width/color decision keys on os.Stdout (the real destination) while +// out is the write target — out is normally os.Stdout in production and a +// buffer under test; both list and prune write their machine-readable lines to +// stdout. +func printIdeaLines(out io.Writer, ideas []idea.Idea, full bool) { + if idea.IsTTY(os.Stdout) { + width := idea.TermWidth(os.Stdout) + color := idea.UseColor(os.Stdout) + for _, i := range ideas { + fmt.Fprintln(out, idea.DisplayListLine(i, width, full, color)) + } + return + } + for _, i := range ideas { + fmt.Fprintln(out, idea.FormatLine(i)) + } +} diff --git a/src/cmd/idea/prune.go b/src/cmd/idea/prune.go index 4403723..5d397c0 100644 --- a/src/cmd/idea/prune.go +++ b/src/cmd/idea/prune.go @@ -1,25 +1,32 @@ package main import ( + "bufio" "fmt" + "os" + "strings" "github.com/sahil87/idea/internal/idea" "github.com/spf13/cobra" ) func pruneCmd() *cobra.Command { - var force bool + var force, full bool cmd := &cobra.Command{ Use: "prune", Short: "Bulk-remove all done ideas from the backlog", Long: `Bulk-remove all done ([x]) ideas from the current worktree's backlog. -Without --force this is a free dry run: each done idea that would be removed -is listed on stdout and nothing is written. --force performs the deletion and -prints only a count. There is no archive — the backlog is committed, so git -history is the recovery path. --main targets the main worktree's backlog, and ---file / IDEAS_FILE point elsewhere (see "idea --help"). +Without --force this lists each done idea that would be removed. On a terminal +it then prompts for confirmation ([y/N]) and deletes only if you confirm; when +the output is piped it stays a free dry run (removable lines on stdout, a hint +on stderr) and never prompts. --force skips the prompt and deletes immediately, +printing only a count. There is no archive — the backlog is committed, so git +history is the recovery path. Long idea text is truncated to the terminal width +(--full shows it in full); piped output is always full and machine-parseable. +--main targets the main worktree's backlog, and --file / IDEAS_FILE point +elsewhere (see "idea --help"). idea prune idea prune --force`, @@ -40,17 +47,39 @@ history is the recovery path. --main targets the main worktree's backlog, and return nil } - if !force { - // Free dry run: stdout carries exactly the removable lines - // (pipe-friendly); the confirm hint is advisory, so it goes - // to stderr like the backfill notice (Constitution VI). - for _, i := range pruned { - fmt.Println(idea.FormatLine(i)) - } + if force { + printBackfillNotice(cmd, backfilled) + fmt.Printf("Pruned %d done idea(s).\n", len(pruned)) + return nil + } + + // Dry-run / pre-confirm path. The leading count header is the + // primary signal, printed to stderr (advisory) before the list so a + // human sees the action first regardless of list length; stdout + // still carries exactly the removable lines (pipe-friendly). + fmt.Fprintf(cmd.ErrOrStderr(), "%d done idea(s) would be pruned\n", len(pruned)) + printIdeaLines(cmd.OutOrStdout(), pruned, full) + + // Interactive confirm only when stdout is a TTY (a prompt on a pipe + // would hang). On a TTY the prompt replaces the trailing hint; on a + // pipe we fall back to the classic dry run with the trailing hint. + if !idea.IsTTY(os.Stdout) { fmt.Fprintln(cmd.ErrOrStderr(), "Re-run with --force to confirm.") return nil } + if !confirmPrune(cmd, len(pruned)) { + fmt.Fprintln(cmd.ErrOrStderr(), "Aborted — no ideas removed.") + return nil + } + + // Confirmed: perform the deletion via the same force path. Use the + // count from this confirmed prune (not the earlier dry run) so the + // reported total is accurate even if the file changed in between. + pruned, backfilled, err = idea.Prune(path, true) + if err != nil { + return err + } printBackfillNotice(cmd, backfilled) fmt.Printf("Pruned %d done idea(s).\n", len(pruned)) return nil @@ -58,6 +87,18 @@ history is the recovery path. --main targets the main worktree's backlog, and } cmd.Flags().BoolVar(&force, "force", false, "Confirm deletion of all done ideas") + cmd.Flags().BoolVar(&full, "full", false, "Show full idea text on a terminal (no truncation)") return cmd } + +// confirmPrune writes the [y/N] prompt to stderr and reads one line from the +// command's stdin. It returns true only for "y"/"yes" (case-insensitive, +// trimmed); any other input (including EOF) aborts. +func confirmPrune(cmd *cobra.Command, n int) bool { + fmt.Fprintf(cmd.ErrOrStderr(), "Prune %d done idea(s)? [y/N] ", n) + reader := bufio.NewReader(cmd.InOrStdin()) + line, _ := reader.ReadString('\n') + answer := strings.ToLower(strings.TrimSpace(line)) + return answer == "y" || answer == "yes" +} diff --git a/src/go.mod b/src/go.mod index e096320..29626d1 100644 --- a/src/go.mod +++ b/src/go.mod @@ -2,9 +2,13 @@ module github.com/sahil87/idea go 1.22 -require github.com/spf13/cobra v1.8.1 +require ( + github.com/spf13/cobra v1.8.1 + golang.org/x/term v0.27.0 +) require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/spf13/pflag v1.0.5 // indirect + golang.org/x/sys v0.28.0 // indirect ) diff --git a/src/go.sum b/src/go.sum index 912390a..dadd163 100644 --- a/src/go.sum +++ b/src/go.sum @@ -6,5 +6,9 @@ github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA= +golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q= +golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/src/internal/idea/term.go b/src/internal/idea/term.go new file mode 100644 index 0000000..7497f0e --- /dev/null +++ b/src/internal/idea/term.go @@ -0,0 +1,154 @@ +package idea + +import ( + "os" + "strconv" + "strings" + + "golang.org/x/term" +) + +// This file is the TTY/width/color/truncation seam (Constitution IV): all +// terminal-aware rendering logic lives here in internal/idea, and the cmd/ +// layer only asks this seam for a decision (is this a TTY? how wide? use +// color?) and for a ready-to-print display line. FormatLine/DisplayLine stay +// the machine/canonical renderers and are intentionally untouched. + +// defaultTermWidth is the column count assumed when the real width cannot be +// detected (GetSize fails and $COLUMNS is unset/invalid). 80 is the universal +// terminal default. +const defaultTermWidth = 80 + +// ANSI styling codes. Color is only ever emitted on a TTY with NO_COLOR unset +// (see UseColor); these are applied AFTER width-based truncation so the width +// math counts visible runes, never escape bytes. +const ( + ansiReset = "\033[0m" + ansiFaint = "\033[2m" // dim — used for the [id] date: prefix + ansiGreen = "\033[32m" // used for a done [x] checkbox +) + +// ellipsis is the single-rune marker appended to a clipped text portion. +const ellipsis = "…" // U+2026 + +// IsTTY reports whether f is connected to an interactive terminal. It is the +// single gate every display change (truncation, color, the prune prompt) keys +// on so that piped/redirected output stays full and canonical (Constitution VI). +func IsTTY(f *os.File) bool { + if f == nil { + return false + } + return term.IsTerminal(int(f.Fd())) +} + +// TermWidth returns the terminal column count for f. Resolution order: +// term.GetSize first; if that fails or yields a non-positive width, honor +// $COLUMNS when it parses to a positive integer; otherwise fall back to 80. +func TermWidth(f *os.File) int { + if f != nil { + if w, _, err := term.GetSize(int(f.Fd())); err == nil && w > 0 { + return w + } + } + if cols := os.Getenv("COLUMNS"); cols != "" { + if n, err := strconv.Atoi(cols); err == nil && n > 0 { + return n + } + } + return defaultTermWidth +} + +// UseColor reports whether styled output should be emitted to f: true only when +// f is a TTY AND the NO_COLOR environment variable is unset. Per the NO_COLOR +// convention, presence disables color regardless of value (including an empty +// string), so the check is os.LookupEnv presence, not a truthiness test. +func UseColor(f *os.File) bool { + if !IsTTY(f) { + return false + } + _, noColor := os.LookupEnv("NO_COLOR") + return !noColor +} + +// dimPrefix wraps s in the ANSI faint code so the scannable [id] date: prefix +// recedes and the eye lands on the idea text. +func dimPrefix(s string) string { + return ansiFaint + s + ansiReset +} + +// greenCheck wraps s (a done "[x]" checkbox) in the ANSI green code. +func greenCheck(s string) string { + return ansiGreen + s + ansiReset +} + +// DisplayListLine renders an idea as a single physical list line for terminal +// display. It builds the canonical escaped line shape — the same prefix and +// escaped text that FormatLine produces — but, when full is false, truncates +// only the text portion to fit width (appending the ellipsis when clipped) and, +// when the escaped text contains a literal "\n" escape (a multiline idea), +// clips at the first newline regardless of width so the line is always one +// physical row. The "- [done] [id] date: " prefix is NEVER truncated. +// +// width is the terminal column count (a parameter so tests inject it rather +// than allocating a PTY). When color is true the prefix is dimmed and a done +// [x] checkbox is greened — applied AFTER truncation so the width math counts +// visible runes, not escape bytes. +func DisplayListLine(i Idea, width int, full, color bool) string { + check := i.StatusCheck() + // The prefix mirrors formatLineWith up to (and including) the "text" + // position: "- [] [] : ". + prefix := "- [" + check + "] [" + i.ID + "] " + i.Date + ": " + text := EscapeText(i.Text) + + if !full { + text = truncateText(text, width-len([]rune(prefix))) + } + + if !color { + return prefix + text + } + + styledCheck := "[" + check + "]" + if i.Done { + styledCheck = greenCheck(styledCheck) + } + // Dim the prefix EXCEPT the checkbox, which carries its own (green) color + // when done. Rebuild the prefix in two dim spans around the checkbox so the + // id/date stay faint while the [x] stays green. + styledPrefix := dimPrefix("- ") + styledCheck + dimPrefix(" ["+i.ID+"] "+i.Date+": ") + return styledPrefix + text +} + +// truncateText returns text clipped to fit avail visible columns, appending the +// ellipsis when it had to clip. It is rune-safe (operates on []rune, never byte +// slices) so multibyte text is never cut mid-rune. A multiline idea — escaped +// text containing a literal "\n" — is always clipped at the first newline (with +// an ellipsis) so the rendered line is one physical row, regardless of width. +// +// When avail is non-positive (the prefix alone already fills or exceeds the +// terminal) the text is reduced to just the ellipsis, since the prefix is the +// scannable anchor and is never itself clipped. +func truncateText(text string, avail int) string { + clipMultiline := false + if idx := strings.Index(text, `\n`); idx >= 0 { + text = text[:idx] + clipMultiline = true + } + + runes := []rune(text) + if !clipMultiline && len(runes) <= avail { + return text + } + + // Need to clip: reserve one column for the ellipsis. + if avail <= 1 { + return ellipsis + } + keep := avail - 1 + if keep >= len(runes) { + // The text fits but a multiline marker forced a clip; keep all of the + // first-line runes and mark that more follows. + return string(runes) + ellipsis + } + return string(runes[:keep]) + ellipsis +} diff --git a/src/internal/idea/term_test.go b/src/internal/idea/term_test.go new file mode 100644 index 0000000..b0aa8dd --- /dev/null +++ b/src/internal/idea/term_test.go @@ -0,0 +1,229 @@ +package idea + +import ( + "os" + "strings" + "testing" + "unicode/utf8" +) + +// TestTermWidth_Fallback exercises the resolution order when the real terminal +// size is undetectable. In the test process os.Stdout is not a real terminal, +// so term.GetSize fails and TermWidth falls back to $COLUMNS, then to the 80 +// default. t.Setenv restores the env after each case. +func TestTermWidth_Fallback(t *testing.T) { + tests := []struct { + name string + columns string // "" means unset + setEnv bool + want int + }{ + {name: "COLUMNS unset falls back to 80", setEnv: false, want: defaultTermWidth}, + {name: "COLUMNS set to a positive integer is honored", setEnv: true, columns: "120", want: 120}, + {name: "COLUMNS empty falls back to 80", setEnv: true, columns: "", want: defaultTermWidth}, + {name: "COLUMNS non-numeric falls back to 80", setEnv: true, columns: "abc", want: defaultTermWidth}, + {name: "COLUMNS zero falls back to 80", setEnv: true, columns: "0", want: defaultTermWidth}, + {name: "COLUMNS negative falls back to 80", setEnv: true, columns: "-5", want: defaultTermWidth}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.setEnv { + t.Setenv("COLUMNS", tt.columns) + } else { + unsetEnvForTest(t, "COLUMNS") + } + // A nil file skips the GetSize branch deterministically (os.Stdout + // is not a real terminal under `go test` either, but nil is the + // portable way to force the fallback path). + if got := TermWidth(nil); got != tt.want { + t.Errorf("TermWidth() = %d, want %d", got, tt.want) + } + }) + } +} + +// TestIsTTY_NonTerminal verifies non-terminal files (and nil) report false. A +// regular temp file is never a terminal. +func TestIsTTY_NonTerminal(t *testing.T) { + if IsTTY(nil) { + t.Error("IsTTY(nil) = true, want false") + } + f, err := os.CreateTemp(t.TempDir(), "tty-test-*") + if err != nil { + t.Fatal(err) + } + defer f.Close() + if IsTTY(f) { + t.Error("IsTTY(regular file) = true, want false") + } +} + +// TestUseColor honors NO_COLOR presence and the TTY gate. Under `go test` the +// given file is never a terminal, so UseColor must be false regardless of +// NO_COLOR — the TTY gate dominates. The NO_COLOR-presence semantics are +// covered directly via the helper's documented contract below. +func TestUseColor_TTYGate(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "color-test-*") + if err != nil { + t.Fatal(err) + } + defer f.Close() + + // Not a TTY: always false, even with NO_COLOR unset. + unsetEnvForTest(t, "NO_COLOR") + if UseColor(f) { + t.Error("UseColor(non-tty) = true, want false (TTY gate)") + } + // nil file is never a TTY. + if UseColor(nil) { + t.Error("UseColor(nil) = true, want false") + } +} + +// TestColorHelpers verifies the dim/green wrappers wrap their input in the +// expected ANSI codes and that the visible text is preserved. +func TestColorHelpers(t *testing.T) { + if got := dimPrefix("abc"); got != ansiFaint+"abc"+ansiReset { + t.Errorf("dimPrefix = %q, want faint-wrapped", got) + } + if got := greenCheck("[x]"); got != ansiGreen+"[x]"+ansiReset { + t.Errorf("greenCheck = %q, want green-wrapped", got) + } +} + +// TestDisplayListLine is the core table: rune-safe truncation that never clips +// the prefix, ellipsis presence, multiline-at-first-newline, --full bypass, and +// color-after-truncation. Width is injected (no PTY) per Constitution V. +func TestDisplayListLine(t *testing.T) { + openShort := Idea{ID: "ab12", Date: "2026-06-01", Text: "short text", Done: false} + // prefix = "- [ ] [ab12] 2026-06-01: " (25 visible runes) + prefixLen := len("- [ ] [ab12] 2026-06-01: ") + + tests := []struct { + name string + idea Idea + width int + full bool + color bool + want string + }{ + { + name: "fits within width is untouched", + idea: openShort, + width: 80, + want: "- [ ] [ab12] 2026-06-01: short text", + }, + { + name: "over-wide text is clipped with ellipsis, prefix intact", + idea: Idea{ID: "ab12", Date: "2026-06-01", Text: "abcdefghij"}, + width: prefixLen + 4, // room for 3 text runes + ellipsis + want: "- [ ] [ab12] 2026-06-01: abc" + ellipsis, + }, + { + name: "full bypasses truncation even when over-wide", + idea: Idea{ID: "ab12", Date: "2026-06-01", Text: "abcdefghij"}, + width: prefixLen + 4, + full: true, + want: "- [ ] [ab12] 2026-06-01: abcdefghij", + }, + { + name: "multiline clipped at first newline regardless of width", + idea: Idea{ID: "ab12", Date: "2026-06-01", Text: "line one\nline two"}, + width: 200, // far wider than the whole escaped text + want: "- [ ] [ab12] 2026-06-01: line one" + ellipsis, + }, + { + name: "prefix never clipped when terminal narrower than prefix", + idea: Idea{ID: "ab12", Date: "2026-06-01", Text: "anything"}, + width: 5, // far narrower than the 25-rune prefix + want: "- [ ] [ab12] 2026-06-01: " + ellipsis, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := DisplayListLine(tt.idea, tt.width, tt.full, tt.color) + if got != tt.want { + t.Errorf("DisplayListLine() =\n %q\nwant\n %q", got, tt.want) + } + }) + } +} + +// TestDisplayListLine_RuneSafe verifies truncation lands on a rune boundary for +// multibyte text (no broken UTF-8) and that the visible text portion before the +// ellipsis is exactly the requested number of runes. +func TestDisplayListLine_RuneSafe(t *testing.T) { + // Five 3-byte runes; clipping mid-rune via byte slicing would corrupt UTF-8. + i := Idea{ID: "ab12", Date: "2026-06-01", Text: "日本語版あ"} + prefixLen := len([]rune("- [ ] [ab12] 2026-06-01: ")) + + got := DisplayListLine(i, prefixLen+3, false, false) // keep 2 runes + ellipsis + if !utf8.ValidString(got) { + t.Fatalf("output is not valid UTF-8: %q", got) + } + want := "- [ ] [ab12] 2026-06-01: 日本" + ellipsis + if got != want { + t.Errorf("DisplayListLine() = %q, want %q", got, want) + } +} + +// TestDisplayListLine_ColorAfterTruncation verifies color is applied around the +// already-truncated plain text: the visible (ANSI-stripped) line equals the +// uncolored render, so the width math counted runes, not escape bytes. The done +// checkbox is greened and the id/date prefix is dimmed. +func TestDisplayListLine_ColorAfterTruncation(t *testing.T) { + i := Idea{ID: "ab12", Date: "2026-06-01", Text: "abcdefghij", Done: true} + prefixLen := len([]rune("- [x] [ab12] 2026-06-01: ")) + width := prefixLen + 4 + + colored := DisplayListLine(i, width, false, true) + plain := DisplayListLine(i, width, false, false) + + if !strings.Contains(colored, ansiGreen) { + t.Errorf("expected green code for done checkbox, got %q", colored) + } + if !strings.Contains(colored, ansiFaint) { + t.Errorf("expected faint code for prefix, got %q", colored) + } + if stripped := stripANSI(colored); stripped != plain { + t.Errorf("ANSI-stripped colored line = %q, want %q (color must not change visible text)", stripped, plain) + } +} + +// unsetEnvForTest forces key to be unset for the duration of the test, capturing +// any prior value and restoring it via t.Cleanup so the unset state never leaks +// into other tests (t.Setenv only restores a value it set, not a deletion). +func unsetEnvForTest(t *testing.T, key string) { + t.Helper() + prev, had := os.LookupEnv(key) + os.Unsetenv(key) + t.Cleanup(func() { + if had { + os.Setenv(key, prev) + } else { + os.Unsetenv(key) + } + }) +} + +// stripANSI removes ANSI escape sequences for visible-text comparison in tests. +func stripANSI(s string) string { + var b strings.Builder + for i := 0; i < len(s); { + if s[i] == '\033' { + // Skip until the terminating 'm' of a CSI sequence. + for i < len(s) && s[i] != 'm' { + i++ + } + if i < len(s) { + i++ // skip the 'm' + } + continue + } + b.WriteByte(s[i]) + i++ + } + return b.String() +}