Skip to content

fix(tui): size the /statusline picker for its padding rows - #7022

Merged
Hmbown merged 1 commit into
codewhale-hq:mainfrom
Lstarsky0:ux/statusline-picker-height
Oct 18, 2026
Merged

Hmbown merged 1 commit into
codewhale-hq:mainfrom
Lstarsky0:ux/statusline-picker-height

Conversation

@Lstarsky0

Copy link
Copy Markdown
Contributor

What and why

/statusline never shows its last two items, Workspace and Git branch, without scrolling, at any terminal size. At 80×24 the list ends here, before:

        │    [✓] Output rate  (average tok/s, including first-token…)  │
        │  Space toggle   a all   n none   Enter save   Esc cancel     │

After:

        │    [✓] Output rate  (average tok/s, including first-token…)  │
        │    [ ] Workspace  (directory this session writes to)         │
        │    [ ] Git branch  (branch the next commit lands on)         │
        │  Space toggle   a all   n none   Enter save   Esc cancel     │

At 120×40 it's the same list cut in the same place, with a dozen empty rows above and below the popup. Both hidden items are off by default, so nothing hints they exist until you arrow past "Output rate".

The picker asks centered_modal_area for one row per item plus five: two header lines, the action footer and the border. The block also has Padding::uniform(1), which takes a row above and below, and those two were never counted. It now asks for seven, the same allowance the /feedback picker makes for the same block. I updated the comment above it to name the padding rows.

Issue

No-Issue: /statusline popup two rows short, no issue filed for it

How I tested it

  • Ran a debug build in a pseudo-terminal with an empty HOME and opened /statusline at 40×12, 60×16, 80×24 and 120×40, with this change and with a build from before it. The rows above are from the 80×24 screens. At 80×24 and 120×40 all eleven items now show; at 60×16 and 40×12 the popup is already clamped to the screen and renders the same as before.
  • cargo fmt --all -- --check, cargo clippy -p codewhale-tui --all-targets --all-features with CI's flags, and cargo check -p codewhale-tui --all-targets: clean. Clippy also on 1.99, plus cargo +1.89.0 check -p codewhale-tui.
  • cargo nextest run -p codewhale-tui --lib: 15349 passed, none failed.

Checklist

  • One focused change, on 9a34d80; it merges cleanly into current main
  • cargo fmt --all passes, and tests cover new or changed behavior
  • If this adds or changes a user-facing feature, I updated its docs and its row in docs/features.toml (cargo test -p codewhale-tui --test feature_registry checks it) — n/a

The picker asks centered_modal_area for one row per item plus five:
two header lines, the action footer and the border. The block also
has Padding::uniform(1), which takes a row above and below, and those
two were never counted. So the popup is always two rows short and the
last two items, Workspace and Git branch, sit below the fold at every
terminal size. At 120x40 the list stops at "Output rate" with a dozen
empty rows above the popup, and nothing says there is more until you
arrow past the bottom.

Ask for seven rows on top of the items, the same allowance the
/feedback picker makes for the same block. Terminals too short for the
whole list still get the clamped popup and the scroll they have today.

Signed-off-by: Lstarsky0 <59827030+Lstarsky0@users.noreply.github.com>
@Lstarsky0
Lstarsky0 requested a review from Hmbown as a code owner October 18, 2026 15:55

@claude claude Bot 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions github-actions Bot added the contribution-gate Author not yet in .github/APPROVED_CONTRIBUTORS; a maintainer grants access with /lgtm label Oct 18, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-18T15:59:58.008710Z 21926a4 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21926a49bd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

// footer that now lives inside the body, plus the border and padding
// rows above and below. centered_modal_area clamps this to the frame
// and lets the scroll offset absorb any remaining overflow.
let needed_height = (self.rows.len() as u16).saturating_add(7);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Account for wrapped localized footer rows

When the localized action labels wrap—German at a normal 80×24 frame is one concrete case because the 64-column popup has a 60-column inner width—render_modal_footer consumes two rows, but rows.len() + 7 budgets only one. This leaves visible_rows == rows.len() - 1 despite spare screen height, so /statusline still opens with Git branch hidden until the user scrolls; calculate the footer's wrapped height and include it in needed_height instead of using a fixed seven-row allowance.

AGENTS.md reference: crates/tui/AGENTS.md:L25-L26

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Claude Opus 5.5, working for the maintainer.

Confirmed and fixed on main in 3352da9. In German at 80×24 the footer wraps onto two rows, so Git branch was still hidden. The picker now measures the wrapped footer with action_footer_lines at the popup inner width, and budgets rows + 6 + footer rows. I merged without reading this review first; that was my miss, not the PR author's.

@Hmbown
Hmbown enabled auto-merge October 18, 2026 16:20
@Hmbown

Hmbown commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

🤖 Claude Opus 5.5, working for the maintainer.

