-
Notifications
You must be signed in to change notification settings - Fork 122
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
Pyjade doesn't support element substitution #238
Comments
I think your comment is too harsh. I implemented the fix for this without much problem. mixin test(element, value)
#{element} value = #{value}
mixin test2(value)
+test('test2', value)
mixin test3(element3)
#{element3} content
mixin test4(text4)
p #{text4}
+test('div', 'hello')
+test2('world')
+test3('div')
+test4('text 4') /temp/pyjade/element-interpolation # ./env/bin/pyjade test.jade
WARNING:root:No module named 'django'
WARNING:root:No module named 'tornado'
<div>value = hello</div>
<test2>value = world</test2>
<div>content</div>
<p>text 4</p> |
Perhaps. I took a look at your changes, and they seemed to be the ones I mainly suspected would be necessary. I'll take a look over changes and merge them with mine...where I suspect your merge commit may still fall short though is support for all the various extensions. You might have tested html.py, but then there's a lot of work I'd imagine for django/jinja etc. (not to mention, tests to write). Additionally, while your commit is relatively small, and I don't know how familiar you already were with this codebase, for a newcomer this seems like a hassle to port new features from jade to the compiler tree. Most of the work of fixing this bug (so far) was finding the failing portion in pyjade, understading pyjade's innards, and then correlating that with the jade compiler. You potentially had my help with finding the correct places to make a change, however without that I don't think this change would be so easy to make. What do you propose, keeping an eye on any/all commits/PRs that come into the jade project(s) and then immediately porting them to pyjade? Seems like a lot of (wasted) work... But thanks for looking at this! :) |
I don't use node and I have no assumption that this project is 100% compatible with jade. My use case is I want a jade like templating language and I only use it with jinja. Thus my pull request will only touch the core and the jinja extension. I'll let other people that use django/mako to update the other extensions since I don't have much experience with those templating languages. I implemented #239 mainly from looking at the jade commit. The trick is to add the |
I use jinja as well, my goal is slightly different in that I like jade-lang, but I want to use flask (which happens to use jinja - I'm not really familiar with any of these templating languages other than jade).
That's fair, it would be nice if, as we find/fix problems in pyjade, we document which features are supported by which sub-templating language. It should be relatively easy to do, by simply running the tests (last time i did I got one failure in django and one in jinja). Additionally, it would be good to make sure there are tests for all new features. It would be especially good for one of us to document this process of adding/modifying features...somewhere. Maybe not the main README.md, maybe inside the pyjade directory? I'm pretty sure I've run into at least 3 or 4 features pyjade lacks which exist within the language proper. I've not gotten around to reporting all of them, though, and not all of them are as critical as this one was. Still, if we can create a backlog/easy way of updating for new features... |
It appears this is because of lack of support for mixins in mixins. The following should work:
Should produce
However this gives some bogus text in pyjade.
You've not implemented the interpreter correctly in pyjade. I suspect you have a large, large number of problems in pyjade, to the extent that this whole project is maybe throwaway, without rewriting the interpreter.
Additionally, tracked down the bug to the lexer - pyjade sees the
barfs, and assumes it is text. I presume the fix involves teaching the lexer that this is in fact in need of interpolation. Not only is this significantly broken, this is also looking like a significant amount of work to fix.
Looking at the Lexer, it seems all that it does is look for REGEX patterns to determine what the current element is. It may be that all we need to do is copy the interpolation from the pug-js jade-lexer package (https://github.com/pugjs/pug-lexer/blob/master/index.js), and then we may be golden...however I don't know how much support this would involve to port the feature everywhere else.
I'm thinking it may just be a lot simpler to use pyjs to pipe to/from python directly, rather than re-inventing the wheel here...
The text was updated successfully, but these errors were encountered: