Skip to content

feat(common-cli): wrap CLI output to terminal width - #349

Merged
matthyk merged 3 commits into
mainfrom
feat/471-cli-output-wrap
Aug 4, 2026
Merged

feat(common-cli): wrap CLI output to terminal width#349
matthyk merged 3 commits into
mainfrom
feat/471-cli-output-wrap

Conversation

@matthyk

@matthyk matthyk commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

What & why

Custom-rendered CLI output (reports, validate, rules list, request, etc.) was emitted without any width management, so long lines ran past the terminal edge and wrapped inconsistently with oclif's own help/error output. This PR makes our custom render layer wrap to the terminal width the same way oclif does, then hardens that wrapping so it never corrupts machine-readable or copy-pasted output.

Closes #471.

How it works

A small render layer in packages/common-cli/src/render/utils.ts:

  • terminalWidth() mirrors oclif's own resolution order — explicit OCLIF_COLUMNS, then settings.columns, then the clamped real process.stdout width — computed per call so tests and live terminals both work.
  • wrap() wraps prose to that width, ANSI-aware (colour codes survive, wrapping is measured by visible width).
  • wrapIndented() wraps leaf prose with a hanging indent: the caller supplies the first line's prefix (indent + optional tree glyph, e.g. " ├── ") and continuation lines align under the content without repeating the glyph.

These are wired through the report renderer, validate, rules list, request, plugins list, feedback, generate, and serve.

Safety hardening (from code review)

Wrapping user-facing output must never make it wrong. The following make wrapping copy-paste- and pipe-safe:

  • Soft wrapping (hard: false) — unbreakable tokens (file paths, URLs, JSON blobs, rule identifiers) are never split mid-token, so they stay copy-paste- and grep-able. Only whitespace-separated prose reflows.
  • Non-TTY output is emitted verbatimterminalWidth() reports no finite width when stdout is not a TTY (and no width is pinned), so thymian … > out.txt and … | grep receive unwrapped output. Machine consumption of piped/redirected output is preserved.
  • Bogus width guarded — only a positive OCLIF_COLUMNS/settings.columns is honoured, so -1/0/non-numeric no longer collapses every line to one column per line.
  • Raw HTTP wire values are never wrapped — in request.ts the status/request line, Host, and header values are printed verbatim so values containing significant whitespace are not mutated.
  • Width-aware run-heading divider — the tool · type · ──── separator in the report is sized from terminalWidth() (80 fallback) instead of a hardcoded 70, so it wraps uniformly with everything else.
  • No dangling glyph lineswrapIndented('') renders nothing instead of a lone tree/bullet glyph.
  • Dead indent removedrules list formatRule no longer builds a leading indent that the caller stripped and re-added; wrapIndented owns the indent.

Testing

  • common-cli, thymian, and core unit suites pass.
  • terminal-width.test.ts covers soft-wrap keeping tokens intact, non-TTY verbatim output, non-positive OCLIF_COLUMNS, indent reservation, and the empty-text guard.
  • cli-report-renderer.test.ts asserts every line (prose and the now width-aware divider) stays within the pinned width.
  • Lint + build clean for the touched packages.

🤖 Generated with Claude Code

