Skip to content
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

Add public split associated function to SimplexStream #7177

Open
KyleCotton opened this issue Feb 25, 2025 · 2 comments
Open

Add public split associated function to SimplexStream #7177

KyleCotton opened this issue Feb 25, 2025 · 2 comments
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-io Module: tokio/io

Comments

@KyleCotton
Copy link
Contributor

Currently the simplex function returns a (ReadHalf<SimplexStream>, WriteHalf<SimplexStream>) which does not conform to the usual convention of (tx, rx) used across the project which can be confusing - see #7175 fixed in #7176.

There are two routes I think we should consider, but I would be very interested in getting some additional input before I submit a PR:

  1. Adding another public method potentially SimplexStream::new_split that would return ( WriteHalf<SimplexStream>), ReadHalf<SimplexStream>) to better conform to the convention.

  2. Add an extension trait (maybe SplitEx ?) that would have a blanket implementation for T: AsyncRead + AsyncWrite which could enable an API such as tokio::io::simplex(64).split() for simplex and other resources across the project.

Personally I think the first option seems the cleanest and would allow us to then optionally deprecate simplex in a future release and have the public API for SimplexStream feel more familiar.

@KyleCotton KyleCotton added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels Feb 25, 2025
@Darksonn Darksonn added the M-io Module: tokio/io label Feb 25, 2025
@maminrayej
Copy link
Member

maminrayej commented Feb 25, 2025

  1. Adding another public method potentially SimplexStream::new_split that would return ( WriteHalf), ReadHalf) to better conform to the convention.

I'm afraid having two split functions may add to the confusion instead of mitigating it.

  1. Add an extension trait (maybe SplitEx ?) that would have a blanket implementation for T: AsyncRead + AsyncWrite which could enable an API such as tokio::io::simplex(64).split() for simplex and other resources across the project.

What would be the difference between this and tokio::io::split?

@KyleCotton
Copy link
Contributor Author

I'm afraid having two split functions may add to the confusion instead of mitigating it.

Given the introduction of SimplexStream I would assume that the documentation for the type would now serve as the primary entry point for new users. Potentially they would see the new_* methods first?

Agreed that having two splitting function could be confusing. I would suggest deprecating the simplex function (in some future release) in favour of the new SimplexStream API and make it more familiar to the other reader/writer APIs i.e. (tx, rx).

What would be the difference between this and tokio::io::split?

No functional difference. Would just enable method chaining uses.

I guess more generally in an ideal world we'd just swap the reader/writer in the simplex function. In reality this would be a major breaking change. I'd be eager to hear other suggestions to improve the ergonomics without having the major version bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-io Module: tokio/io
Projects
None yet
Development

No branches or pull requests

3 participants