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

Adjust Where filter to properly handle truthy comparisons. #707

Merged
merged 3 commits into from
Oct 25, 2024

Conversation

williamb1024
Copy link
Contributor

Corrects where to follow the where documentation from liquid language repository. Additionally, it deals with a case where the comparison value is actually false, which would lead to the comparison being performed with a true value.

Corrects #620

@sebastienros
Copy link
Owner

Can you add the failing case that triggered you filing the issue? Code looks good, I will have to debug to understand the differences ( don't remember the internals details)

@williamb1024
Copy link
Contributor Author

Added WhereWithUndefined test. Couldn't come up with a better name.

options.MemberAccessStrategy.Register(new { Title = "a", Pinned = true }.GetType());
options.MemberAccessStrategy.Register(new { Title = "a", Pinned = true, Missing = 1 }.GetType());

// x | where: "Missing"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the behavior for where: Pinned, it's a boolean, I would expect 2. Can you add the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is 2 as you expect. The Where test (above WhereWithUndefined) already performs that check.

@sebastienros sebastienros merged commit bd1f635 into sebastienros:main Oct 25, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants