Skip to content

fix(cli): keep config box within terminal width - #25

Merged
m-szymanska merged 8 commits into
mainfrom
agent/terminal-aware-config-box
Aug 24, 2026
Merged

fix(cli): keep config box within terminal width#25
m-szymanska merged 8 commits into
mainfrom
agent/terminal-aware-config-box

Conversation

@m-szymanska

Copy link
Copy Markdown
Member

Summary

  • cap the human-readable PRVIEW CONFIG panel to the active terminal width
  • wrap long refs and preset notes using Unicode display widths and grapheme-safe splits
  • use a readable unboxed fallback for panes narrower than 24 columns
  • keep redirected human output deterministic at 100 columns; JSON output is unchanged

Root cause

The panel sized itself from its longest logical row with no terminal-width cap. The fast remote-only note made the frame 124 columns wide, so a 116-column pane wrapped the right border onto stray lines.

Verification

  • cargo fmt --check
  • cargo clippy --all-targets -- -D warnings
  • cargo test (1555 unit tests plus all integration suites, zero failures)
  • cargo build --release
  • focused config renderer tests across 40, 64, 80, 100, 116, 124, and 160 columns
  • real 116-column pseudo-TTY run against PR fix: complete decision-core operator truth #24: aligned walls, wrapped note, generated pack SANITY 5/5

The only build warning is the pre-existing future-incompatibility notice from proc-macro-error2 v2.0.1.

Copilot AI lite review requested due to automatic review settings August 23, 2026 18:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the CLI config panel to respect terminal width with Unicode-safe wrapping and narrow-pane fallback behavior.

Changes:

  • Caps and wraps human-readable output by terminal width.
  • Adds grapheme-safe Unicode handling and narrow-terminal fallback rendering.
  • Updates tests, documentation, changelog, and dependencies.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Summary
src/output/mod.rs Moderate issue (4 votes): narrow fallback rows can still exceed terminal width and require wrapping.
docs/usage.md Documents terminal-width behavior.
CHANGELOG.md Records the configuration panel fix.
Cargo.toml Adds Unicode handling dependencies.
Cargo.lock Updates locked dependencies.
Suppressed comments (2)

docs/usage.md:367

  • This says human-readable runs begin with the config panel, but App::run prints the === prview - PR Review & Artifact Generator === banner and a blank line before calling print_config (src/lib.rs:89-111). Reword this to say the panel appears after the initial banner.
Human-readable runs begin with a `PRVIEW CONFIG` panel. The panel follows the

src/output/mod.rs:932

  • If stdout is a TTY but terminal::size() fails, this branch falls back to 100 columns and can recreate an oversized boxed panel on a narrow terminal—the failure mode this change is meant to eliminate. Keep the 100-column fallback for redirected output only; an unknown TTY width should take a safe unboxed path instead.
        crossterm::terminal::size()
            .map(|(columns, _)| usize::from(columns))
            .unwrap_or(CONFIG_BOX_FALLBACK_COLUMNS)

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/output/mod.rs Outdated
Copilot AI review requested due to automatic review settings August 23, 2026 19:09

Copilot AI left a comment

Copy link
Copy Markdown

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 4 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

docs/usage.md:367

  • This wording is not consistent with the actual output order: App::run still prints the === prview ... banner before calling print_config (src/lib.rs:89-111), so the run does not begin with this panel. Please describe the panel as being included in the human-readable output instead.
Human-readable runs begin with a `PRVIEW CONFIG` panel. The panel follows the

