Skip to content

Fix string slicing for fs.write - #835

Merged
Daniel Imms (Tyriar) merged 2 commits into
microsoft:mainfrom
lhecker:dev/lhecker/fs-write-fix
Dec 17, 2025
Merged

Fix string slicing for fs.write#835
Daniel Imms (Tyriar) merged 2 commits into
microsoft:mainfrom
lhecker:dev/lhecker/fs-write-fix

Conversation

@lhecker

@lhecker Leonard Hecker (lhecker) commented Dec 15, 2025

Copy link
Copy Markdown
Member

This PR contains my spin on #831. The difference is it wraps any input as Buffer first and lacks a _writeInProgress field. This fixes a bug in #831 where the written callback parameter is used to slice string arguments even though it's probably meant for byte counts.

Comment thread src/unixTerminal.ts Outdated
@deepak1556 Robo (deepak1556) added this to the December / January 2026 milestone Dec 15, 2025

@Tyriar Daniel Imms (Tyriar) 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.

Seems to bring back the notorious interleaving bug:

Image

Comment thread src/unixTerminal.ts Outdated
Comment thread src/unixTerminal.ts
Comment thread src/unixTerminal.ts
Comment thread src/unixTerminal.ts Outdated
@Tyriar Daniel Imms (Tyriar) self-assigned this Dec 17, 2025
@Tyriar

Copy link
Copy Markdown
Contributor

Seems to bring back the notorious interleaving bug:

Image

This is only for cooked mode, will follow that up in new PR #833 (comment)

@lhecker

Copy link
Copy Markdown
Member Author

Thanks for fixing up my PR!

@Tyriar
Daniel Imms (Tyriar) merged commit a0ce44f into microsoft:main Dec 17, 2025
5 checks passed
Daniel Imms (Tyriar) added a commit to microsoft/vscode that referenced this pull request Dec 19, 2025
Brings in further improvements to handling of write backpressure. After
this we no longer throttle at all which means that for older versions of
bash at least on macOS interleaving is possible, but pasting and sending
should be near instantaneous for any reasonable amount of text. I
measured zsh injesting 5mb of data in ~14 seconds.

See:

- microsoft/node-pty#835
- microsoft/node-pty#837
- microsoft/node-pty#839

Part of #246204, #283056
@Tyriar Daniel Imms (Tyriar) modified the milestones: December / January 2026, 1.1.0 Dec 21, 2025
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.

4 participants