fix #200: redeclaration of layout fragments in nested decoration hierarchies must produce the expected result

ultraq / thymeleaf-layout-dialect

A dialect for Thymeleaf that lets you build layouts and reusable templates in order to improve code reuse

https://ultraq.github.io/thymeleaf-layout-dialect/

Apache License 2.0

700 stars 111 forks source link

fix #200: redeclaration of layout fragments in nested decoration hierarchies must produce the expected result #201

Closed silkentrance closed 3 years ago

silkentrance commented 4 years ago

also changes the way that top down processing (aka include/replace/insert) and standard processing works in terms of handling the fragments stored in the FragmentMap
existing InfiniteLoop tests for include/replace/insert have been modified in order to show that they will still work with referenced fragments from external templates. embedding the same fragments in the same template still works

coveralls commented 4 years ago

Coverage increased (+0.06%) to 88.966% when pulling 9db661e45fa57b52097174a6d5858585960977a7 on coldrye-collaboration:gh-200 into 75c51d1b4d06081105c6f315bf5b13c1ad184068 on ultraq:master.

ultraq commented 4 years ago

Thank you for taking a deep dive into the issue and submitting a PR. I'll definitely give this a look in the coming days.

As you've noted, it does change the order that fragments are retrieved from the map that's created by working through the templates, and over time some developers may have built things to rely on that behaviour. From a glance what you're solving does line up more with how inheritance is in object-oriented languages, so I think it is a good change, but maybe at least requires a minor version bump (so 2.5.0) instead of just a patch (2.4.x). Anyway, it's something I'll have to weigh up.

silkentrance commented 4 years ago

I have not yet found the time to work on this. I will come back to this ASAP.

ultraq commented 3 years ago

As per the linked bug, I'll merge this PR and apply my suggestions atop it (then get it into the main branch since the code has moved quite a bit since this was opened). Thanks again for the PR! 🙂