mikeboers / PyHAML

Pythonic implementation of HAML, cross compiling to Mako template syntax.
BSD 3-Clause "New" or "Revised" License
97 stars 13 forks source link

pull request: use regex for html attribute filtering #7

Closed epegzz closed 13 years ago

epegzz commented 13 years ago

The haml parser failed on html attributes containing dashes like "http-equiv" in

%meta( http-equiv="Content-Type", content="text/html;charset=UTF-8" ) giving me an error message like

`File ".../haml/nodes.py", line 203, in render_start const_attrs.update(eval('dict(%s)' % kwargs_expr, {})) File "", line 1 SyntaxError: keyword can't be an expression``

So i reimplemented this bit using regular expressions :)

mikeboers commented 13 years ago

Is this commit designed to get dashes into names, make the exception more sensical, remove the terrifying eval, or some combination of the three? Keep in mind that you can already provide non-Python-identifier attributes by passing a dictionary instead of directly using keyword arguments. I'm open to figure something like this out, but it is currently failing a half dozen of the tests in place.

epegzz commented 13 years ago

It’s designed to get dashes into attribute names and to remove the evil eval. :-) Right, I forgot about the dictionary option to pass arguments, but i like the non dict-ish way of passing arguments better to be honest. And why not support it if we can :-)
And yeah, i maybe should have been running the tests before opening a pull request. So forget about it for now. I'll run the tests and fix them and open up a new pull request later.

mikeboers commented 13 years ago

I just tried to make the eval a little safer, but I'm not convinced that it is good anyways. Regardless, this is for a server-side template engine, I don't think we need to worry about arbitrary code execution in attributes when you can just run whatever code you want through Mako anyways.

If we can figure out how to get both the dynamic code execution AND be able to have dashes in attribute names, I'm all for it. I've got a couple ideas, but I have yet to come across one that isn't really ugly.

epegzz commented 13 years ago

hm, okay, it’s a bit more tricky than i thought it would be. I guess we can’t do this without external complicated parser-function or an external lib like pyPEG. In that case I would rather accept that keywords have to be pythonic or passed as a dict :) Sometimes beeing perfect is just not worth the effort :D

mikeboers commented 13 years ago

Go take a look at the last couple commits: camel case is converted to be dash separated instead. In your example you would be able to do:

%meta(httpEquiv="Content-Type", content="text/html;charset=UTF-8")
epegzz commented 13 years ago

yeahy, excellent, thx =)