-
Notifications
You must be signed in to change notification settings - Fork 37.1k
Remove throttle on pty input #283065
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove throttle on pty input #283065
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes the throttling mechanism for PTY input handling in the terminal process, simplifying the input flow by eliminating queuing and delayed writes. The changes directly write input to the PTY process instead of chunking and throttling writes through a queue system.
Key Changes
- Removed the
chunkInput()utility function that split large input into smaller chunks (max 50 characters) to prevent data corruption - Simplified the
input()method to write directly to the PTY process instead of using a write queue with 5ms intervals - Removed supporting infrastructure including
_writeQueue,_writeTimeout,_startWrite(), and_doWrite()methods
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/vs/platform/terminal/node/terminalProcess.ts |
Removed import of chunkInput, removed queue-related fields and methods (_writeQueue, _writeTimeout, _startWrite(), _doWrite()), and simplified input() method to write directly to PTY process |
src/vs/platform/terminal/common/terminalProcess.ts |
Removed chunkInput() utility function and related constants that were used to split large input into chunks |
Includes key change microsoft/node-pty#831 and follow up microsoft/node-pty#832 Fixes #246204 Fixes #283056
See microsoft/node-pty#831 for a detailed write up on the cause and solution.
Fixes #283056
Fixes #246204
cc @deepak1556
After test by pasting 50kb of data takes ~500ms - the reason that this is not instantaneous would likely require moving all writing to the fd into C++ (microsoft/node-pty#833):
After test ensure EAGAIN is correctly handled by pasting > 1kb during a shell sleep: