Skip to content

feat(tui): wrap exec_command invocations when expanded - #36

Closed
jinroh wants to merge 7 commits into
mainfrom
jinroh/exec-command-wrap
Closed

jinroh wants to merge 7 commits into
mainfrom
jinroh/exec-command-wrap

Conversation

@jinroh

@jinroh jinroh commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Changes

Expanded exec_command blocks now word-wrap long commands instead of truncating them. Collapsed view is unchanged.

  • First line wraps under the command's first column; metadata (in <dir> · timeout · exit) follows the command.
  • Later lines wrap behind │, with soft continuations indented further.
  • Syntax colors are kept across wrapped rows (lipgloss.Wrap).

How to test

BITS_FAKE_BACKEND=1 go run .
load("fakedata/exec_wrap.star", "all"); all()

The expanded view word-wraps every command line instead of truncating it, keeping syntax colors across rows. The first line hangs under the command's first column; later lines wrap behind the │ branch. Adds fakedata/exec_wrap.star for manual QA.
@jinroh
jinroh requested a review from a team as a code owner October 16, 2026 20:30

@datadog-official datadog-official 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.

Bits Code Review: FAIL

Two wrapping defects exist in the heredoc/command rendering path at tool.go:655. The more critical one is that lipgloss.Wrap can return rows exceeding the target width at breakpoint boundaries, causing truncation to silently drop command characters (e.g., && operators or trailing characters). A separate issue is that lipgloss.Wrap strips leading source whitespace from heredoc rows, misrepresenting indentation in Python or Makefile content.

Open Bits AI session

🤖 Bits Code Review · Commit a7f5768

Comment thread internal/tui/chat/tool.go Outdated
func hangWrap(lead, cont string, prefixWidth int, body string, width int) []string {
width = max(1, width)
bodyWidth := max(1, width-prefixWidth)
rows := strings.Split(lipgloss.Wrap(body, bodyWidth, shellBreakpoints), "\n")

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 Preserve source indentation when wrapping heredoc rows

At width 32, a heredoc row containing eight spaces followed by print('abcdefghij') becomes an empty branch row followed by the code without its source indentation. The row previously fit without truncation. This misrepresents significant Python or Makefile indentation in expanded commands. Preserving source whitespace requires adjusting the wrapping path rather than allowing lipgloss.Wrap to discard it.

React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it

What each reaction means
  • 👍 Useful: correct, and you're fixing it or will.
  • 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
  • 😕 Unclear: you can't tell what it means or why it matters.
  • 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.

🤖 Bits Code Review · Open Bits AI session

Comment thread internal/tui/chat/tool.go Outdated
func hangWrap(lead, cont string, prefixWidth int, body string, width int) []string {
width = max(1, width)
bodyWidth := max(1, width-prefixWidth)
rows := strings.Split(lipgloss.Wrap(body, bodyWidth, shellBreakpoints), "\n")

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 Hard-wrap oversized rows before truncation

At a rendering width of 20, expanding go test ./... && go vet ./... hides the &&, rendering ✓ ran go test ./...… followed by go vet ./.... The wrapping library can return oversized rows at breakpoint boundaries, and the subsequent truncation discards command characters. Expanded view should retain these operators so users can accurately inspect the command.

Suggested change
rows := strings.Split(lipgloss.Wrap(body, bodyWidth, shellBreakpoints), "\n")
body = ansi.Hardwrap(ansi.Wrap(body, bodyWidth, shellBreakpoints), bodyWidth, true)
rows := strings.Split(lipgloss.Wrap(body, bodyWidth, shellBreakpoints), "\n")

React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it

What each reaction means
  • 👍 Useful: correct, and you're fixing it or will.
  • 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
  • 😕 Unclear: you can't tell what it means or why it matters.
  • 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.

🤖 Bits Code Review · Open Bits AI session

…ands

Leading indentation is repeated on every wrapped row, the first row wraps to its own width, and rows the word wrapper leaves too wide are hard-wrapped instead of truncated. Drop & from the shell breakpoints. Adds indentation() and operators() to fakedata/exec_wrap.star.
Wrap styled segments to a cell width with prefixes, breakpoints, kept
indentation and optional ellipsis. Exec invocations and inspection rows
now use it instead of ad-hoc ANSI wrappers.
diffrender.Highlight returns styled segments instead of ANSI strings,
and Options.Wrap wraps long diff lines under the gutter. Expanded file
changes use it; compact rendering is byte-identical.
@jinroh jinroh closed this Oct 16, 2026
@jinroh
jinroh deleted the jinroh/exec-command-wrap branch October 16, 2026 22:58
@jinroh
jinroh restored the jinroh/exec-command-wrap branch October 16, 2026 22:59
@jinroh
jinroh deleted the jinroh/exec-command-wrap branch October 16, 2026 22:59
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.

1 participant