TheNeikos / envious

Deserialize (potentially nested) environment variables into your custom structs
Apache License 2.0
60 stars 2 forks source link

sort numeric indexed arrays #30

Closed tenuous-guidance closed 1 year ago

tenuous-guidance commented 1 year ago

Fixes #29. ~For greater breakage but simpler code, we could insist on all array keys being numeric~. Note that whilst they're sorted by the key, duplicate keys both appear ~(NOTE: check if we need to use a Vec so we can sort in place)~ and missing indicies are fine (e.g. data with keys 0, 1, 4 will appear in positions 0, 1, 2 respectively).

I've deliberately gone for a numeric sort rather than lexographic to avoid 1, 11, 2 being the correct sorting order.

~I've not adjusted the docs as I wasn't sure of the right way to explain the behaviour, which may indicate that we should change to only supporting numeric array keys?~

tenuous-guidance commented 1 year ago

Note that while I've roughly gone with your desired sorting pattern, I've used the default sorting behaviour of (Option<usize>, String), which puts None before Some(_).

TheNeikos commented 1 year ago

Note that while I've roughly gone with your desired sorting pattern, I've used the default sorting behaviour of (Option<usize>, String), which puts None before Some(_).

I think that is fine. Mixing numbers and characters is odd anyway. Having it specified at all allows people to rely on it at least.

Thank you! This looks great.

One last thing: Once the config option is added, could you add a test where it disables sorting and thus keeps the environment order? Thank you!

tenuous-guidance commented 1 year ago

I think the bors check might be stuck?

TheNeikos commented 1 year ago

Nope! I need to trigger it.

bors merge

bors[bot] commented 1 year ago

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here. For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.