matthyk and others added 2 commits August 3, 2026 11:25
Add terminalWidth()/wrap()/wrapIndented() to @thymian/common-cli, mirroring
oclif's screen.js width source (OCLIF_COLUMNS, settings.columns, clamped TTY,
non-TTY -> 80) through public API only, and apply ANSI-aware wrapping across the
render/* renderers and the thymian command layer. Structured payloads (raw
bodies, JSON/YAML dumps, generated templates) stay unwrapped. Adds wrap-ansi as
a direct dependency and pins OCLIF_COLUMNS=80 in the e2e clean env.

Refs #471

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address code-review findings on the CLI output-wrapping change:

- wrap() now soft-wraps (hard: false) so unbreakable tokens (file paths,
  URLs, JSON blobs, rule identifiers) are never split mid-token and stay
  copy-paste/grep-able; only whitespace-separated prose reflows.
- terminalWidth() returns Infinity for non-TTY output with no pinned
  width, and wrap() emits text verbatim in that case, so piping and
  redirection are no longer corrupted (> out.txt, | grep work again).
- terminalWidth() only accepts a positive OCLIF_COLUMNS/settings.columns,
  so a bogus value (-1, 0, non-numeric) no longer collapses every line to
  one column.
- wrapIndented('') renders nothing instead of a dangling glyph line.
- cli-report run-heading divider is sized from terminalWidth() (80
  fallback) instead of a hardcoded 70, wrapping uniformly with the rest.
- request.ts emits raw HTTP wire values (status/request line, Host,
  header values) verbatim so significant whitespace is never mutated.
- rules/list.ts formatRule no longer builds a leading indent that the
  caller stripped and re-added; wrapIndented owns the indent.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 3, 2026 11:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a terminal-width-aware wrapping layer to Thymian’s custom CLI rendering so custom output (reports, validate, rules list, request, etc.) wraps consistently with oclif, while preserving pipe-/redirect-safe and copy-paste-safe behavior.

Changes:

  • Introduces terminalWidth(), wrap(), and wrapIndented() in @thymian/common-cli and exports them publicly.
  • Wires wrapping through the CLI report renderer and multiple packages/thymian commands to enforce consistent width/indent behavior.
  • Adds unit tests for wrapping behavior and pins OCLIF_COLUMNS=80 in e2e env for deterministic output.

Reviewed changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/thymian/src/hooks/feedback.ts Wraps the feedback hook tip line to terminal width.
packages/thymian/src/commands/validate.ts Wraps validate output lines and issue bullets with hanging indents.
packages/thymian/src/commands/serve.ts Wraps the serve-mode status message.
packages/thymian/src/commands/rules/list.ts Wraps rule list entries and removes “dead” pre-indentation from formatRule.
packages/thymian/src/commands/request.ts Wraps human-facing result messages; keeps raw HTTP wire output unwrapped.
packages/thymian/src/commands/plugins/list.ts Wraps plugin names when listing plugins.
packages/thymian/src/commands/generate/rule.ts Wraps the “rule written to …” confirmation line.
packages/thymian/src/commands/generate/config.ts Wraps various generate-config informational lines.
packages/thymian/src/commands/feedback.ts Wraps feedback command intro and last-error hint line.
packages/common-cli/test/terminal-width.test.ts Adds unit tests for terminalWidth, wrap, and wrapIndented.
packages/common-cli/test/cli-report-renderer.test.ts Adds assertions that report output lines stay within pinned width and preserve hanging indent alignment.
packages/common-cli/src/render/utils.ts Implements terminalWidth, wrap, and wrapIndented helpers and ANSI-stripping width logic.
packages/common-cli/src/render/test-executions.ts Applies wrapIndented to test-step lines to keep tree alignment while wrapping.
packages/common-cli/src/render/findings.ts Applies wrapIndented to finding titles and expected/actual detail lines.
packages/common-cli/src/render/create-execution-renderer.ts Wraps headings, grouped entries, and messages while preserving indentation.
packages/common-cli/src/render/cli-report.ts Uses width-aware run dividers and wraps the report summary line.
packages/common-cli/src/index.ts Re-exports terminalWidth, wrap, and wrapIndented from common-cli.
packages/common-cli/package.json Adds wrap-ansi dependency to support ANSI-aware wrapping.
package-lock.json Updates lockfile to include wrap-ansi.
e2e-tests/src/env-utils.ts Pins OCLIF_COLUMNS=80 for stable wrapping in e2e runs.

Comment thread packages/common-cli/src/render/utils.ts
Comment thread packages/common-cli/src/render/utils.ts Outdated
Comment thread packages/common-cli/test/terminal-width.test.ts
- positiveInt() parses OCLIF_COLUMNS strictly via Number(...) instead of
  parseInt, so a partially-numeric override like "80cols" is rejected
  rather than silently pinning the width to 80.
- visibleWidth() now uses string-width (the same measurement wrap-ansi
  applies to content) instead of stripped .length, so wrapIndented's
  hanging-indent columns stay consistent with where the text actually
  wraps for wide (CJK/emoji) and surrogate-pair glyphs. Adds string-width
  as a direct dependency (already resolved transitively via wrap-ansi).
- terminal-width test no longer leaks mutated globals: it saves/restores
  process.stdout.getWindowSize and deletes settings.columns when it was
  originally absent. Adds coverage for the strict parse and wide-glyph
  hanging indent.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 3, 2026 11:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.

Suppressed comments (1)

packages/common-cli/src/render/cli-report.ts:70

  • runHeading() sizes the divider using prefix.length, but the PR’s wrapping logic is terminal-column-based (via wrap-ansi/string-width). .length counts UTF-16 code units and can diverge from displayed width for wide glyphs (CJK/emoji) or ANSI sequences, which can make the divider exceed the intended terminal width or appear too short.

Consider computing the prefix width with string-width (or reusing a shared visible-width helper) and subtracting that instead: const prefixWidth = stringWidth(prefix); '─'.repeat(Math.max(1, fill - prefixWidth)).

  const prefix = `${toolName} · ${runType} · `;
  const width = terminalWidth();
  const fill = Number.isFinite(width) ? width : 80;

  return prefix + '─'.repeat(Math.max(1, fill - prefix.length));

@BaggersIO BaggersIO left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read through render/utils.ts and every call site, and checked the edge cases this touches against the exact pinned wrap-ansi@7.0.0/string-width@4.2.3 versions (ANSI survival across wrap points, non-SGR escapes, tokens far longer than the wrap width). All of that holds: color codes survive wrapping correctly, and the non-TTY / positive-only-width guards are correctly implemented and tested (including decimals, negatives, and "80cols").

Two things worth addressing:

  1. create-execution-renderer.ts:245 and :302 — the rule-id reference line ('› ' + rule.id / '› ' + execution.ruleId) never goes through wrap/wrapIndented, unlike every sibling line in the same functions (I confirmed this directly — the status line, heading, and label/message lines right next to it are all converted, this one isn't). Real rule ids in rules-rfc-9110 run to ~85 chars (e.g. rfc9110/final-recipient-should-exclude-sensitive-request-data-from-response-to-trace), so this line still overflows a standard 80-col terminal in the default report view — the exact class of bug this PR fixes everywhere else. Not a regression (same overflow existed before this PR), but it's in-scope and easy to miss because no wrap-test fixture attaches a ruleId long enough to trip it. Worth a fast follow-up given it's the primary output path.
  2. positiveInt() (utils.ts:41) uses strict Number(...), but the actual installed @oclif/core@4.13.0 parses OCLIF_COLUMNS via lenient Number.parseInt. So OCLIF_COLUMNS=80cols wraps oclif's own --help at 80 but leaves this custom output unwrapped — a real exception to the "consistent with oclif" framing, probably the right call (oclif's own handling of e.g. -1 is arguably worse) but worth a docstring caveat.

Minor: findings.ts's expected/actual JSON blobs do get split at whitespace inside string values, despite the "JSON blobs never split mid-token" framing in the description — the soft-wrap guarantee only covers whitespace-free tokens, so a JSON value with a prose string inside it can still wrap across lines.

None of this blocks merge — core module is correct and well-tested against the hardest edge cases. Approving.

@matthyk
matthyk merged commit 42f32c8 into main Aug 4, 2026
18 checks passed
@matthyk
matthyk deleted the feat/471-cli-output-wrap branch August 4, 2026 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants