Skip to content

jsc: preserve tabs in runtime error caret padding - #37540

Open
Ersaoktaviannn wants to merge 4 commits into
oven-sh:mainfrom
Ersaoktaviannn:fix/runtime-error-caret-tabs
Open

jsc: preserve tabs in runtime error caret padding#37540
Ersaoktaviannn wants to merge 4 commits into
oven-sh:mainfrom
Ersaoktaviannn:fix/runtime-error-caret-tabs

Conversation

@Ersaoktaviannn

@Ersaoktaviannn Ersaoktaviannn commented Aug 11, 2026

Copy link
Copy Markdown

Summary

  • preserve tabs from source lines when positioning runtime error carets
  • account for supplementary-plane codepoints when building caret padding
  • add regression coverage for tab-indented runtime errors

Root cause

Runtime error source lines retain literal tabs, but the caret prefix was rendered entirely as spaces. Terminals therefore expanded the source and caret prefixes differently, placing the caret several columns to the left.

The updated padding logic walks the displayed UTF-8 source by codepoint, emits tabs at their original logical positions, and emits two padding cells for supplementary-plane codepoints.

Testing

  • cargo fmt --all --check
  • git diff --check
  • verified both regression cases fail against Bun 1.3.14 with the previous all-space padding

Fixes #10857

@claude claude Bot 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2cc961a4-aaf0-4c42-8c70-ae40522696be

📥 Commits

Reviewing files that changed from the base of the PR and between 9a543cc and 6cbbe8a.

📒 Files selected for processing (2)
  • src/jsc/VirtualMachine.rs
  • test/js/bun/test/stack.test.ts

Walkthrough

The error renderer now aligns carets with tabs and supplementary Unicode characters by using UTF-16 columns. Regression tests verify caret placement and process exit behavior.

Changes

Source error caret alignment

Layer / File(s) Summary
UTF-16-aware caret rendering
src/jsc/VirtualMachine.rs, test/js/bun/test/stack.test.ts
write_caret_padding preserves tabs and accounts for supplementary Unicode characters. Error rendering uses the helper, and regression tests verify caret alignment, empty stdout, and exit status 1.

Suggested reviewers: robobun, cirospaciari, dylan-conway

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: preserving tabs in runtime error caret padding.
Description check ✅ Passed The description explains the changes, root cause, verification steps, and linked issue.
Linked Issues check ✅ Passed The changes address issue #10857 by preserving tab spacing and aligning runtime error carets correctly.
Out of Scope Changes check ✅ Passed The implementation and regression tests remain within the scope of the linked issue and stated objectives.

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.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/test/stack.test.ts`:
- Around line 88-107: Extend the test named “runtime error caret preserves tabs
before the error column (`#10857`)” with a supplementary-plane Unicode character
and a tab before error(). Assert the resulting source and caret lines, including
the expected UTF-16-width column alignment, so the cursor.c > 0xFFFF branch is
covered while preserving the existing stdout and exit behavior.
🪄 Autofix

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 Plus

Run ID: 5ec99204-f2f6-455e-bbaf-940966701c5a

📥 Commits

Reviewing files that changed from the base of the PR and between da3851e and 048b02f.

📒 Files selected for processing (2)
  • src/jsc/VirtualMachine.rs
  • test/js/bun/test/stack.test.ts

Comment thread test/js/bun/test/stack.test.ts Outdated

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/VirtualMachine.rs`:
- Around line 6032-6039: Update the padding loop near codepoints_written and
codepoint_width so codepoints_written advances by codepoint_width rather than a
fixed 1, preserving UTF-16 column accounting for supplementary code points while
retaining the existing tab and space output behavior.
🪄 Autofix

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 Plus

Run ID: e33788e5-71ba-4ca0-8038-1c58bff3f741

📥 Commits

Reviewing files that changed from the base of the PR and between 048b02f and 7af8423.

📒 Files selected for processing (2)
  • src/jsc/VirtualMachine.rs
  • test/js/bun/test/stack.test.ts

Comment thread src/jsc/VirtualMachine.rs
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

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.

Error display column spacing is off with Tabs in source code

1 participant