Thanks, @Lstarsky0. I checked the arithmetic against status_picker.rs: the border and Padding::uniform(1) take four rows, the action footer takes one at this width, and the body spends two on the header lines. That leaves height - 7 rows for items, so +5 always hid the last two (Workspace and Git branch) and +7 shows all eleven. Auto-merge is on and will land this once the remaining test jobs pass.

🤖 Generated by Claude Opus 5.5 for the Codewhale maintainers.

@Hmbown
Hmbown merged commit 89d1407 into codewhale-hq:main Oct 18, 2026
28 checks passed
Hmbown pushed a commit that referenced this pull request Oct 18, 2026
…mands keep their lines

All three were seen in the same real DeepSeek session that found the
approval-before-refusal bug (CW/artifacts/readme-shot-20261018/).

Diff gutter: format_line_numbers printed both the old and the new line
number on every row, so a removed line showed a new-side number and an
added line an old-side number. The patch card for the slugkit fix read
"   9    9 -     words = ..." and "  10    9 +     lower = ...", which
looks like line 9 exists on both sides. A '+' row now leaves the old
column blank and a '-' row leaves the new column blank; context rows
keep both. The existing context-numbering assertion is unchanged.

Diff context indentation: every row's marker was removed with
trim_start_matches, which strips all leading copies of the character. On
a context row (' ' prefix) that removed the code's own indentation, so
unchanged lines rendered flush left while the -/+ rows beside them kept
theirs; on a '-' or '+' row it also ate content that began with the same
character (a removed "- item" Markdown line lost its dash). All seven
sites now drop exactly the one marker byte (&raw[1..]; is_added,
is_deleted and the ' ' check guarantee it is there).

Shell card command: render_command_mode wrapped the whole command with
wrap_text, which keeps '\n' inside a chunk, and the span renderer then
dropped it, so "echo first-line\necho second-line" displayed as
"command: echo first-lineecho second-line". It now wraps each line
separately, matching how the approval card already shows the command.

Also credits @Lstarsky0 for #7022 in both changelogs.

Checks:
- cargo test -p codewhale-tui --lib -- tools::shell core::engine
  tui::history diff approval: 1500 passed; 0 failed; 5 ignored (same run
  as the refusal commit, before the indentation change)
- after the indentation change: cargo test -p codewhale-tui --lib -- diff
  patch file_mutation: 493 passed; 0 failed
- clippy with CI's flags: clean on the tree before the &raw[1..] change;
  after it, only cargo build and the diff tests were run

Verified in a real TUI with a scripted mock edit (phase5 in
CW/artifacts/cw7087-20261018/mock/server.py; captures diff-after-1.txt and
diff-after4-1.txt in CW/artifacts/approval-card-20261018/):
     4    4   def slug(text):
     5    5       """Join words with dashes."""
     6      -     words = text.split()
          6 +     lower = text.lower()
          7 +     words = lower.split()
     7    8       return '-'.join(words)
and the refused two-line mock command shows "command: echo first-line"
with "echo second-line" on the next row (multiline-after4-1.txt).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBKbhx19BRbZtvWK8rVKjz
Hmbown pushed a commit that referenced this pull request Oct 18, 2026
Both PRs merged with an unread Codex review finding. I merged them
without reading the review thread; these are the follow-ups.

#7022 (/statusline height, P2): the picker budgeted one footer row
(rows + 7), but render_modal_footer wraps the action hints. In German at
80x24 the 60-column inner width wraps "Space umschalten … Enter
speichern / Esc abbrechen" onto two rows, so Git branch was still hidden
until you scrolled. The picker now builds its hints once, measures them
with action_footer_lines at the popup's inner width (the width
centered_modal_area will give it, minus border and padding) and budgets
rows + 6 + footer rows. The hints array is reused by render_modal_footer.

#7024 (settings choice rail, P2): the rail paints the last column of the
choice rows, but each choice hitbox still spanned the full inner width,
so hovering or clicking the paint-only rail selected (and live-previewed)
the choice on that row. Hitboxes now give up that column when the rail is
drawn, matching the label width.

The third finding (#7012, P1: the /statusline mode hint shown in English)
was already fixed: StatusItem::hint(locale) routes through
MessageId::StatusPickerModeHint.

Verified in a real TUI at 80x24 with an isolated HOME
(CW/artifacts/approval-card-20261018/run-statusline-{de,en}.sh):
- German, before (codewhale-after6): the list ends at "[ ] Workspace",
  footer on two rows
- German, after: "[ ] Git branch" shown above the two footer rows
- English, after: all 11 items and the one-row footer, as before
The rail-hitbox change was checked by reading handle_mouse's use of
last_choice_hitboxes; no mouse event was driven.

Checks:
- cargo test -p codewhale-tui --lib -- views:: config_view edit_theme
  settings status_picker statusline: 528 passed; 0 failed; 1 ignored
- cargo clippy -p codewhale-tui --all-targets --all-features --locked with
  CI's flags: clean; cargo fmt --check: clean

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NBKbhx19BRbZtvWK8rVKjz
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution-gate Author not yet in .github/APPROVED_CONTRIBUTORS; a maintainer grants access with /lgtm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants