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

std::io::Chain could implement BufRead #32536

Closed
troplin opened this issue Mar 27, 2016 · 7 comments
Closed

std::io::Chain could implement BufRead #32536

troplin opened this issue Mar 27, 2016 · 7 comments

Comments

@troplin
Copy link
Contributor

troplin commented Mar 27, 2016

The implementation is straight forward:

impl<T: BufRead, U: BufRead> BufRead for Chain<T, U> {
    fn fill_buf(&mut self) -> Result<&[u8]> {
        if !self.done_first {
            match try!(self.first.fill_buf()) {
                buf if buf.len() == 0 => { self.done_first = true; }
                buf => return Ok(buf),
            }
        }
        self.second.fill_buf()
    }
    fn consume(&mut self, amt: usize) {
        if !self.done_first {
            self.first.consume(amt)
        } else {
            self.second.consume(amt)
        }
    }
}

Or am I overlooking something?

@rphmeier
Copy link
Contributor

std::net::Chain -> std::io::Chain?

Now that the ? operator has landed, I would guess that would be favored over try! in std.
The tricky thing about trait implementations is that they become insta-stable, so it's usually good to be apprehensive of adding new ones. This one seems uncontroversial, so it's likely that it may have just been overlooked previously.

@troplin troplin changed the title std::net::Chain could implement BufRead std::io::Chain could implement BufRead Mar 27, 2016
@troplin
Copy link
Contributor Author

troplin commented Mar 27, 2016

Whoops, yes.
I've changed the title.

The implementation is not set in stone of course.

@sfackler
Copy link
Member

This seems reasonable I think - want to open a PR?

@troplin
Copy link
Contributor Author

troplin commented Mar 27, 2016

I'll give it a try. I'm still learning how to everything works with github...

@troplin
Copy link
Contributor Author

troplin commented Mar 27, 2016

@rphmeier you mentioned that it will be insta-stable. Does that mean that I have to tag it as stable for my PR?

#[stable(feature = "rust1", since = "1.9.0")]

Like that?

@sfackler
Copy link
Member

Yep, though you can't use the rust1 feature since feature names are tied to a specific release. Just make one up though - chain_buf_read or whatever.

bors added a commit that referenced this issue Mar 29, 2016
@Aatch
Copy link
Contributor

Aatch commented Mar 30, 2016

Closed by #32541

@Aatch Aatch closed this as completed Mar 30, 2016
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