add support for BUN_CONFIG_ELIDE_LINES (& fixes #16286) - #18111
martinamps wants to merge 4 commits into
Conversation
6b289fc to
9d5e375
Compare
|
You should update the description to say "fixes #16286" so it gets closed as soon as this is merged. And thanks for this! |
|
aha good point, done. Hopefully gets unblocked soon cc @RiskyMH |
|
sorry @Jarred-Sumner those windows ones are a pain 😁 i should've made a way to mock that behavior probably |
3182af1 to
82331bc
Compare
|
All passing @RiskyMH @Jarred-Sumner :) anything else I need to do on this one, especially around docs? first time changing those |
|
bump @Jarred-Sumner @RiskyMH 🙇 |
|
@martinamps thanks for this change, I'd love to see it merged! Could you please resolve the conflicts and maybe add the maintainers as reviewers explicitly? Hopefully that'll help :) |
|
So looking forward to this 🔥 |
|
@martinamps would be very grateful if you could get this over the line. The fact that using |
…non-terminal environments (oven-sh#16286) - add support for BUN_CONFIG_ELIDE_LINES (oven-sh#11465)
2488bad to
1f77577
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a new exported env var Changes
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/filter_run.zig`:
- Around line 553-555: The warning currently checks state.pretty_output (derived
from Output.enable_ansi_colors_stdout) which can be true when colors are forced;
instead perform a real TTY check for stdout before warning. Modify the branch
that inspects ctx.bundler_options.elide_lines and state.pretty_output to call
the platform TTY detection (e.g., use the std.os isatty/getStdOut equivalent)
and only warn when stdout is not a TTY; update the condition around
ctx.bundler_options.elide_lines to use that TTY check rather than
state.pretty_output so forced-color scenarios don't bypass the non-terminal
warning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f950bcff-8330-4665-b2f8-32b422547f88
📒 Files selected for processing (5)
docs/runtime/environment-variables.mdxsrc/cli/Arguments.zigsrc/cli/filter_run.zigsrc/env_var.zigtest/cli/run/filter-workspace.test.ts
Split out from #18111 by @martinamps. `--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 --------- Co-authored-by: Martin Amps <m@rtin.so> Co-authored-by: Martin Amps <mamps@anthropic.com> Co-authored-by: robobun <robobun@bun.sh>
Split out from #18111 by @martinamps. Adds `BUN_CONFIG_ELIDE_LINES` as an environment-variable equivalent of `--elide-lines`, so the limit can be set once globally instead of per command. The CLI flag takes precedence when both are set. Closes #11465. Rebased after #28977 landed on main (which simplified the non-terminal error path into a silent no-op), so this branch no longer needs its own CLI-source tracking. Fixes on top of a straight env-var fallback: - Consolidate the two duplicated `--elide-lines` parse blocks in `Arguments.zig` into a single `parseElideLinesOption` helper, called after `loadConfigWithCmdArgs` so CLI + env var beat bunfig.toml. - Empty `--elide-lines ""` (e.g. `--elide-lines "$MAYBE_UNSET"`) falls through to the env var instead of shadowing it. - Env-var value is clamped via `std.math.cast` rather than `@intCast`, so 32-bit targets don't panic on values above `maxInt(usize)`. - `bunfig.zig` rejects negative, non-finite, fractional, and out-of-range `run.elide-lines` floats with an `Expected a non-negative integer` error. The `>=` upper bound accounts for f64 rounding `maxInt(u64)` up to `2^64` — a strict `>` would let the boundary value slip into a panicking `@intFromFloat`. Tests cover CLI vs env-var precedence, the empty-string fallback, the bunfig fractional rejection, and the non-terminal no-op contract.
Split out from #18111 by @martinamps. Adds `BUN_CONFIG_ELIDE_LINES` as an environment-variable equivalent of `--elide-lines`, so the limit can be set once globally instead of per command. The CLI flag takes precedence when both are set. Closes #11465. Rebased after #28977 landed on main (which simplified the non-terminal error path into a silent no-op), so this branch no longer needs its own CLI-source tracking. Fixes on top of a straight env-var fallback: - Consolidate the two duplicated `--elide-lines` parse blocks in `Arguments.zig` into a single `parseElideLinesOption` helper, called after `loadConfigWithCmdArgs` so CLI + env var beat bunfig.toml. - Empty `--elide-lines ""` (e.g. `--elide-lines "$MAYBE_UNSET"`) falls through to the env var instead of shadowing it. - Env-var value is clamped via `std.math.cast` rather than `@intCast`, so 32-bit targets don't panic on values above `maxInt(usize)`. - `bunfig.zig` rejects negative, non-finite, fractional, and out-of-range `run.elide-lines` floats with an `Expected a non-negative integer` error. The `>=` upper bound accounts for f64 rounding `maxInt(u64)` up to `2^64` — a strict `>` would let the boundary value slip into a panicking `@intFromFloat`. Tests cover CLI vs env-var precedence, the empty-string fallback, the bunfig fractional rejection, and the non-terminal no-op contract.
…#28977) Split out from oven-sh#18111 by @martinamps. `--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 oven-sh#16286 --------- Co-authored-by: Martin Amps <m@rtin.so> Co-authored-by: Martin Amps <mamps@anthropic.com> Co-authored-by: robobun <robobun@bun.sh>
Split out from #18111 by @martinamps. Adds `BUN_CONFIG_ELIDE_LINES` as an environment-variable equivalent of `--elide-lines`, so the limit can be set once globally instead of per command. The CLI flag takes precedence when both are set. Closes #11465. Rebased after #28977 landed on main (which simplified the non-terminal error path into a silent no-op), so this branch no longer needs its own CLI-source tracking. Fixes on top of a straight env-var fallback: - Consolidate the two duplicated `--elide-lines` parse blocks in `Arguments.zig` into a single `parseElideLinesOption` helper, called after `loadConfigWithCmdArgs` so CLI + env var beat bunfig.toml. - Empty `--elide-lines ""` (e.g. `--elide-lines "$MAYBE_UNSET"`) falls through to the env var instead of shadowing it. - Env-var value is clamped via `std.math.cast` rather than `@intCast`, so 32-bit targets don't panic on values above `maxInt(usize)`. - `bunfig.zig` rejects negative, non-finite, fractional, and out-of-range `run.elide-lines` floats with an `Expected a non-negative integer` error. The `>=` upper bound accounts for f64 rounding `maxInt(u64)` up to `2^64` — a strict `>` would let the boundary value slip into a panicking `@intFromFloat`. Tests cover CLI vs env-var precedence, the empty-string fallback, the bunfig fractional rejection, and the non-terminal no-op contract.
…#28977) Split out from oven-sh#18111 by @martinamps. `--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 oven-sh#16286 --------- Co-authored-by: Martin Amps <m@rtin.so> Co-authored-by: Martin Amps <mamps@anthropic.com> Co-authored-by: robobun <robobun@bun.sh>
Split out from #18111 by @martinamps. Adds `BUN_CONFIG_ELIDE_LINES` as an environment-variable equivalent of `--elide-lines`, so the limit can be set once globally instead of per command. The CLI flag takes precedence when both are set. Closes #11465. Implemented against the Rust CLI (`src/runtime/cli/Arguments.rs`, `src/bun_core/env_var.rs`, `src/bunfig/bunfig.rs`) after the src port: - Consolidate the two duplicated `--elide-lines` parse blocks into a single `parse_elide_lines_option` helper, called after `load_config_with_cmd_args` so CLI + env var beat `run.elide-lines` in bunfig.toml. - Empty `--elide-lines ""` (e.g. `--elide-lines "$MAYBE_UNSET"`) falls through to the env var instead of shadowing it. - Env var value is clamped via `usize::try_from(...).unwrap_or(usize::MAX)` so 32-bit targets don't lose high bits above `usize::MAX`. - `bunfig.rs` rejects negative, non-finite, fractional, and out-of-range `run.elide-lines` numbers with `Expected a non-negative integer` instead of silently saturating/truncating via `as usize`. Tests cover CLI vs env-var precedence, the empty-string fallback, the bunfig fractional rejection, and the non-terminal no-op contract. The bunfig test uses `--config=./bunfig.toml` (`=` form) because clap's `.one_optional` does not consume a following argv element.
Summary
Adds an
BUN_CONFIG_ELIDE_LINESenv var per this comment and fixes #16286Repro
Script:
Output before/after:
Cause
Not implemented
Fix
Implement support for the flag
Verification