cli: make --elide-lines a no-op in non-terminal environments by alii · Pull Request #28977 · oven-sh/bun

alii · 2026-04-08T00:00:05Z

--elide-lines currently exits with an error when stdout is not a terminal, which breaks scripts that pass the flag and run in both interactive and CI/hook contexts. The flag is already a no-op in this case (the elision code only runs in the TTY redraw path), so the error serves no purpose. This removes it.

Fixes #16286

…non-terminal environments (#16286) - add support for BUN_CONFIG_ELIDE_LINES (#11465)

- Revert BUN_CONFIG_ELIDE_LINES additions (separate PR) - Revert unrelated scripts/build/flags.ts change - Remove the warning entirely instead of erroring; the flag is already a no-op when pretty_output is false (redraw() returns early before reading elide_count), so neither error nor warning is needed - Add cross-platform regression test for non-TTY + --elide-lines Fixes #16286

robobun · 2026-04-08T00:00:18Z

^{Updated 9:06 PM PT - Apr 7th, 2026}

❌ Your commit 948d8634 has 2 failures in Build #44361 (All Failures):

test/js/valkey/valkey.test.ts - timeout on 🐧 25.04 aarch64
test/js/valkey/valkey.test.ts - timeout on 🐧 25.04 x64
test/js/valkey/valkey.test.ts - timeout on 🐧 13 x64
test/js/valkey/valkey.test.ts - timeout on 🐧 13 aarch64
test/js/valkey/valkey.test.ts - timeout on 🐧 13 x64-baseline
test/js/valkey/valkey.test.ts - timeout on 🐧 3.23 x64-baseline
test/js/valkey/valkey.test.ts - timeout on 🐧 13 x64-asan
test/js/valkey/valkey.test.ts - timeout on 🐧 3.23 x64
test/js/valkey/valkey.test.ts - timeout on 🐧 3.23 aarch64
test/js/bun/http/bun-serve-file.test.ts - code 1 on 🐧 3.23 x64-baseline

🧪 To try this PR locally:

bunx bun-pr 28977

That installs a local version of the PR into your bun-28977 executable, so you can run:

bun-28977 --bun

coderabbitai · 2026-04-08T00:07:32Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 63b2e9fb-0c6b-4f3e-a1dc-def1feca5a51

📥 Commits

Reviewing files that changed from the base of the PR and between 48ab94d and 948d863.

📒 Files selected for processing (1)

test/cli/run/filter-workspace.test.ts

Walkthrough

Removed the runtime error that blocked use of --elide-lines when stdout was not a terminal; the flag is now ignored and the command proceeds without exiting on that condition.

Changes

Cohort / File(s)	Summary
CLI validation removal `src/cli/filter_run.zig`	Removed the early runtime check that printed `--elide-lines is only supported in terminal environments` and exited when `ctx.bundler_options.elide_lines` was set while `state.pretty_output` was false.
Test updates & additions `test/cli/run/filter-workspace.test.ts`	Dropped the `win32ExpectedError` argument and Windows-only assertion; updated spawn handling to capture `stdout`/`stderr`, assert absence of terminal-environment error in `stderr`, require `exitCode === 0`, verify `stdout` contains 20 `log_line` matches and optionally check for absence of antipatterns; added a new non-terminal test invoking `bun run --filter ... --elide-lines 5` with `NO_COLOR=1` asserting no error and successful run.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description lacks the required template structure with 'What does this PR do?' and 'How did you verify your code works?' sections, though it provides relevant context.	Restructure the description to follow the template with explicit 'What does this PR do?' and 'How did you verify your code works?' sections.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: converting --elide-lines from an error-causing flag to a no-op in non-terminal environments.
Linked Issues check	✅ Passed	The code changes directly address issue `#16286` by removing the error exit condition when --elide-lines is used in non-terminal environments, allowing the flag to become a no-op as intended.
Out of Scope Changes check	✅ Passed	All changes are scoped to removing the terminal environment validation and updating related tests; no unrelated modifications are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

claude

LGTM — simple removal of a spurious error that was already a no-op.

Extended reasoning...

Overview

Removes 6 lines from src/cli/filter_run.zig that exited with an error when --elide-lines was passed in a non-TTY context. The elision logic only runs inside the pretty_output TTY redraw path, so the error was serving no purpose. Test updates reflect the new correct behavior: no error, exit code 0, and all output visible.

Security risks

None. This is a pure behavioral change removing an unnecessary early-exit guard. No auth, crypto, or permissions code is touched.

Level of scrutiny

Low. The diff is minimal — a 6-line deletion and corresponding test cleanup. The logic is easy to verify by reading the readChunk and redraw functions: the if (\!this.pretty_output) return; guard in redraw() means elision is already skipped in non-TTY mode regardless of elide_count.

Other factors

The one bug flagged (inline comment) is a test quality nit — the new non-TTY test uses --elide-lines 0, which is a universal no-op, rather than a positive value that would more strongly validate non-TTY elision bypass. This does not affect implementation correctness.

test/cli/run/filter-workspace.test.ts

alii · 2026-04-08T00:26:41Z

@robobun adopt

robobun · 2026-04-08T00:27:58Z

🔴 Build #44361 test-bun shards on alpine/debian/ubuntu (x64, aarch64, baseline, asan) and darwin are failing — but the exact same set of shards is also red on unrelated PR #28979 (build #44365) which only touches node:http abort handling. Two independent PRs hitting identical Linux+macOS test-bun failures means this is a main-side infrastructure/test environment issue, not something this PR can fix.

The only annotated tests in my build are:

test/js/bun/http/bun-serve-file.test.ts > Stress testing > memory usage stays reasonable (RSS 116 MB vs 100 MB threshold — memory flake)
test/js/valkey/valkey.test.ts (TLS Basic Operations → ERR_REDIS_CONNECTION_CLOSED + main process killed by undefined but no core file found across all Linux distros)

Neither touches src/cli/filter_run.zig or test/cli/run/filter-workspace.test.ts. Waiting on a retrigger or main-side fix; nothing actionable on my side.

…TY path

claude

LGTM — my previous feedback on was addressed; the two remaining nits are in test code only and don't affect production correctness.

Extended reasoning...

Overview

The PR removes 6 lines from src/cli/filter_run.zig: the runtime guard that rejected --elide-lines with exit code 1 when stdout is not a terminal. The Zig change is a clean, mechanical deletion. The test changes update the Windows branch of runElideLinesTest (removing the now-obsolete error expectation) and add a new explicit test for the non-TTY no-op behavior.

Security risks

None. This is a CLI UX fix with no auth, crypto, or permission surface.

Level of scrutiny

Low. The implementation change is a single block removal with no logic added — only the error path is deleted, and the flag was already functionally a no-op in non-TTY mode (the elision code only runs inside redraw(), which returns early when pretty_output is false). The correctness of the no-op behavior is unchanged.

Other factors

My prior review flagged the use of --elide-lines 0 (a universal no-op) in the new test; the author addressed this by bumping to --elide-lines 5. The two remaining bug reports from the automated system are nit-level test quality issues (Windows branch not checking the antipattern array, and a vacuously-true stderr assertion that should target stdout). Neither affects production behavior — elision text is never written to stderr, and Windows piped output cannot trigger elision regardless. The inline comments on those nits are sufficient.

test/cli/run/filter-workspace.test.ts

…win32

claude

LGTM — all prior review feedback addressed, straightforward no-op promotion.

Extended reasoning...

Overview

The PR removes a 6-line validation block in src/cli/filter_run.zig that errored when --elide-lines was used outside a terminal, and updates test/cli/run/filter-workspace.test.ts to reflect the new no-op behavior.

Security risks

None. This is a pure UX fix: removing an error guard for a display-only flag. No auth, crypto, permissions, or data-exposure surfaces are touched.

Level of scrutiny

Low. The Zig change is a deletion-only diff with no new logic. The test changes are additive and address coverage gaps identified in prior reviews.

Other factors

All three issues I raised in previous review rounds were resolved: (1) the new test now uses --elide-lines 5 instead of 0 to catch future non-TTY elision regressions; (2) the not.toMatch(/lines elided/) assertion correctly targets stdout where flushDrawBuf writes; (3) the Windows branch now iterates the antipattern parameter. No outstanding comments remain.

martinamps and others added 5 commits April 2, 2026 23:37

- change from error to warning: --elide-lines will have no effect in …

1f77577

…non-terminal environments (#16286) - add support for BUN_CONFIG_ELIDE_LINES (#11465)

real tty check for elide lines warning

60619bd

Merge branch 'main' into elide-lines-env-var

6a0dc0c

Merge branch 'main' into elide-lines-env-var

36a53df

claude bot reviewed Apr 8, 2026

View reviewed changes

test/cli/run/filter-workspace.test.ts Show resolved Hide resolved

test: use --elide-lines 5 to guard against elision leaking into non-T…

48ab94d

…TY path

claude bot reviewed Apr 8, 2026

View reviewed changes

test/cli/run/filter-workspace.test.ts Show resolved Hide resolved

test/cli/run/filter-workspace.test.ts Outdated Show resolved Hide resolved

test: assert elision marker absent on stdout, enforce antipattern on …

948d863

…win32

claude bot reviewed Apr 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: make --elide-lines a no-op in non-terminal environments#28977

cli: make --elide-lines a no-op in non-terminal environments#28977
alii wants to merge 7 commits intomainfrom
claude/fix-16286-elide-lines-non-tty

alii commented Apr 8, 2026

Uh oh!

robobun commented Apr 8, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Apr 8, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

claude bot left a comment

Uh oh!

Uh oh!

alii commented Apr 8, 2026

Uh oh!

robobun commented Apr 8, 2026 •

edited

Loading

Uh oh!

claude bot left a comment

Uh oh!

Uh oh!

Uh oh!

claude bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

alii commented Apr 8, 2026

Uh oh!

robobun commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Uh oh!

alii commented Apr 8, 2026

Uh oh!

robobun commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Uh oh!

Uh oh!

claude bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robobun commented Apr 8, 2026 •

edited

Loading

coderabbitai bot commented Apr 8, 2026 •

edited

Loading

robobun commented Apr 8, 2026 •

edited

Loading