Skip to content

Fix scrollback-limit byte handling#2927

Open
austinywang wants to merge 3 commits intomainfrom
issue-2879-scrollback-limit-ignored
Open

Fix scrollback-limit byte handling#2927
austinywang wants to merge 3 commits intomainfrom
issue-2879-scrollback-limit-ignored

Conversation

@austinywang
Copy link
Copy Markdown
Contributor

@austinywang austinywang commented Apr 15, 2026

Fixes #2879.

Summary

  • treat scrollback-limit as bytes in the local Ghostty config model
  • size the DEBUG large-scrollback generator by bytes and stream it with awk instead of brace expansion
  • update the docs example to use a byte-based scrollback-limit value

Testing

  • attempted ./scripts/reload.sh --tag issue-2879-scrollback-limit-ignored --launch
  • local tagged reload was blocked by repeated Xcode build-service hangs/crashes before the app launched in this environment

Note

Medium Risk
Changes the semantics and default of scrollback-limit, which can affect runtime memory usage and user expectations if existing configs assumed lines. Also adjusts the debug scrollback generator command generation, so regressions would surface mainly in debug tooling and config parsing.

Overview
Fixes scrollback-limit handling to match Ghostty’s byte-based semantics by updating the local config model default to 10_000_000 bytes and allowing underscore-separated integer literals via a new parseIntegerLiteral parser.

Updates the DEBUG large-scrollback tab generator to size output by target bytes (with 2MB–200MB bounds) and emit lines via a streamed awk loop instead of shell brace expansion. Documentation is updated to show a byte-scale scrollback-limit example value.

Reviewed by Cursor Bugbot for commit 3856ffa. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Treat scrollback-limit as bytes (not lines) and update the debug scrollback generator to size and stream by bytes. Fixes #2879 and aligns code and docs with Ghostty’s byte-based setting.

  • Bug Fixes
    • Config: parse scrollback-limit as bytes, default to 10_000_000, allow underscores, ignore invalid/negative values.
    • Debug generator: size by bytes (double limit with 2MB–200MB caps), compute lines by bytes-per-line with 6-digit width and a 2k line floor, stream via awk.
    • Docs: switch example to a byte value (scrollback-limit = 50000000).

Written for commit 3856ffa. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes

    • Scrollback capacity is now measured in bytes instead of lines, enabling much larger terminal history; default increased to 10,000,000.
    • Generating scrollback output now produces consistent, correctly numbered lines and always includes a trailing newline.
    • Configuration parsing accepts digit-group separators (e.g., underscores) and ignores invalid or non‑positive values.
  • Documentation

    • Updated configuration examples to reflect the new scrollback-limit units and default.

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 15, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Apr 16, 2026 2:06am

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 15, 2026

📝 Walkthrough

Walkthrough

Scrollback sizing was changed from a fixed line-based clamp to a byte-budget estimate; the default scrollback limit was increased (10_000 → 10_000_000 bytes). Config parsing now normalizes integer literals with underscores. Debug scrollback seeding estimates UTF‑8 bytes-per-line and generates lines via an awk-based generator.

Changes

Cohort / File(s) Summary
Scrollback Configuration
Sources/GhosttyConfig.swift
Default scrollbackLimit changed from 10_000 to 10_000_000 (interpreted as bytes). Added parseIntegerLiteral to strip _ separators and validate non-negative integer parsing for scrollback-limit.
Debug Scrollback Seeding
Sources/AppDelegate.swift
openDebugScrollbackTab now clamps config.scrollbackLimit to non-negative, computes a doubled effectiveLimit bounded to a byte budget, estimates bytes-per-line from a sample string to derive lineCount (min 2000), and switches the generator from a shell brace loop to an awk-based emitter that appends a trailing newline.
Docs / Example Config
web/app/.../docs/configuration/page.tsx
Updated rendered example scrollback-limit value in the documentation code block from 50000 to 50000000.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I’m a rabbit counting bytes with joy,
Lines no longer vanish, not one, oh boy!
I strip the underscores, measure UTF‑8 cheer,
awk hops in to print each line clear,
🐇📜✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description covers summary, testing, and a detailed rationale with risk assessment. However, it is missing critical checklist items, a demo video section, and bot review triggers as specified in the template. Complete the checklist items (testing confirmation, test coverage, docs updates), add a demo video link or note why one is not applicable, and include the review trigger block if bot reviews are needed.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix scrollback-limit byte handling' directly and concisely summarizes the primary change: updating scrollback-limit semantics from lines to bytes across config model, debug generator, and docs.
Linked Issues check ✅ Passed The PR directly addresses issue #2879 by changing scrollback-limit to bytes, updating the config model default to 10_000_000, adding byte-based parsing via parseIntegerLiteral, and updating the debug generator to size by bytes instead of lines, fully meeting the objective of making terminal surfaces honor the scrollback-limit value.
Out of Scope Changes check ✅ Passed All three modified files directly support the core objective: AppDelegate changes the debug generator, GhosttyConfig updates the model semantics, and docs reflects the byte-based value. No unrelated changes detected.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-2879-scrollback-limit-ignored

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.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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 `@Sources/GhosttyConfig.swift`:
- Around line 256-257: When parsing the scrollback-limit, don't accept zero or
negative values from Self.parseIntegerLiteral; after obtaining limit in
GhosttyConfig (the code block that sets scrollbackLimit), validate that limit >
0 and only then assign scrollbackLimit, otherwise leave the default and treat
the input as invalid. Apply the same non-positive-value rejection to the other
parse site that uses Self.parseIntegerLiteral (the similar block around lines
320-323) so both parsed byte limits require limit > 0 before assignment.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8390b642-0767-4955-aeca-ce6c389b476b

