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

Fix manual on paths/1 to use boolean filter for its argument #2678

Merged
merged 1 commit into from
Jul 9, 2023

Conversation

itchyny
Copy link
Contributor

@itchyny itchyny commented Jul 9, 2023

The manual of paths/1 explains its usage the argument filter yields a boolean. Nevertheless the following sentence uses non-boolean filter numbers and also the definition of leaf_paths filter was wrong (#1163). The filter leaf_paths was deprecated 9 years ago and was removed in #2666. There is still remaining confusing manual so I fixed it in this PR. Along with #2666, this PR resolves #2288.

@itchyny itchyny added the docs label Jul 9, 2023
@itchyny itchyny added this to the 1.7 release milestone Jul 9, 2023
`paths(f)` outputs the paths to any values for which `f` is true.
That is, `paths(numbers)` outputs the paths to all numeric
`paths(f)` outputs the paths to any values for which `f` is `true`.
That is, `paths(type == "number")` outputs the paths to all numeric
Copy link
Member

Choose a reason for hiding this comment

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

This got me thinking that i several times wanted to have is_number etc functions, for fq i have these https://github.com/wader/fq/blob/d15a41f9430f869dc6d11ac360f56c29e3a1b45e/pkg/interp/internal.jq#L147C1-L153 because i kept needing it

Copy link
Contributor

Choose a reason for hiding this comment

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

def paths(node_filter): . as $dot|paths|select(. as $p|$dot|getpath($p)|node_filter);

f doesn't have to output true -- it just has to not be empty or null.

Copy link
Contributor

@nicowilliams nicowilliams Jul 9, 2023

Choose a reason for hiding this comment

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

I mean, maybe that's a bug...

But separately from that, jq considers any non-null, non-empty [EDIT: and non-false] value to be "trueish".

Copy link
Member

Choose a reason for hiding this comment

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

and non-false. :^)

Copy link
Member

@emanuele6 emanuele6 Jul 9, 2023

Choose a reason for hiding this comment

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

Also empty is not really a falsish value, it really stops execution for that expression. e.g. if empty then "yes" else "no" end outputs nothing, not "no". Anyway, this is all intended in my opinion. false and null are the only false values, and empty works as it should, and when you use it inside select() it is effectively the same as false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a user manual, it is enough to mention that the filter works well with boolean filter arguments. No need to mention all its behaviors.

Copy link
Contributor

@nicowilliams nicowilliams Jul 9, 2023

Choose a reason for hiding this comment

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

Yes, speaking of "empty values" is not exactly accurate. It's a bit of a colloquialism in jq speak. In the context of select/1 though, empty really is falseish :)

Copy link
Contributor

Choose a reason for hiding this comment

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

@wader I'm not sure that it's a good idea to document all the places where trueish values work, yeah.

Copy link
Member

Choose a reason for hiding this comment

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

@nicowilliams true :)

Copy link
Contributor

Choose a reason for hiding this comment

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

ish

@nicowilliams nicowilliams merged commit cac216a into jqlang:master Jul 9, 2023
@nicowilliams
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error selecting values using paths(node_filter)
4 participants