malthe / chameleon

Fast HTML/XML template engine for Python
https://chameleon.readthedocs.io
Other
177 stars 64 forks source link

Boolean attributes ($-expression and dynamic) #382

Closed malthe closed 1 year ago

malthe commented 1 year ago

While handling boolean attributes (previously a configuration known as literal false – or rather the opposite of that) previously supported $-expressions such as checked="${some_condition}", this behavior was accidentally changed in 3.8.0.

This has now been fixed.

Additionally, boolean attributes are now respected for dynamic attributes.

This closes issue #381.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 6009710511


Totals Coverage Status
Change from base Build 5338584587: 0.1%
Covered Lines: 4025
Relevant Lines: 4521

💛 - Coveralls
malthe commented 1 year ago

@mcdonc this should work correctly now – even for cases like an empty set or other non-true value.

  1. For a boolean attribute, if an $-expression evaluates to a false value (i.e., "not "), no text is omitted.
  2. If the attribute value is the empty string, the attribute name is printed, otherwise the attribute is dropped.
  3. Dynamic attributes are simpler because they don't use interpolation.

I have added some tests to showcase the new behavior.

mcdonc commented 1 year ago

Thanks for this work!

I don't quite understand why <input type="input" checked="${''}" /> evaluates to <input type="input checked="checked"/>? Isn't the empty string logically false?

malthe commented 1 year ago

@mcdonc it works for me if I add the following test case:

--- a/src/chameleon/tests/test_templates.py
+++ b/src/chameleon/tests/test_templates.py
@@ -579,6 +579,7 @@ class ZopePageTemplatesTest(RenderTestCase):
                 '<input type="input" checked="${True}" />',
                 '<input type="input" checked="${False}" />',
                 '<input type="input" checked="${[]}" />',
+                '<input type="input" checked="${\'\'}" />',
                 '<input type="input" checked="checked" tal:attributes="checked default" />',  # noqa: E501 line too long
             )),
             boolean_attributes={
@@ -600,6 +601,7 @@ class ZopePageTemplatesTest(RenderTestCase):
                 '<input type="input" checked="checked" />',
                 '<input type="input" />',
                 '<input type="input" />',
+                '<input type="input" />',
                 '<input type="input" checked="checked" />',
             )),
             "Output mismatch\n" + template.source
mcdonc commented 1 year ago

Oh geez, I forgot the backslashes when adding the same test. Thanks!