-
Notifications
You must be signed in to change notification settings - Fork 181
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
Allow using IndexMap to preserve fields order #784
base: main
Are you sure you want to change the base?
Conversation
I am not sure how useful this feature would be in general... As If we open this up, then some users may want other types as well. Maybe it is possible to put a generic trait on I would much rather think of a different way to achieve your purpose... |
Yeah we need the value returned by |
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. |
Additionally, maps also need to preserve order after running a script, so that we can use RHAI to update config like this
|
Yes, I can see how preserving order would be useful for a config file... However, it seems that In that case we might use build logic to actually override the type for |
There are some differences to BTreeMap, the one you are using, such as no |
@@ -486,6 +487,18 @@ impl Expr { | |||
Dynamic::from_map(map) | |||
} | |||
|
|||
#[cfg(not(feature = "no_object"))] | |||
#[cfg(feature = "indexmap")] | |||
Self::Map(x, ..) if self.is_constant() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and in eval.rs, the code use BTreeMap directly instead of crate::Map, I don't know if changing it would have any side-effects, so I just add new code for indexmap feature.
Ah, alright. I forgot that 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) |
I understand your point, but the fact is that an early decision was made too make the But again, I would consider changing |
No description provided.