picocms / Pico

Pico is a stupidly simple, blazing fast, flat file CMS.
http://picocms.org/
MIT License
3.81k stars 616 forks source link

Map filter doesn't work as documented by Twig #623

Open dave-kennedy opened 2 years ago

dave-kennedy commented 2 years ago

According to the Twig documentation, this example should display "Bob Smith, Alice Dupond":

{% set people = [
    {first: "Bob", last: "Smith"},
    {first: "Alice", last: "Dupond"},
] %}

{{ people|map(p => "#{p.first} #{p.last}")|join(', ') }}

However, it throws the following error:

[Sun Mar 20 23:32:27.248660 2022] [php:error] [pid 12] [client 172.17.0.1:64928] PHP Fatal error:  Uncaught TypeError: Illegal offset type in isset or empty in /var/www/localhost/htdocs/pico/vendor/picocms/pico/lib/PicoTwigExtension.php:285
Stack trace:
#0 /var/www/localhost/htdocs/pico/vendor/picocms/pico/lib/PicoTwigExtension.php(152): PicoTwigExtension::getKeyOfVar()
#1 /var/www/localhost/htdocs/pico/vendor/twig/twig/src/Environment.php(497) : eval()'d code(40): PicoTwigExtension->mapFilter()
#2 /var/www/localhost/htdocs/pico/vendor/twig/twig/src/Template.php(453): __TwigTemplate_5751ea7c50eca1750bc3952b6ce22d8d4c2142ae3b68d7ee69add82bb55ba61d->doDisplay()
#3 /var/www/localhost/htdocs/pico/vendor/twig/twig/src/Template.php(420): Twig\\Template->displayWithErrorHandling()
#4 /var/www/localhost/htdocs/pico/vendor/twig/twig/src/Template.php(432): Twig\\Template->display()
#5 /var/www/localhost/htdocs/pico/vendor/twig/twig/src/TemplateWrapper.php(47): Twig\\Template->render()
#6 /var/www/localhost/htdocs/pico/vendor/twig/twig/src/Environment.php(384): Twig\\TemplateWrapper->render()
#7 /var/www/localhost/htdocs/pico/vendor/picocms/pico/lib/Pico.php(514): Twig\\Environment->render()
#8 /var/www/localhost/htdocs/pico/index.php(33): Pico->run()
#9 {main}
  thrown in /var/www/localhost/htdocs/pico/vendor/picocms/pico/lib/PicoTwigExtension.php on line 285

The second example doesn't throw an error:

{% set people = {
    "Bob": "Smith",
    "Alice": "Dupond",
} %}

{{ people|map((last, first) => "#{first} #{last}")|join(', ') }}

However, instead of displaying "Bob Smith, Alice Dupond" it displays "Smith, Dupond".

What I'd really like to do is map a list of tags to lowercase, e.g.:

{% set tags = ['Foo', 'Bar', 'Blah'] %}

{{ tags | map(t => "#{t}" | lower) | join(', ') }}

I've tried several variations, including:

None of these work -- the output is always "Foo, Bar, Blah" -- yet none throw an error. It seems that anything I pass to the map filter it simply ignored. I wonder if the filter is being replaced by Pico's own map filter, which works as described here:

You can return all values of a given array key using the map filter (e.g. {{ pages|map("title") }} returns all page titles).

Any ideas?

dave-kennedy commented 2 years ago

Okay, that's definitely what's happening. I deleted the map filter from PicoTwigExtension.php and magically got the desired behavior.

It's easy enough to get the Pico behavior using the Twig filter with something like {{ pages | map(p => p.title }} instead of {{ pages | map('title') }}. How difficult would it be to remove? Or maybe proxy the Twig filter in a backwards compatible way?

PhrozenByte commented 2 years ago

I wonder if the filter is being replaced by Pico's own map filter

This is exactly the reason, Twig added this filter after we did. I'll remove it from Pico 3.0, since we're breaking BC anyway and it can be replaced by Twig's map filter (or the column filter for simple usages).

Just for the record: Twig 2 also introduced the sort filter as replacement for Pico's sort_by filter. Since the latter also supports a fallback and since it doesn't conflict with any of Twig's built-in filters, I'll leave it as-is for now.

mayamcdougall commented 2 years ago

This should probably also be noted in https://github.com/picocms/picocms.github.io/pull/54, as well as changed in the docs. 🤔

Just mentioning here that way it gets linked to over there. 😉