Skip to content

WIP: make macOS multiline PTY chunking byte-aware#300740

Closed
jcansdale wants to merge 2 commits intomicrosoft:mainfrom
jcansdale:jcansdale/pty-multiline-byte-aware
Closed

WIP: make macOS multiline PTY chunking byte-aware#300740
jcansdale wants to merge 2 commits intomicrosoft:mainfrom
jcansdale:jcansdale/pty-multiline-byte-aware

Conversation

@jcansdale
Copy link
Copy Markdown
Contributor

@jcansdale jcansdale commented Mar 11, 2026

Summary

This draft now includes both:

The root cause was that the macOS multiline chunking path used JavaScript string length and string slicing, while the relevant PTY limit is in UTF-8 bytes. This PR makes the chunking gate byte-aware and splits multiline writes on UTF-8-safe byte boundaries.

A failing CI run for the multibyte regression test is here:
https://github.com/microsoft/vscode/actions/runs/22947927444/job/66604943627#step:15:21324

What Changed

  • refactors the multiline PTY test helper so ASCII and multibyte cases share the same PTY lifecycle code
  • adds a multibyte regression test using CJK payloads
  • switches the macOS multiline chunking gate from data.length to Buffer.byteLength(data, 'utf8')
  • chunks writes on UTF-8-safe byte boundaries so multibyte characters are not split across PTY writes
  • keeps the existing 5 ms pacing between macOS multiline chunks

Notes

Copilot AI review requested due to automatic review settings March 11, 2026 10:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the TerminalProcess multiline write node tests to refactor command construction and add coverage for multibyte (UTF-8) payload sizing behavior relevant to macOS PTY buffering/chunking.

Changes:

  • Refactors multiline command generation into reusable helpers (ASCII and multibyte variants).
  • Refactors the core test runner to accept options (output file, command builder, wait timeout).
  • Adds a new multibyte payload test that asserts a UTF-16 vs UTF-8 size mismatch scenario.

@jcansdale jcansdale marked this pull request as ready for review March 11, 2026 10:55
@vs-code-engineering
Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@Tyriar

Matched files:

  • src/vs/platform/terminal/node/terminalProcess.ts

@jcansdale
Copy link
Copy Markdown
Contributor Author

Builds on reverted PR.

@jcansdale jcansdale closed this Mar 12, 2026
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.

5 participants