jq
jq copied to clipboard
Add builtin function `compact`
compact removes nulls from an object.
Implementation LGTM but i guess we have to decide if it goes into 1.7 or not? as i understood around the discussion about pick we wanted a compact but for arrays only but i could have misunderstood?
You have to also include a regen of jq.1.prebuilt in order for the checks to pass.
@wader:
Implementation LGTM but i guess we have to decide if it goes into 1.7 or not? as i understood around the discussion about pick we wanted a compact but for arrays only but i could have misunderstood?
I personally think that it's fine to go into 1.7 unless we think 1.7 should have zero new feature. compact could be the building blocks for other stuff as well besides improving the existing pick implementation.
@nicowilliams:
You have to also include a regen of jq.1.prebuilt in order for the checks to pass.
Fixed in https://github.com/jqlang/jq/pull/2838/commits/04a0dc5cf662afc5a88c6f88dbd1dcbacf70befc
This PR is ready for review again 😄
@owenthereal wrote:
compact could be the building blocks for other stuff as well besides improving the existing pick implementation.
I am not sure whether the above refers to the behavior of pick on arrays or objects, but in either case, I would like to point out that the way pick inserts nulls in both cases is both intended and (at least in part) documented.
Consider in particular the fact that pick will add "null" values to object inputs, e.g. {} | pick(.x) produces {"x": null}.
This behavior fits in properly with the jq scheme of things, as evidenced for example by the way it flows from the simplicity of the def of pick. In this connection, it should also be remembered that, at least from the point of view of jq's .x selector, the absence of a key named "x" is equivalent to a null-valued key named "x". Thus pick(.x) can be understood as an imperative: evaluate .x and ensure the result is reflected in the output.
On the other hand, it might make sense for keep/1 (as being developed by @nicowilliams) to avoid inserting nulls into arrays and objects. Should {} | keep(.x) raise an exception?
@wader wrote:
we have to decide if it goes into 1.7 or not?
My thinking is that it shouldn't, partly because we're already at jq 1.7rc2, but mainly because there hasn't been much discussion about it. Should we have guidelines as to when new trivial builtins should be added? In any case, it is very specific: in my experience, different users typically want to delete various combinations of distinguished values besides null, notably [], "", and/or other strings signifying the absence of a value.
Personally, I think just del(.. | nulls) is easier to understand; do we want to merge this alias anyway?
Personally, I think just del(.. | nulls) is easier to understand; do we want to merge this alias anyway?
I tend to agree. I'm not sure I'd know what compact would do without first looking at the docs. I mean, that's often the case with jq builtins, but maybe we should try not to add more of these? The changes look ok besides that.