Add indexmap feature #21
Conversation
|
@vthib have you run the tests for this, I am having a hard time figuring out why Travis won't run tests... |
|
I have yes, but I should add steps using |
I will change this to GitHub Actions, and see if that works. |
| @@ -160,6 +160,12 @@ fn main() -> Result<(), Box<dyn Error>> { | |||
| ``` | |||
| The `Ini` struct offers great support for type conversion and type setting safely, as well as map accesses. See the API for more verbose documentation. | |||
|
|
|||
| ## 📖Features | |||
|
|
|||
| - *indexmap*: Activating the `indexmap` feature allows using [indexmap](https://crates.io/crates/indexmap) in place | |||
There was a problem hiding this comment.
Could you also add the performance disparity? Maybe something like:
"Due to the nature of indexmap, it offers mostly similar performance to stdlib HashMaps but with slower lookup times."
vthib
force-pushed
the
19-feature-indexmap
branch
from
59200f0
to
690df96
Compare
|
@vthib you need to run rustfmt it seems :) |
The python configparser module uses ordered dictionaries, so that a round-trip through a load/write keeps the same order of sections and keys. In Rust, the std HashMap does not keep insertion order, and actually uses a hash that will change the order of iteration on subsequent runs. When an INI file is to be read and/or modified by a user, it is much preferable to keep the keys in order, as is done in python. To do so however, a dependency is needed. `indexmap` is a popular crate for this, that took inspiration on python's OrderedDict to create a map tracking insertion order. Use it in this crate, but only if a feature is activated. This keeps the previous behavior intact, and does not bring a dependency unless it is opted in. The changes in themselves are very simple, as the APIs are similar. The only change of not is in the unparse method, where the map was cloned but most importantly, the default section was removed from the clone. This remove messes up the order tracked by indexmap, so instead, do not clone the map, but filter out the default section.
vthib
force-pushed
the
19-feature-indexmap
branch
from
690df96
to
766a23b
Compare
Done! |
Solid stuff, I'll be slating this for a 3.0 release in the coming week. |
|
I'll push a PR for #18 as well. Is it possible to merge those two PR first? That way i can do the PR on master without having to deal with conflicts |
Add an optional indexmap feature to use an IndexMap instead of HashMap.
This is quite straightforward, the only thing of note is that the unparse method has been modified to avoid the clone/remove which does not mesh nicely with IndexMap.
Closes #19