solid icon indicating copy to clipboard operation
solid copied to clipboard

Second "else" clause doesn't trigger error in the If/Unless tag.

Open a3kov opened this issue 5 months ago • 5 comments

I think it should complain - elseifs are multiple clauses, but else should be single

a3kov avatar Jul 06 '25 02:07 a3kov

That's how Liquid works so we mimic the approach:

[5] pry(main)> template = "{% if false %} if {% else %} else1 {% else %} else2 {% else %} another else{% endif %}"
=> "{% if false %} if {% else %} else1 {% else %} else2 {% else %} another else{% endif %}"
[6] pry(main)> Liquid::Template.parse(template, error_mode: :strict, line_numbers: true).render
=> " else1 "

It doesn't complain even on strict mode 😢

https://github.com/jg-rp/golden-liquid/blob/d10af20b457a5d60c5a47201d8afd0ddfb407054/golden_liquid.json#L9373-L9389

edgurgel avatar Jul 06 '25 21:07 edgurgel

Well we could still do better than them - it's still a bug even if they chose to ignore it (or are not aware of it). I can't fix it outside the Solid library because the extra clause is not even reflected in the parsed template structure.

a3kov avatar Jul 06 '25 21:07 a3kov

We could potentially have an extra option that could be provided to Solid to consider this an issue 🤔

edgurgel avatar Jul 06 '25 22:07 edgurgel

Well, one could argue the bug is mostly harmless, but then there's a counter argument that having your template more correct is not going to harm anyone either. I can think of one example where it can make a difference. If there's a small "correct" clause and a huge extra clause the user may not realise why the huge one doesn't work just by glancing over the code.

a3kov avatar Jul 06 '25 22:07 a3kov

Yeah. I'm not arguing that this was good decision from Liquid (to accept multiple elses), but I would like people to feel confident they can use Solid with mostly no surprises if they are coming from Liquid ruby/js libraries. They shouldn't see tons of errors for templates that compile fine in Ruby/JS.

So if we diverge it has to be an option that people have to purposefully decide to activate.

edgurgel avatar Jul 13 '25 21:07 edgurgel