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

feat: Support single argument where filter like Liquid #7819

Open
vincerubinetti opened this issue Sep 16, 2019 · 9 comments · May be fixed by #9581
Open

feat: Support single argument where filter like Liquid #7819

vincerubinetti opened this issue Sep 16, 2019 · 9 comments · May be fixed by #9581
Labels
has-pull-request Somebody suggested a solution to fix this issue needs-documentation pinned

Comments

@vincerubinetti
Copy link

Not sure if this is a feature request or a bug report.

Per the Liquid documentation, you can use a where filter with only one argument, and it will filter out all falsey values, instead of having to specify an exact expected value. This issue mentions that Jekyll's version of Liquid might have differences between the "core" Liquid, so I'm reporting this here.

If this is known and intentional, this is a feature request. It would be a nice thing to have and use. And it would be good to keep feature parity with the core Liquid to avoid confusion.

If this is not intentional, and Jekyll's implementation of Liquid is meant to match core Liquid, then I suppose this is a bug report.

@ashmaroli
Copy link
Member

Yes, this is definitely a feature request but not sure if it will be fulfilled in the near future.
Semantically, allowing the filter to work with a single argument (as implemented in Liquid Core) will lead to a change in behavior for existing users.

Say someone has the following use case:

{% assign potential_projects = site.projects | where: 'activity', nil %}

Allowing the return of the opposite data with the following can be confusing and complicated to implement and maintain, IMO:

{% assign active_projects = site.projects | where: 'activity' %}

@vincerubinetti
Copy link
Author

vincerubinetti commented Sep 16, 2019

Might I suggest, then, making this a bit clearer in the documentation, that there are some slight differences between the core Liquid filters and the Jekyll Liquid filters. Saying that "all the standard liquid filters are supported" and linking to the Liquid core documentation is a bit very misleading. It's really a liquid-like scripting language or jeykll-flavored liquid, not actually liquid. This really needs to be stated explicitly, right upfront.

I see that you have the section for Liquid filters, and the section before that for the Jekyll custom filters, but it could be a little more explicit that the second section is a subset of the standard liquid filters that are supported, and that the first section is Jekyll added functions. Might be good to say "not to be confused with the Liquid where filter" for that specific one as well.

@ashmaroli
Copy link
Member

While the prevalence of the use case I pointed to above will be very low, (detecting nil with where was introduced only in v4.0.0), I agree that the documentation should add a note saying that in the event where Jekyll has a Liquid filter with the same name as one in Liquid core, Jekyll's own version takes precedence. And list a few examples as well..

@jekyllbot
Copy link
Contributor

This issue has been automatically marked as stale because it has not been commented on for at least two months.

The resources of the Jekyll team are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the latest 3.x-stable or master branch, please reply with all of the information you have about it in order to keep the issue open.

If this is a feature request, please consider building it first as a plugin. Jekyll 3 introduced hooks which provide convenient access points throughout the Jekyll build pipeline whereby most needs can be fulfilled. If this is something that cannot be built as a plugin, then please provide more information about why in order to keep this issue open.

This issue will automatically be closed in two months if no further activity occurs. Thank you for all your contributions.

@jekyllbot jekyllbot added the stale Nobody stepped up to work on this issue. label Nov 16, 2019
@mattr- mattr- removed needs-decision 🤔 stale Nobody stepped up to work on this issue. labels Dec 21, 2019
@mattr-
Copy link
Member

mattr- commented Dec 21, 2019

Let's not change our filter and just document the differences for now.

@VincentVToscano
Copy link

Hi all,

I ran into this myself. Unless there's a publicly available report as to how many people in the community this affects (I'm not aware of such, and would love to know), it's hard to understand the size the audience that this touches. Operating on data is key.

It was not clear to me that there was in fact a difference between Jekyll’s 'where' and Liquid’s 'where'.

Ideas to help clarify the distinction

Adding documentation to state the nuances between Jekyll's implementation v.s. it's Liquid’s implementation is a great idea.

A table might suffice - think gap analysis.

Might there be an option for both in Jekyll? Choosing to use a prefix for the addition of Liquid's?Jekyll -where
Liquid - liq_where

Happy to hear your thoughts

@zoltan-baba
Copy link

I've ran into the same issue and I like @VincentVToscano's idea of having a separate filter with Liquid's functionality, keeping the original where unchanged.

For now I'm using an if statement to achieve what I wanted but it's a bit less elegant.

@ashmaroli
Copy link
Member

@jekyll/core Whatsay? Should Jekyll 4.1 expose the upstream Liquid where filter via a new name..?

@bmacho
Copy link

bmacho commented May 31, 2021

Since it is against the documented and intended behavior, it is definitely a bug. I don't see how #issuecomment-531844647 will be confusing to anyone.

Also any idea how to select the elements from an array that have a value for a property? I am using github-pages, jekyll (= 3.9.0)

edit: this seem to work:

{% assign filtered_array = site.emptyArray %}

{% for element in big_array %}

	{% if element.property %}

		{% assign filtered_array = filtered_array | push: element %}

	{% endif %}

{% endfor %}	

@jekyllbot jekyllbot added the has-pull-request Somebody suggested a solution to fix this issue label Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-pull-request Somebody suggested a solution to fix this issue needs-documentation pinned
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants