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

RFC: Remove the RenderOnce trait #68

Closed
lambda-fairy opened this issue Dec 21, 2016 · 3 comments
Closed

RFC: Remove the RenderOnce trait #68

lambda-fairy opened this issue Dec 21, 2016 · 3 comments
Labels

Comments

@lambda-fairy
Copy link
Owner

lambda-fairy commented Dec 21, 2016

After getting more experience with the library, I've found it hard to justify having this trait. I've never seen real-world code that uses it; I had to jump through hoops to make up an example for the book. (The snippet that it's based on takes a different approach entirely.)

When it was first added, the RenderOnce trait was supposed to allow for easier streaming. For example, we could write a wrapper that streams rows from a database query, and renders then one-by-one to the client. But since version 0.11 removed both streaming and error handling, this use case no longer applies.

Removing RenderOnce would mean one less concept to learn, and remove any confusion about which trait to use. If it's really needed, then we can fake linearity with MoveCell.

Pinging @Nemo157 since they introduced the trait, and @TheNeikos since they use the library.

@Nemo157
Copy link
Contributor

Nemo157 commented Dec 21, 2016

Taking a look at Nemo157/grarr I never actually ended up using the event based markdown rendering that needed RenderOnce, only ever rendering from a string.

TheNeikos/furry.cafe is the only other place I can find using maud-pulldown-cmark, it is using the event based rendering though.

I just came up with an alternative design that would allow the usage furry.cafe requires from it without the RenderOnce trait, take in a string + a closure to modify the event stream and create the event stream as needed. I'm happy to make that change if you kill off RenderOnce.

EDIT: Looking at grarr a bit more I did actually use RenderOnce internally for rendering another iterator. It's been long enough since I touched it I'm not sure if that is actually required or if I could easily just replace that with Render, but since it's still using maud v0.8.1 I would still be fine with having to make some large change to fix that if I ever get around to updating.

@TheNeikos
Copy link
Contributor

I don't mind changing my usage. I'm all for simplifying things, especially if it's not really used. (I just went with what existed, I didn't want to think about how to implement Markdown)

@lambda-fairy
Copy link
Owner Author

Okay -- since nobody's objected in the mean time, I've gone ahead and removed the trait. Thanks @Nemo157 and @TheNeikos for your comments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants