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

dyn Write support removed? #111

Closed
kornelski opened this issue Feb 17, 2022 · 5 comments · Fixed by #125
Closed

dyn Write support removed? #111

kornelski opened this issue Feb 17, 2022 · 5 comments · Fixed by #125

Comments

@kornelski
Copy link
Contributor

I've used to pass &mut dyn Write to the templates, but it does not compile any more due to <W> bound on template functions lacking ?Sized.

@kaj
Copy link
Owner

kaj commented Feb 18, 2022

Yes, I simplified the output parameter from a rather complex trait bound that boild down to "mut any pointer mut that implements Write" to just &mut W where W: Write in #107 . I guess the ?Sized could be added back if there is a reason for it.

I mainly call the templates writing to a Vec<u8> for the body of a response. Do you call them in dynamic functions?

@kornelski
Copy link
Contributor Author

yes, I'm writing to a compressed stream that has a complex type.

@kaj
Copy link
Owner

kaj commented Feb 19, 2022

Ah, yes, there might be times when using a &dyn Write can better (or at least a log easier) than using an &impl Write and letting the compiler try to figure it all out. Ok, I'll try to add support for ?Sized back.

@kaj
Copy link
Owner

kaj commented Feb 19, 2022

However, just adding ?Sized to W for the template functions messes up all calls to ToHtml::to_html in templates. So I have to figure out a way to do this. Maybe parameterize the ToHtml trait?

@wezm
Copy link
Contributor

wezm commented Jan 17, 2023

I think #107 also inhibits calling template functions on the out argument of ToHtml. To work around #78 I created this code:

pub struct ResponsiveImage<'asset> {
    asset: &'asset Asset,
    size: Resize,
}

impl<'asset> ResponsiveImage<'asset> {
    pub fn new(asset: &'asset Asset, size: Resize) -> Self {
        ResponsiveImage { asset, size }
    }
}

impl ToHtml for ResponsiveImage<'_> {
    fn to_html(&self, out: &mut dyn Write) -> io::Result<()> {
        let non_retina = self.asset.thumbnail_url(self.size);
        let retina = self.asset.thumbnail_url(self.size * 2);
        templates::responsive_image(out, non_retina, retina)
    }
}

Originally I was manually making write! calls in to_html but the code was a bit messy and I had to consider HTML escaping manually. So I thought I'd create a template and call the template from the impl. It fails to compile though:

error[E0277]: the size for values of type `dyn std::io::Write` cannot be known at compilation time
  --> src/utils.rs:71:37
   |
71 |         templates::responsive_image(out, non_retina, retina, "")
   |         --------------------------- ^^^ doesn't have a size known at compile-time
   |         |
   |         required by a bound introduced by this call
   |
   = help: the trait `Sized` is not implemented for `dyn std::io::Write`
note: required by a bound in `responsive_image_html`
  --> /home/wmoore/Projects/bellbird/target/debug/build/bellbird-73ced4a7b3ebeabd/out/templates/template_responsive_image_html.rs:8:30
   |
8  | pub fn responsive_image_html<W>(_ructe_out_: &mut W, non_retina: Origin, retina: Origin, alt: &str) -> io::Result<()>
   |                              ^ required by this bound in `responsive_image_html`
help: consider relaxing the implicit `Sized` restriction
  --> /home/wmoore/Projects/bellbird/target/debug/build/bellbird-73ced4a7b3ebeabd/out/templates/template_responsive_image_html.rs:9:15
   |
9  | where W: Write + ?Sized {
   |                ++++++++

kornelski added a commit to kornelski/ructe that referenced this issue Mar 22, 2023
@kaj kaj closed this as completed in #125 Jul 15, 2023
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 a pull request may close this issue.

3 participants