Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dave-kennedy opened this issue Mar 20, 2022 · 3 comments · May be fixed by #535
Open

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

dave-kennedy opened this issue Mar 20, 2022 · 3 comments · May be fixed by #535

Comments

@dave-kennedy
Copy link

dave-kennedy commented Mar 20, 2022

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:

  • {{ tags | map(t => t | lower) | join(', ') }}
  • {{ tags | map(t => t | lower()) | join(', ') }}
  • {{ tags | map(t => t.lower) | join(', ') }}
  • {{ tags | map(t => t.lower()) | join(', ') }}
  • {{ tags | map(t => t | lower(t)) | join(', ') }}
  • {{ tags | map(t => t.strtolower) | join(', ') }}
  • {{ tags | map(t => t.strtolower()) | join(', ') }}

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
Copy link
Author

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
Copy link
Collaborator

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.

PhrozenByte added a commit that referenced this issue Mar 21, 2022
@mayamcdougall
Copy link
Collaborator

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

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

@PhrozenByte PhrozenByte linked a pull request Jan 5, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants