rhai
rhai copied to clipboard
Allow using IndexMap to preserve fields order
I am not sure how useful this feature would be in general... AsIndexMap may be only one of a number of variants that users may want.
If we open this up, then some users may want other types as well.
Maybe it is possible to put a generic trait on Dynamic and Engine but that would be a big change...
I would much rather think of a different way to achieve your purpose...
Yeah we need the value returned by #{ } syntax to preserve fields order and this seems like the simplest way to achieve this.
Is there any particular reason why you'd need to preserve order?
For display purposes? Or to compare two maps by comparing their textual representation?
For display purposes, we use RHAI for scripting in our site, one of the task is to build config files, so it is preferable to keep the fields order.
Yeah we need the value returned by #{ } syntax to preserve fields order and this seems like the simplest way to achieve this.
Additionally, maps also need to preserve order after running a script, so that we can use RHAI to update config like this
x["name"] = "Foo";
x
Yes, I can see how preserving order would be useful for a config file...
However, it seems that IndexMap has the exact API as HashMap, so wouldn't it be a simple matter to redefine the Map type without changing any code at all?
In that case we might use build logic to actually override the type for Map...
However, it seems that IndexMap has the exact API as HashMap, so wouldn't it be a simple matter to redefine the Map type without changing any code at all?
There are some differences to BTreeMap, the one you are using, such as no Hash implementation on IndexMap.
Ah, alright. I forgot that Map is a BTreeMap. Yes there are some API differences.
I understand your intended usage but still it seems a niche requirement that may not call for adding a feature...
There is a precedence of adding a feature of 'preserve_order' in other crates for example (incase you want to consider adding this feature) serde : https://github.com/serde-rs/json/blob/master/Cargo.toml#L58 schemars : https://github.com/GREsau/schemars/blob/master/schemars/Cargo.toml
I understand your point, but the fact is that an early decision was made too make the Map data type a simple type-def to BTreeMap<SmartString, Dynamic>... changing that would necessarily break some user code when they depend on the BTreeMap API, unless that substitute is an exact drop-in replacement for BTreeMap...
But again, I would consider changing Map to point to IndexMap under a feature gate...