Skip to content

fix(web): preserve terminal links across wide cells - #10815

Open
satyalyadav wants to merge 2 commits into
pingdotgg:mainfrom
satyalyadav:fix/terminal-unicode-links
Open

fix(web): preserve terminal links across wide cells#10815
satyalyadav wants to merge 2 commits into
pingdotgg:mainfrom
satyalyadav:fix/terminal-unicode-links

Conversation

@satyalyadav

@satyalyadav satyalyadav commented Sep 8, 2026

Copy link
Copy Markdown

Summary

  • Treat Ghostty wide-cell spacer tails and soft-wrap spacer heads as zero-width when reconstructing terminal text.
  • Preserve link hit-testing and highlight ranges across CJK, emoji, and wrapped terminal rows.
  • Add regressions for URLs, file paths, real spaces, emoji, and soft-wrapped links.

Validation

  • pnpm --filter @t3tools/web exec vp test run src/terminal/ghostty/surface.test.ts --project unit
    • 52 passed
  • pnpm --filter @t3tools/web typecheck
    • passed; existing suggestion diagnostics only
  • Browser verification is pending manual testing.

Summary by CodeRabbit

  • Bug Fixes
    • Improved terminal text extraction and link detection for wide characters, including emojis.
    • Links now remain contiguous across wide-character spacer cells and trailing emoji cells.
    • Fixed link handling when links span wrapped rows.
    • Soft-wrapped wide-character spacer heads no longer incorrectly break links.
    • Prevented spacer cells from being mistakenly included in or excluded from detected link ranges.

Copilot AI lite review requested due to automatic review settings September 8, 2026 17:36
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 8, 2026

Copilot AI 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.

🟢 Approval recommended

The core link/offset logic changes are cohesive and well-covered by targeted unit tests, with only a minor test-fixture realism suggestion.

Pull request overview

This PR improves the Ghostty terminal surface’s link reconstruction and hit-testing so URLs/paths remain contiguous and correctly highlighted across wide-cell spacer tails (CJK/emoji) and soft-wrapped spacer heads.

Changes:

  • Treat spacerTail and spacerHead cells as zero-width when reconstructing terminal row text for link extraction.
  • Update column↔text-offset mapping so link ranges include trailing spacer tails when appropriate (e.g., wide emoji at the end of a link).
  • Add regression tests covering CJK, emoji, real blanks, and soft-wrapped links.
File summaries
File Description
apps/web/src/terminal/ghostty/surface.ts Adjusts text reconstruction and offset/column mapping to preserve link detection across wide/spacer cells and wraps.
apps/web/src/terminal/ghostty/surface.test.ts Adds regression tests for spacer-tail/head behavior across URLs/paths, emoji, and wrapped rows.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +374 to +383
text: cells
.map(
(value) =>
value.text ||
(value.wide === GHOSTTY_CELL_WIDE.spacerTail || value.wide === GHOSTTY_CELL_WIDE.spacerHead
? ""
: " "),
)
.join("")
.trimEnd(),

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 8a6437d. rowFromCells.text now matches GhosttyTerminalCore exactly (cell.text || " "), while the production link reconstruction continues to derive text from the cell data and spacer flags.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: d46c2e59-ea3a-4950-973c-3017b1a5f202

📥 Commits

Reviewing files that changed from the base of the PR and between 8a6437d and 56bb01e.

📒 Files selected for processing (2)
  • apps/web/src/terminal/ghostty/surface.test.ts
  • apps/web/src/terminal/ghostty/surface.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The terminal now treats wide-cell spacer heads and tails as part of the same visual character during text extraction and link-range mapping. Tests cover URLs, paths, trailing emoji, wrapped rows, and soft-wrap spacer heads.

Changes

Wide-cell link resolution

Layer / File(s) Summary
Wide-cell terminal mapping
apps/web/src/terminal/ghostty/surface.ts
Terminal text and column calculations now account for wide-cell spacer cells. Link-range mapping includes a spacer tail at the end of a link.
Wide-cell link tests
apps/web/src/terminal/ghostty/surface.test.ts
Test helpers model wide cells and verify link resolution across spacer tails, wrapped rows, trailing emoji, and soft-wrap spacer heads.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 56bb0

Terminal link detection and highlight ranges now correctly span wide characters and soft-wrapped rows. The targeted unit coverage passes, with no concrete merge-blocking risk identified.

Suggested reviewers: juliusmarminge, stienswout, utkarshusername

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preserving terminal links across Ghostty wide cells.
Description check ✅ Passed The description explains the changes, the problem addressed, regression coverage, and validation results. It uses Summary and Validation headings instead of the template headings and omits the checkli…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@satyalyadav
satyalyadav force-pushed the fix/terminal-unicode-links branch from 8a6437d to 56bb01e Compare September 8, 2026 22:04
@satyalyadav

Copy link
Copy Markdown
Author

@StiensWout When you have time, could you review this small bug fix? It preserves terminal links across CJK/emoji wide cells and wrapped rows. Tests and browser verification are complete. No rush though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants