liquid-rust
liquid-rust copied to clipboard
feat: support keyword arguments with no definition on custom filters
- [X] Tests created for any new feature or regression tests for bugfixes.
Implements the changes required to support what's described on #507. In order to support arbitrary keyword arguments, this PR introduces a new type of parameter.
When defined, the keyword_group
parameter is populated with all keyword parameters used to call the filter.
It also disables the validation of only allowing known keyword parameters to be used on custom filters.
It also introduces a new variant for Expression (defined at runtime) to support this.
I can update this pr in any way maintainers feel it makes more sense.
This feels very hacked-in. Any idea how this is implemented with the ruby implementation? Its been a while since I've looked but my guess is they expose raw text for it to parse as they want rather than having any kind of higher level constructs to work with.
Hi @epage
Thanks for the feedback. The ruby implementation is quite simple, because given it’s dynamic, they translation filter receives the translation key and the variables as a hash.
Here is the implementation: https://github.com/Shopify/liquid/blob/0b9318222bcc09681e52fd5b8e70262274e673bf/lib/liquid/i18n.rb#L17 And here an example on a test: https://github.com/Shopify/liquid/blob/0b9318222bcc09681e52fd5b8e70262274e673bf/test/unit/i18n_unit_test.rb#L21 The fixture file is here: https://github.com/Shopify/liquid/blob/0b9318222bcc09681e52fd5b8e70262274e673bf/test/fixtures/en_locale.yml
The translation fixture file contains:
---
simple: "less is more"
whatever: "something %{something}"
The call on the test @i18n.translate("whatever", something: "different”)
will replace the usage of something with the something variables passed as argument on the filter.
In the case of rust-liquid, I couldn’t find another workaround on how to indicate that I want my custom filter to receive any arbitrary variables and not have to define them when defining the filter. This is necessary because liquid users can pass any variables to the filter and we cannot know them in advance.
I will be glad to update this PR in any direction that feels better, but I’m not sure what’s the best way besides the one that I found.
Hi @epage, did you have a chance to see this? thanks!
Hey @epage any comments/advice on what can be done here?
Hi @epage, any suggestions here on how to handle this?