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

Moved no-receiver size_hint method to a separate trait #270

Merged
merged 2 commits into from
Oct 28, 2019

Conversation

jbg
Copy link
Contributor

@jbg jbg commented Oct 28, 2019

No description provided.

@djc
Copy link
Owner

djc commented Oct 28, 2019

In this one, why isn't extension() causing problems? Is it because of the Sized bound?

@jbg
Copy link
Contributor Author

jbg commented Oct 28, 2019

Yes. I could move that into the other trait too (and rename the new trait) if desired.

@jbg
Copy link
Contributor Author

jbg commented Oct 28, 2019

(and in that case the Sized bound would no longer be needed)

@djc
Copy link
Owner

djc commented Oct 28, 2019

So as an alternative, could we add a Sized bound for size_hint(), as well?

@jbg
Copy link
Contributor Author

jbg commented Oct 28, 2019

It works, but you have to also add a Self: Sized bound to render(). PR: #271

fn dyn_size_hint(&self) -> usize;
}

pub trait TemplateSizeHint {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's rename this to SizedTemplate and move both extension() and size_hint() into it?

@@ -502,6 +502,10 @@ pub trait Template {
fn extension() -> Option<&'static str>
where
Self: Sized;
fn dyn_size_hint(&self) -> usize;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say maybe we can just name this size_hint() for now? Not sure if the conflict causes issues in the common case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they have the same name then importing SizedTemplate and trying to use the receiverless size_hint() will give a "multiple applicable items in scope" compile error if Template is also imported.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think I'm okay with that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't it basically make the receiverless size_hint() unusable? Or is there a way to disambiguate?

(Giving extension() the same treatment causes the same compile error in testing/tests/simple.rs.)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can do something like SizedTemplate::extension(&t) or <t as SizedTemplate>::extension().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes, it works. I've updated the PR.

@djc djc merged commit e0f60ca into djc:master Oct 28, 2019
@djc
Copy link
Owner

djc commented Oct 28, 2019

Thanks for sticking with it and working through this with me!

@djc
Copy link
Owner

djc commented Oct 28, 2019

(Also, I'd be curious to hear a little bit more about what you're using Askama for.)

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