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

BytesMut does not implement io::Write #425

Open
Ten0 opened this issue Aug 8, 2020 · 9 comments
Open

BytesMut does not implement io::Write #425

Ten0 opened this issue Aug 8, 2020 · 9 comments

Comments

@Ten0
Copy link

Ten0 commented Aug 8, 2020

Is there any particular reason why BytesMut does not implement std::io::Write? Vec<u8> has it implemented. It looks like BytesMut should support that as well.

(NB: BytesMut has std::fmt::Write implemented, I'm talking about io::Write here, which is a different trait)

@fanatid
Copy link
Contributor

fanatid commented Aug 9, 2020

It's implemented, but only through bytes::buf::BufMutExt trait (method writer).

@Ten0
Copy link
Author

Ten0 commented Aug 9, 2020

Thank you, had not noticed, I can remove my own wrapper now ^^'
Looks to me like BytesMut could still implement it directly, it would make the interface easier.

@seanmonstar
Copy link
Member

It does seem like it could be implemented... I can't remember, I feel it used to be. Did we remove it?

@octylFractal
Copy link

Came across this today and just wanted to update for future readers that in 0.6.0, BytesMut::writer is available, the ext traits have been removed. Perhaps this should be closed?

@rich-murphey
Copy link

Can we bring back the writer in BufMutExt?

Consider the use case where one is using both BufMut methods, and wants to also use Write methods.
writer() consumes the BufMut, after which one would have to go through get_mut() to access the BufMut,

If the use case involves using both BufMut methods and Write methods on a given instance, BufMutExt would be a good fit.

@Ten0
Copy link
Author

Ten0 commented Jan 28, 2021

Came across this today and just wanted to update for future readers that in 0.6.0, BytesMut::writer is available, the ext traits have been removed. Perhaps this should be closed?

It still looks like it could be implemented directly on BufMut, no?

@rich-murphey
Copy link

rich-murphey commented Jan 28, 2021

yea, directly on BytesMut, could look something like:

impl std::io::Write for BytesMut {
    fn write(&mut self, src: &[u8]) -> std::io::Result<usize> {
        self.extend_from_slice(src);
        Ok(src.len())
    }
    fn flush(&mut self) -> std::io::Result<()> {
        Ok(())
    }
}

The docs do say that Buf and BufMut "serve different purposes" from Read and Write.

However, while working with actix-web, the HTTP Response object takes body composed of Bytes. To efficiently construct Bytes, I use a BytesMut, where portions of the protocol framing are best handled by buf.put_*(), while other parts are handled by serde::to_writer(), which wants a Write object.

It'd be nice to have a direct implementation somewhat similar to how Vec implements Write, if that's reasonable of course.

@rich-murphey
Copy link

I'd be glad to write a PR if needed. Are there design concerns about this?

For example, should Write be implemented on a separate BytesMutExt trait to keep it separate, or should it be directly on BytesMut? The reason as ask is... writer() eventually moved from the extension to directly on BytesMut. If that's where Write would eventually end up, it may be helpful if it's there from the start.

Here's why. Look at the current stable actix-web 3.3.2 for example. It uses bytes version 0.5.3 released December 12, 2019. Sol there's the possibility that changes such as this may delayed for dependent crates, and the impact of changing ti from BytesMutExt to BytesMut could occur long afterward.

@rich-murphey
Copy link

rich-murphey commented Feb 7, 2021

I submitted a PR #478 to implement Write for BytesMut.
It uses the same trait impl as Vec does.

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

No branches or pull requests

5 participants