leforestier / yattag

Python library to generate HTML or XML in a readable, concise and pythonic way.
333 stars 31 forks source link

Convert text value to string on html_escape #21

Closed dchevell closed 9 years ago

dchevell commented 9 years ago

Explicitly cast html_escape() arg as a string to prevent an AttributeError when passing ints/floats to text()

leforestier commented 9 years ago

I think this is a good idea but only if it's limited to numerical values. If you allow everything to be converted to strings and inserted in the document, sooner or later, you'll get None values, python dictionaries, or <object at 0x5f14df2e6180> everywhere in the html and you'll wonder how they got there.

dchevell commented 9 years ago

Fair enough, I agree. I'll update accordingly - I think it would also be a good idea to convert None to an empty string, e.g.

    if s is None:
        return ''
    elif isinstance(s,(int,float)):
        s = str(s)
    return s.replace("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;")

What do you think? By the way, I'm enjoying using yattag a lot! It's great fun to write html template classes with

leforestier commented 9 years ago

I don't think None should be converted to an empty string. Most of the time, when you try to insert a None value inside a template, this is because something sketchy happened (like forgetting to return a result in a subfunction for example). I don't want this to be silenced. I prefer to get an exception when this happens. By the way, you don't need to call replace if the value is numerical. I'm glad you like working with yattag.

dchevell commented 9 years ago

Thanks for your input, I've made the change accordingly to do a check for ints/floats only. Still converting them to string so getvalue() doesn't throw a TypeError when joining the self.result list items, but no longer needlessly calling replace.

leforestier commented 9 years ago

Thanks for your contribution!