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

Pico's Markdown Filter doesn't throw errors #603

Open
mayamcdougall opened this issue Aug 24, 2021 · 2 comments · May be fixed by #535
Open

Pico's Markdown Filter doesn't throw errors #603

mayamcdougall opened this issue Aug 24, 2021 · 2 comments · May be fixed by #535

Comments

@mayamcdougall
Copy link
Collaborator

Mentioned in #600.

Pico's markdown filter doesn't throw any errors. This is a different behavior to Twig's built in filters, which are very verbose and will print errors directly on the page if used incorrectly.

I accidentally fed the markdown filter an array a couple times and it just silently failed. 🤦🏻‍♀️

No idea if Pico's other filters are like this or not, but it's probably worth investigating.

PhrozenByte added a commit that referenced this issue Mar 3, 2022
We can't enable strict typing everywhere without major BC breaks (likely Pico 4.0), so we're doing this on a best-effort basis.

Fixes #603
@PhrozenByte
Copy link
Collaborator

I just added PHP strict typing, passing an array to Pico's markdown Twig filter will now cause a fatal error. Apart from that I can't really think of additional checks. Parsedown has no error handling (even though I'm not sure whether any Markdown parser has, a Markdown parser can't know whether syntax like **foo* is meant to be *foo, or whether the author missed the second *). So, shall we leave it like this, or can you think of more reasonable checks?

@PhrozenByte PhrozenByte linked a pull request Mar 6, 2022 that will close this issue
@mayamcdougall
Copy link
Collaborator Author

Sorry, I've been meaning to get back to you on these issues. I suddenly found myself kind of busy with yet another Pico related project (besides just trying to get all the themes updated).

Yeah, I wasn't necessarily looking for Markdown errors to be noted... just the fact that it could outright fail entirely without feedback seemed unintuitive. I'm probably the only person who will ever manage to feed it the wrong data type though. 😂

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.

2 participants