src/output/mod.rs:987

  • split_whitespace() does not preserve the contents of a ref when it wraps this row: valid Git refs can contain non-ASCII whitespace (for example U+00A0/U+2003), but those separators are collapsed to ASCII spaces and any leading/trailing ones are removed. Since target.name and base.name are identifiers and were previously printed verbatim, wrap the whitespace spans without rewriting them (or use a ref-specific grapheme wrapper) so a long Unicode ref remains recognizable and exact.
    for word in input.split_whitespace() {
        let separator = if UnicodeWidthStr::width(current.as_str()) > indent_width {
            1
        } else {
            0
        };
        if UnicodeWidthStr::width(current.as_str()) + separator + UnicodeWidthStr::width(word)
            <= max_width
        {
            if separator == 1 {
                current.push(' ');
            }
            current.push_str(word);

Comment thread src/output/mod.rs
Copilot AI review requested due to automatic review settings August 23, 2026 19:13

Copilot AI left a comment

Copy link
Copy Markdown

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 4 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/output/mod.rs:975

  • When a row is long enough to wrap, split_whitespace() treats Unicode whitespace inside a valid Git ref (for example, a non-breaking space) as a separator and reconstructs it as an ASCII space. That changes the displayed target/base name instead of merely wrapping it; split only on the ASCII formatting whitespace used by these rows so Unicode whitespace in refs is preserved.
    for word in input.split_whitespace() {

src/output/mod.rs:968

  • For a one-column terminal (or a reported width of zero, which render_unboxed_config clamps to one), this branch forces a one-column indent even though it cannot fit. Wrapped rows with the normal leading spaces therefore emit at least two columns and violate the fallback's width cap; use no indent when the original indent does not fit.
    let indent = if UnicodeWidthStr::width(leading.as_str()) < max_width {
        leading
    } else {
        " ".to_string()

Copilot AI review requested due to automatic review settings August 23, 2026 19:22

Copilot AI left a comment

Copy link
Copy Markdown

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 4 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/output/mod.rs:1000

  • When a wrapped ref contains valid non-ASCII whitespace (for example U+00A0, which Git refnames can contain), split_whitespace() treats it as a separator and rebuilds the row with an ASCII space, so the displayed target/base name no longer matches the actual ref. Use ASCII-only separators for the renderer's own formatting whitespace so Unicode whitespace inside a ref is preserved and then split only at grapheme boundaries.
    for word in input.split_whitespace() {

Copilot AI review requested due to automatic review settings August 24, 2026 02:14

Copilot AI left a comment

Copy link
Copy Markdown

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 4 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/output/mod.rs:932

  • A PTY can report a successful 0-column window size (for example before its size is initialized). This turns into Some(0), but the fallback later clamps that to width 1, so the renderer emits lines wider than the reported terminal and does not take the documented unknown-width path. Treat zero as an unavailable size before calling render_config.
        crossterm::terminal::size()
            .map(|(columns, _)| usize::from(columns))
            .ok()

src/output/mod.rs:984

  • The widest_grapheme calculation excludes graphemes made entirely of Unicode whitespace. A ref can contain a wide whitespace grapheme such as U+3000; with max_width = 2, the long four-space indent is reduced to one column and content_width becomes 1, but that width-2 grapheme then reaches split_at_display_width and is emitted together with the indent, producing a 3-column line. Include whitespace grapheme widths in this maximum (or otherwise handle them) so the narrow fallback's no-overflow invariant also holds for these refs.
    let widest_grapheme = input
        .graphemes(true)
        .filter(|grapheme| !grapheme.chars().all(char::is_whitespace))
        .map(UnicodeWidthStr::width)

Copilot AI review requested due to automatic review settings August 24, 2026 02:32

Copilot AI left a comment

Copy link
Copy Markdown

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 4 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

docs/usage.md:386

  • This documentation overstates the coverage: prview --watch is human-readable, but App::run_watch calls run_quick, and neither path calls print_config, so watch runs do not show this panel. Limit the sentence to non-watch runs or add the panel to the watch path.
After the initial banner, human-readable runs show a `PRVIEW CONFIG` panel. The

src/output/mod.rs:977

  • This iterator yields &str grapheme slices, but String cannot be collected directly from Iterator<Item = &str>; the new code therefore fails to compile. Concatenate the slices explicitly (or fold with push_str) when building normalized.
        .collect::<String>();

src/output/mod.rs:1775

  • trim_start_matches returns &str, so this collect::<String>() has the same unsupported FromIterator<&str> conversion and makes the new test module fail to compile. Concatenate the collected slices explicitly instead.
            .collect::<String>();

Copilot AI review requested due to automatic review settings August 24, 2026 02:38

Copilot AI left a comment

Copy link
Copy Markdown

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 4 out of 5 changed files in this pull request and generated no new comments.

@m-szymanska
m-szymanska merged commit 35b8d25 into main Aug 24, 2026
15 checks passed
@m-szymanska
m-szymanska deleted the agent/terminal-aware-config-box branch August 24, 2026 02:45
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.

2 participants