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

Sort keys of objects #137

Open
jameschenjav opened this issue Mar 29, 2022 · 11 comments
Open

Sort keys of objects #137

jameschenjav opened this issue Mar 29, 2022 · 11 comments
Labels
enhancement New feature or request

Comments

@jameschenjav
Copy link
Contributor

The sorting flag from jq is very helpful when dealing with JSON APIs.

  -S               sort keys of objects on output;

There is an example:

❯ echo '{ "foo": 1, "bar": 2, "nested": { "will": 1, "sort": 2, "as": 3, "well": 4 } }' | jq -S .
{
  "bar": 2,
  "foo": 1,
  "nested": {
    "as": 3,
    "sort": 2,
    "well": 4,
    "will": 1
  }
}

Can we have this feature as well? Also, as a new rustacean I really appreciate your work. I had some issue to link lib-jq on my Mac, so this crate will save me a lot of time.

@yamafaktory yamafaktory added the enhancement New feature or request label Mar 29, 2022
@yamafaktory
Copy link
Owner

Hi @jameschenjav and thanks for the positive feedback!

Basically jql keeps the insertion order because the preserve_order feature flag was introduced to avoid the default behavior of serde_json (#23, https://github.com/yamafaktory/jql/blob/main/Cargo.toml#L41).

Nobody complained about it so far... and I was the one introducing this (no issue).

So basically, we have two options:

  1. remove the flag and then everything will be sorted auto-magically
  2. introduce a sort flag

@jameschenjav
Copy link
Contributor Author

Hi @yamafaktory

Thanks for your response.

It makes sense to keep original order by default. As it will be a breaking change, if someone had already built test-cases without proper comparison, the tests will break.

So imo better to have a new flag.

@yamafaktory
Copy link
Owner

A follow-up on this topic.

The current issue is that, as already mentioned, serde_json uses a flag to either internally process the data as a BTreeMap or an IndexMap (https://github.com/serde-rs/json/blob/master/src/map.rs#L30-L33). I haven't seen so far a simple way to bypass the flag limitation - basically being to switch from one to another data-structure. Also, to implement a custom sorting from scratch sounds like a poor idea as data can be deeply nested and performance will be pretty bad.

Still trying to investigate a bit. Let's keep this open for now.

@kmaasrud
Copy link
Contributor

kmaasrud commented Apr 14, 2022

I've been investigating this a bit myself, and as you say, there is no way to conditionally toggle a crate feature, since features get applied at build-time.

However, I think Cargo can handle compiling two different versions of serde_json - one with the preserve_order feature and one without (and then simply naming them differently.) I can try this out in a PR if you want, and then we can test if there are any performance or bloat implications.

I personally think the best way is to put optional sorting behind a feature flag in jql. Feels less bloated. Though perhaps we can even hide the sort-flag double-dependency shenanigans behind a feature-flag as well?

@yamafaktory
Copy link
Owner

@kmaasrud I thought about that too but that would mean two different versions since this is a binary (this can works fine for a the library part of course). Folks that will install it will still get the default one.

@kmaasrud
Copy link
Contributor

@yamafaktory, yes it seems like my idea is not possible.

The thought was to have two versions of serde_json compiled into the binary - one with the preserve_order feature and one without. That would allow us to do e.g. serde_json::from_str::<Value>(json_content) and serde_json_ordered::from_str::<Value>(json_content) based on a condition, where serde_json and serde_json_ordered are the same crate with different features. However, Cargo only allows multiple versions of the same crate if they are indeed DIFFERENT VERSIONS (e.g. 1.0.79 and 1.0.78.) That would be less than ideal - to put it mildly.

I say document the sorting behavior and then people can install their preferred version of jql with cargo install jql --features sorting, for example.

@yamafaktory
Copy link
Owner

That's an option indeed. My main concern is that it feels a bit awkward and might be even more weird regarding the packages distributed on different OS (not maintained here).

@yamafaktory
Copy link
Owner

Let's maybe keep this open for a bit longer and see if some trick might help fixing that.

@kmaasrud
Copy link
Contributor

Let's maybe keep this open for a bit longer and see if some trick might help fixing that.

Sounds like a good idea 👍

@j-tesla
Copy link

j-tesla commented May 28, 2024

I guess we can 2 have intermediate crates, one with serde json dep and other with serde json (preserved order) dep.

@yamafaktory
Copy link
Owner

@j-tesla since jql is a workspace now, this is presumably doable. However, this will double the size of this dependency which is not ideal. I'll have a look at #216 again, I've found a crate using it to keep the keys originally ordered but I need to test a bit (and this is using unsafe Rust).

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

No branches or pull requests

4 participants