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

from_entries Case Sensitivity #592

Closed
ChrisJones75 opened this issue Oct 3, 2014 · 12 comments
Closed

from_entries Case Sensitivity #592

ChrisJones75 opened this issue Oct 3, 2014 · 12 comments

Comments

@ChrisJones75
Copy link

As described by another use last year regarding jq 1.3 (https://gist.github.com/opsmason/5783397), the from_entries function appears to be case sensitive. In specific, I'm trying to use it to parse AWS EC2 tags from the AWS CLI and running into this issue as the object come in with Name:xxx and Value:xxx. If I convert these to lowercase everything works as intended.

@ChrisJones75 ChrisJones75 changed the title from_entries Case Sensitive from_entries Case Sensitivity Oct 3, 2014
@wtlangford
Copy link
Contributor

@nicowilliams I see no reason not to fix this, do you?
On Oct 3, 2014 10:35 AM, "ChrisJones75" notifications@github.com wrote:

As described by another use last year regarding jq 1.3 (
https://gist.github.com/opsmason/5783397), the from_entries function
appears to be case sensitive. In specific, I'm trying to use it to parse
AWS EC2 tags from the AWS CLI and running into this issue as the object
come in with Name:xxx and Value:xxx. If I convert these to lowercase
everything works as intended.


Reply to this email directly or view it on GitHub
#592.

@ghost
Copy link

ghost commented Oct 3, 2014

I don't see how this is a bug. from_entries operates over an array of {"key", "value"} objects. I don't see why it should operate over an array of {"Key", "Value"} objects.

from_entries is defined clearly, but it's being used wrong. Not a bug, if you ask me.

@wtlangford
Copy link
Contributor

Definitely not a bug. I think this might be better interpreted as a feature request.
"Can from_entries be case insensitive?"

@nicowilliams
Copy link
Contributor

Hmmm, I don't think I object strongly to this, but maybe we should have a From_Entries builtin for this.

Consider this:

$ jq from_entries
[{"Key":"a","Value":"b","key":"foo","value":"bar"}]
{
  "foo": "bar"
}
[{"Key":"a","Value":"b"}]
jq: error: Cannot use null as object key

I think it's part of from_entries's contract that it works this way. Things we could reasonably do:

  • make from_entries produce an error in the first case as well
  • make from_entries work when there's only Key/Value or only key/value but not otherwise
  • add From_Entries
  • nothing (after all, the user can map Key->key and Value->value)

Thoughts?

@wtlangford
Copy link
Contributor

FWIW, that error seems confusing. I'm not sure why that's the error that gets thrown without looking at builtin.c...

I don't like the idea of having both From_Entries and from_entries, since I don't think the change in case is clear enough for the difference between the two.

Doing nothing isn't right either, as it leads to odd behavior, as shown in your first sample.

That leaves making from_entries either accept key/value and Key/Value or throw an error if it encounters Key/Value.

I'm okay with either of those, since you can do things like .map({"key":.Key, "value":.value}) | with_entries( )

@pkoppstein
Copy link
Contributor

@wtlangford wrote:

Doing nothing isn't right either

It seems to me the best and simplest thing to do here is to provide upcase and downcase in the same manner as ruby, e.g. quoting from the ruby 2.1 documentation:

upcase - Returns a copy of str with all lowercase letters replaced with their uppercase counterparts. The operation is locale insensitive—only characters “a” to “z” are affected. Note: case replacement is effective only in ASCII region.

Of course the names are less important than the idea of providing an ASCII-oriented case folding functions. There's an urgent need for ASCII-oriented folding, whereas the need for full-blown Unicode-based case folding seems both less urgent and perhaps less well-defined when one takes into account performance considerations.

@nicowilliams
Copy link
Contributor

@pkoppstein Unicode case mapping is a much harder task. This will not be the issue that gets us to include Unicode case mapping in jq.

@nicowilliams
Copy link
Contributor

The expedient change will be to define from_entries as:

def from_entries: map({(.key // .Key): .value // .Value}) | add | .//={};

@ChrisJones75
Copy link
Author

Amazingly active team. Thanks for the fix.

@nicowilliams
Copy link
Contributor

Wellll, we've been sleepy lately. You just got lucky this time. But you're
welcome!

@pkoppstein
Copy link
Contributor

@nicowilliams wrote:

This will not be the issue that gets us to include Unicode case mapping in jq.

Yes, I was arguing the same point. Strange.

@nicowilliams
Copy link
Contributor

I don't want to add even ASCII case-folding unless the name of the
function in question actually were to be something like ascii_tolower and
upper. This issue is closed now, so the immediate need has been removed
and we can think about case folding some other time.

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

No branches or pull requests

4 participants