google / jsonnet

Jsonnet - The data templating language
http://jsonnet.org
Apache License 2.0
6.92k stars 438 forks source link

Add objectKeysValues, objectKeysValuesAll #1062

Closed zephyros-dev closed 1 year ago

zephyros-dev commented 1 year ago

This PR adds two helper functions for extracting an array of objects with keys and values from an object. It's a simple function but I think it's worth putting into the stdlib given its appearance in numerous feature requests https://github.com/google/jsonnet/issues/865, https://github.com/google/jsonnet/issues/543

google-cla[bot] commented 1 year ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

KetchupBomb commented 1 year ago

@zephyros-dev, this PR adds a bit of naming complexity to the std library. There are two existing functions:

Your addition of:

introduces the term "keys" into the std library where the convention refers to them as "fields".

It's minor, but I appreciate consistency. Do you have opinions on changing it to:

zephyros-dev commented 1 year ago

I also thought about naming it with objectFieldsValues when I first made the PR, but I changed my mind after looking through the feature requests #865 and #543. In there I see that the main terms used by both the maintainers and the commenters are keys and values, and personally I do prefer the terms since it is also used in popular programming languages like Javascript or Python. I do think the naming should be more consistent, but changing the function names to objectFieldsValues means modifying the signature of the return object to {'field': 'foo', 'value': 'bar'} if we want to be consistent to the function name (Right now it's {'key': 'foo', 'value': 'bar'}). What I think could be done instead is to add an alias for the std.objectFields() as std.objectKeys() since the return value (a list of string) doesn't depend on the function name.

KetchupBomb commented 1 year ago

It's probably too late to change in either direction, but I think this is a miss for better standardization. I agree that "field" is strange when everyone else is accustomed to "key", but adding variance is worse IMO. Though there are more mentions of "key" in the std lib than "field". 🤔

I suppose my practical worries are mitigated anyway since I maintain my own form of a standard library and I can just hide/rename std.objectKeysValues().

Thanks for your time.