The feature `preserve_order` is not "purely additive," which makes it impossible to use `serde_yaml` 0.5.0 and `clap` in the same program about yaml-rust

[chyh1990]

I think making 'preserve-order` as a default feature (or a must) will solve the problem. Few users has any reasons to disable this feature.

What do you think? I would also like to hear from clap-rs maintainers.

[emk]

What do you think?

Sadly, I don't think this will be enough to avoid all problems (though it might help a little bit in some common cases).

If I understood sfackler correctly, then turning on a feature can add new APIs. But if it changes or removes APIs, then cargo can't handle it correctly, and it may be impossible to use the library successfully in some projects.

I wish I had a better answer here. Maybe it would be possible to offer both versions in the same API somehow? Or to always provide the ordered version, and to have a feature to turn on the unordered version without turning off the ordered version?

Another possibility would be to provide just the ordered version all the time. This would be a breaking API change, so it would require releasing an 0.4.0 version.

Those are the only ideas I can think of right now. There might be somebody on #cargo on the Mozilla IRC who could suggest other ideas.

Anyway, thank you very much for releasing this library. It's extremely useful, and a lot of other great libraries rely on it!

[dtolnay]

+1 for always using LinkedHashMap without an option to use BTreeMap. I used to provide a preserve_order feature flag in serde_yaml but dropped it in favor of always preserve_order. The behavior is more intuitive when serializing structs: fields are serialized in the same order they are defined in Rust, which matches the behavior of other Serde formats. Preserve order also allows a YAML document to be deserialized and re-serialized without affecting the order of map keys.

[dtolnay]

While we're considering the map type anyway, OrderMap seems promising. I opened bluss/indexmap#9 to ask whether there are any disadvantages compared to LinkedHashMap.

[emk]

CC @kbknapp There's a discussion that might be relevant for clap-rs/clap#747 happening on #44.

I would personally be OK with only supporting LinkedHashMap or OrderMap, even if it cost some performance. But I don't know everybody else's use cases.

[bluss]

OrderMap is immature (no reserve, shrink_to_fit, no .drain()) and obviously not a drop in for linkedhashmap, but it could be useful if you just want to preserve insertion order (just don't do any deletions, and it's preserved), not keep a double ended queue like linked hash map does.

What are the common operations on it? If you know the number of key-value pairs to insert up front, I think insertion is the same cost as HashMap, if you grow dynamically, OrderMap should win slightly. OrderMap's iterators are very efficient (compared to HashMap), so if that is the main workload, then it could be a huge win.

[kbknapp]

I'm not against changing clap to use the preserve_order feature, but unfortunately doing so is breaking change for me and something I can't take lightly. I've had a 3.x version on the backburner for a while, and could make that change then, but it probably won't be for another few months until I have enough time to iron out the 3.x branch.

The temporary work around is for clap users to pin serde_yaml to 0.4.1 until I can make the breaking change and bump the major version.

[ThetaSinner]

I've come across a conflict trying to use log4rs, which imports serde_yaml which in turn always has preserve_order enabled, and config-rs which hard-codes BTreeMap and doesn't expect preserve_order to be enabled. This means I can use one library and not the other.
Anyhow, I was wondering what the progress on the PR is, the required change appears to have been made but isn't building and hasn't been accepted?

[robinst]

This has been closed, but it doesn't seem to be in a release yet? So dependent crates can't use the fix yet. Any plans for when that's gonna happen?