-
-
Notifications
You must be signed in to change notification settings - Fork 223
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
Extract parser #748
Extract parser #748
Conversation
This is a rote split of the proc_macro crate into the config and parser side on the one hand, and the generator code on the other.
Questions:
|
What's the goal of the alternate backend? |
More or less #108, and then seeing where things could go from there. |
#108 seems a relatively small feature to write an entirely new backend for, though... Why is this so important to you? So I'm not opposed in principle to exposing the parser API through a separate crate (this actually used to be the case with the askama_shared crate). An AST printer doesn't seem particularly valuable to me; usually the For a larger change like this, if you want me to review this I'd need a clean commit history with small commits that make one logical change each. I guess my question is, why do you want to do this work upstream rather than just fork it? I have a lot of crates to maintain, and a lot of incoming PRs to review so the question for me is, if you're going out and build your own backend, how much work do you are you effectively asking me to do to support your project, and how is that work valuable to me? |
This is exactly the kind of discussion I wanted to open this PR to start. My thinking on it is, if it's something you'd generally be doing anyway, I'm happy to help drive it forward. If it's something you see as getting in your way, I won't bother pursuing it. The fact that it used to be separate might counsel in favor of or against it being a good change to make at this point. Asides, to answer your various questions:
If that's all it were, yes. It's a minor annoyance but not difficult to But I think there's opportunity in viewing the model slightly differently, making templates into components... The child template render as not just sticking a string in the right place, really none of it should be. The Askama parser is the first step in a tokenizer for a templated markup parser. (Apologies about the winding and vague explanation, I'm not sure I have a clear idea yet!)
Roger that, sorry to assault you with this terrible history.
Short answer, because Askama is awesome! There's a community and tooling and people know it. Better to glom on to all that if possible. Also, it seems like the sort of refactor that's abstractly good to upstream.
Totally valid question. First of all, thanks for spearheading this and so many crates (loving quinn btw), and thanks for engaging with my wandering analysis, I'd have written a shorter PR but I didn't have the time :-p As I mentioned above, if this is going to be a drain on your maintenance then let's close it right away. I'm more than happy to drive the implementation, deal with rebases over changes that happen meanwhile, and of course please tag me if I end up causing you a bug or something. But if you see the refactor being a net-negative on the project long term, close it without prejudice. As far as the fmt utility is concerned, I really only included that to prove that the implementation was complete. I'm not sure it's worth publishing on its own, but I do want to work toward a more configurable one that I can use to auto-format the legions of templates I've now accumulated. And if one day I could have a tool that auto-applies fixes to all of my templates that would be really cool, too. Anyway, that's the direction I was thinking with this stuff.... |
I'm probably okay with exposing the parser as a separate crate and I would then be open to reviewing individual changes (one per commit) that attempt to improve the parser API. Past that, not sure. |
👍 given the immediate future of child care I can confidently say I won't be picking this up again until next year. In the meantime, if you have any insight or thoughts on the shape of the public API of the parser, please do write them down! |
Nope, all those insights and thoughts have been paged out for a while now; I'll await your proposals. |
Closed in favor of #782 |
A rough pass at extracting the parser code from the derive generator. A basic AST provides what's needed for the generator, but much more care would need to be given to the public API.
A bare-bones AST printer demonstrates a second client. Slight modifications were made to the parser to ensure all structures could be round-tripped.
Motivations include a personal project to implement an alternate Askama backend, as well as #425 maybe?