-
Notifications
You must be signed in to change notification settings - Fork 83
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
Use a ring buffer to pipe data between IO objects #819
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #819 +/- ##
==========================================
+ Coverage 57.36% 58.59% +1.22%
==========================================
Files 74 75 +1
Lines 10559 10732 +173
==========================================
+ Hits 6057 6288 +231
+ Misses 4502 4444 -58 ☔ View full report in Codecov by Sentry. |
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.
Overall, looks good to me but I left some suggestions.
src/exec/use_pty/pipe.rs
Outdated
// ▲ | ||
// │ | ||
// start | ||
buf.insert(&mut [0u8; HALF_LEN].as_slice()).unwrap(); |
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.
in this particular case you could set the initial condition by directly setting the value of start
to HALF_LEN
. that way this unit test does not rely on insert
and remove
correctly working in cases 1.1 and 2.1.
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.
Yeah I thought about it but I think I'd rather leave those tests as implementation agnostic as possible.
This PR supersedes #617