Skip to content

Use Write instead of owned strings - #8

Merged
Schniz merged 5 commits into
masterfrom
use-writer-instead-of-owned-strings
Oct 3, 2019
Merged

Schniz merged 5 commits into
masterfrom
use-writer-instead-of-owned-strings

Conversation

@Schniz

@Schniz Schniz commented Sep 26, 2019 •

Copy link
Copy Markdown
Collaborator

This should give better performance and be more "rusty"

cc @CasualX (as suggested in your comment in Reddit 😄)

@Schniz Schniz self-assigned this Sep 26, 2019
@CasualX

CasualX commented Sep 26, 2019

Copy link
Copy Markdown

Cool! I see you've picked the Write trait from io instead of fmt. The reason I suggested std::fmt::Write is because it's naturally oriented to strings (String implements the trait) where as std::io::Write naturally works with bytes (Vec<u8> implements the trait).

This would make the first a more natural fit. Adapting one to write to the other is fairly trivial if users have a need.

@Schniz

Schniz commented Sep 26, 2019

Copy link
Copy Markdown
Collaborator Author

@CasualX yes! I initially started with fmt but the htmlescaping library uses io so I just did simple find and replace

@Schniz

Schniz commented Sep 26, 2019

Copy link
Copy Markdown
Collaborator Author

I didn’t find a way to cast fmt::Write to io::Write, so I just used io. What’s the simple way to do that?

@CasualX

CasualX commented Sep 26, 2019 •

Copy link
Copy Markdown

You cannot cast them directly, you need an adapter struct:

pub struct IoAdapter<W>(pub W);
impl<W: io::Write> fmt::Write for IoAdapter<W> {
	fn write_str(&mut self, s: &str) -> fmt::Result {
		io::Write::write_all(&mut self.0, s.as_bytes()).map_err(|_| fmt::Error)
	}
	fn write_fmt(&mut self, args: fmt::Arguments) -> fmt::Result {
		io::Write::write_fmt(&mut self.0, args).map_err(|_| fmt::Error)
	}
}
impl IoAdapter<io::Stdout> {
	pub fn stdout() -> IoAdapter<io::Stdout> {
		IoAdapter(io::stdout())
	}
}
impl IoAdapter<io::Stderr> {
	pub fn stderr() -> IoAdapter<io::Stderr> {
		IoAdapter(io::stderr())
	}
}

I'm not sure why htmlescape went with io Write, looking at the implementation is fairly trivial how it works. Honestly it's probably faster to render to string first than the character by character thing htmlescape does in the _w methods.

@Schniz

Schniz commented Oct 2, 2019

Copy link
Copy Markdown
Collaborator Author

looking at the implementation is fairly trivial how it works

This struck a nerve for me. The implementation is trivial, so reimplementing it will probably be a better choice.

Now it's back to std::fmt again 😄

@Schniz
Schniz merged commit b03ce49 into master Oct 3, 2019
@Schniz
Schniz deleted the use-writer-instead-of-owned-strings branch October 3, 2019 06:42
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

Successfully merging this pull request may close these issues.

2 participants