📥 Commits

Reviewing files that changed from the base of the PR and between c5f2e8c and 5b126b9.

📒 Files selected for processing (3)
  • Sources/AppDelegate.swift
  • Sources/GhosttyConfig.swift
  • web/app/[locale]/docs/configuration/page.tsx

Comment thread Sources/GhosttyConfig.swift
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 16, 2026

Greptile Summary

This PR corrects a fundamental unit mismatch: cmux was treating Ghostty's scrollback-limit as a line count when it is actually a byte count. The fix updates the default (10 K lines → 10 MB), teaches the config parser to handle underscore-separated integer literals, reworks the debug scrollback generator to target bytes (replacing bash brace expansion with awk to handle the resulting large counts), and updates the docs example to a byte-appropriate value.

Confidence Score: 5/5

Safe to merge; all findings are minor P2 style suggestions that do not affect correctness.

The core fix is correct and isolated — scrollbackLimit is only consumed by the debug generator. No overflow, no injection risk, and the awk-based generator is sound. Both P2 comments are minor approximation notes and a clarifying comment suggestion, neither blocks merge.

No files require special attention.

Important Files Changed

Filename Overview
Sources/GhosttyConfig.swift Default scrollbackLimit changed from 10,000 (lines) to 10,000,000 (bytes); parseIntegerLiteral added to handle underscore-separated numeric literals from Ghostty configs.
Sources/AppDelegate.swift Debug scrollback generator reworked to size output by bytes (not line count), with awk replacing brace expansion to handle the resulting large line counts; capping logic prevents overflow.
web/app/[locale]/docs/configuration/page.tsx Example scrollback-limit updated from 50000 (which Ghostty treats as 50 KB) to 50000000 (50 MB), aligning the docs with the byte-based unit.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["GhosttyConfig.parse()"] -->|"case scrollback-limit"| B["parseIntegerLiteral(value)\nStrip '_', parse Int"]
    B -->|success| C["scrollbackLimit = bytes\n(default: 10_000_000)"]
    B -->|failure| D["scrollbackLimit unchanged\n(silent no-op)"]
    C --> E["openDebugScrollbackTab()"]
    E --> F["doubledLimit = min(limit, 100M) × 2"]
    F --> G["targetBytes = clamp(doubledLimit, 2M, 200M)"]
    G --> H["lineCount = ⌈targetBytes / 18⌉, min 2000"]
    H --> I["awk command sent to terminal\ngenerates lineCount lines"]
Loading

Reviews (1): Last reviewed commit: "Fix scrollback limit handling" | Re-trigger Greptile

Comment thread Sources/AppDelegate.swift Outdated
Comment thread Sources/GhosttyConfig.swift
Comment thread Sources/AppDelegate.swift Outdated
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Sources/AppDelegate.swift">

<violation number="1" location="Sources/AppDelegate.swift:7817">
P2: Clamp `scrollbackLimit` to a non-negative range before doubling; current ordering can overflow on extreme negative config values.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread Sources/AppDelegate.swift Outdated
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 8ec3a86. Configure here.

Comment thread Sources/AppDelegate.swift Outdated
Comment thread Sources/GhosttyConfig.swift
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Sources/GhosttyConfig.swift">

<violation number="1" location="Sources/GhosttyConfig.swift:324">
P2: `parsed > 0` rejects `0`, so `scrollback-limit = 0` (which means "unlimited" in Ghostty) silently falls back to the 10MB default. The previous code accepted zero via `Int(value)`. Use `parsed >= 0` to preserve the semantics.</violation>
</file>

<file name="Sources/AppDelegate.swift">

<violation number="1" location="Sources/AppDelegate.swift:7823">
P1: Infinite loop: the `while true` convergence loop oscillates when the line count lands exactly at a digit-count boundary. For example, with `targetBytes ≈ 18M`, `digitCount=6` produces 1,000,000 lines (7 digits) and `digitCount=7` produces 947,369 lines (6 digits), bouncing forever. Add an iteration cap or break when `digitCount` revisits a previous value.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread Sources/AppDelegate.swift Outdated
Comment thread Sources/GhosttyConfig.swift Outdated
@austinywang
Copy link
Copy Markdown
Contributor Author

Addressed the remaining review feedback in 3856ffaa.

  • scrollback-limit = 0 is accepted again in the Ghostty config parser.
  • The DEBUG large-scrollback helper now sizes from the padded 6-digit floor instead of trying to converge on digit width, so it still emits at least the target byte budget without hanging at boundary cases.

Resolved the related review threads.

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.

scrollback-limit from Ghostty config is ignored — buffer capped at ~500 lines

1 participant