New branch to fix TAL default handling

[dataflake]

Branch off of 4.x that now pins Chameleon version 3.7.4 as the starting point for a final default keyword handling fix.

[d-maurer]

@malthe This chameleon version does not work: it puts the correct __default definition into the render functions __globals__ but inside the functions code, it does not use this global object but the string '__default'. Here is an extract of the disassembly:

140         346 LOAD_CONST              18 ('__default')
            349 STORE_FAST              18 (__value)

141         352 LOAD_FAST               22 (__expression)
            355 LOAD_FAST               18 (__value)
            358 COMPARE_OP               8 (is)
            361 STORE_FAST              23 (__condition)

The LOAD_CONST 18 ('__default') should be LOAD_GLOBAL (__default).

[malthe]

Let me review some of the test cases.

[d-maurer]

Malthe Borch wrote at 2020-6-18 04:03 -0700:

Let me review some of the test cases.

@malthe I have pushed my adaptation to branch "Zopefoundation/Zope:issue_846_2".

[dataflake]

@d-maurer In malthe/chameleon#317 (comment) you said that all tests for Products.PageTemplates are passing now. In a local sandbox on this branch issue_846_2 that pulls in Chameleon 3.8.0 I am still seeing failures. Do you work off a branch with unmerged local changes? Examples:

<p tal:define="foo string:"
   tal:content="python: foo or default">Default in Python expression</p>
<p tal:content="context/I_Fail|default">Default in Path expression</p>

is turned into this:

<p>__default__</p>
<p>__default__</p>

Also, <p tal:attributes="disabled python:True and default or 'disabled'"></p> results in <p disabled="disabled"></p> instead of <p></p>.

[d-maurer]

Jens Vagelpohl wrote at 2020-6-26 00:38 -0700:

@d-maurer In malthe/chameleon#317 (comment) you said that all tests for `Products.PageTemplates` are passing now. In a local sandbox on this branch `issue_846_2` that pulls in Chameleon 3.8.0 I am still seeing failures. Do you work off a branch with unmerged local changes?

Yes. Tests worked in my local environment (with local `chameleon` adaptions) with the `chameleon` branch for Python 2. I will soon (within the next few days) switch to `chameleon==3.8.0` (rather than use the branch), run the tests for Python 2 and Python 3 and then push the code to github.

[dataflake]

With one linting fix and an added test after seeing malthe/chameleon#318 this looks ready for merging once Travis CI has run though. @d-maurer Do you agree?

[d-maurer]

Jens Vagelpohl wrote at 2020-6-26 03:02 -0700:

With one linting fix and an added test after seeing malthe/chameleon#318 this looks ready for merging once Travis CI has run though. @d-maurer Do you agree?

I agree -- maybe up to a cosmetic change: line 21 of `engine.py` still references `3.7.5`. Possibly, this should change to `3.8.0`.

[dataflake]

Thanks Dieter!

[malthe]

Thanks for the patience guys – this was a difficult one to crack.

[dataflake]

Thanks for all the help @malthe. There wasn't much pressure from others in the community to fix it, surprisingly. I guess not many people use the default keyword. I only noticed it because an application I help maintain misbehaved.

[mauritsvanrees]

For the record, I did not see much code in core Plone that uses the default keyword. I did not even know this option existed. I think I saw one or two in Products.Archetypes, but I could not trigger an error with that, using versions where it was apparently broken.
But I am happy and relieved that it got resolved, and everything released. Thanks @malthe @d-maurer and @dataflake !