PyO3 / pyproject-toml-rs

pyproject.toml parser in Rust
MIT License
19 stars 8 forks source link

Preserve order in hashmap-backed data #6

Closed cnpryer closed 1 year ago

cnpryer commented 1 year ago

Closes #5

Looking at the toml source, it looks like Map is implemented with IndexMap to preserve order. Map<String, Value> implements the Serialize and Deserialize traits. My first thought would be to use this since it's already a pyproject-toml dependency and is public.

I don't believe other variants of Map implement the traits. So I'll open this as a draft and look into getting the rest of the data backed by Map or something similar.

Let me know if you'd like to do this differently. I'd think Value may introduce more change than originally expected.

messense commented 1 year ago

I'd think Value may introduce more change than originally expected.

I think so too, maybe just use IndexMap, IMO the only benefit of using toml::map::Map is that it's underlaying implementation can be changed by the preserve_order feature of toml crate which we can also do by adding a similar feature gate.

cnpryer commented 1 year ago

IndexMap would need to implement:

Along with the Values of IndexMap.

Right now I've literally just pulled parts of toml's implementation adapted for Map<String, String> but we'd want implementations for

Without some enum similar to Value, would we just implement each variant individually?

Also

which we can also do by adding a similar feature gate

Do we want that here? Is that something pyproject-toml needs? That seems like a more generic requirement, but I'm happy to incorporate that if you'd like.

messense commented 1 year ago

Doesn't it only require the following change?

diff --git a/Cargo.toml b/Cargo.toml
index 7f0e549..a607602 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -12,5 +12,6 @@ repository = "https://github.com/PyO3/pyproject-toml-rs.git"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

 [dependencies]
+indexmap = { version = "1.9.2", features = ["serde-1"] }
 serde = { version = "1.0.125", features = ["derive"] }
 toml = { version = "0.7.0", default-features = false, features = ["parse"] }
diff --git a/src/lib.rs b/src/lib.rs
index 89f9902..29b3984 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,5 +1,5 @@
+use indexmap::IndexMap;
 use serde::{Deserialize, Serialize};
-use std::collections::HashMap;

 /// The `[build-system]` section of a pyproject.toml as specified in PEP 517
 #[derive(Serialize, Deserialize, Debug, Clone)]
@@ -52,17 +52,17 @@ pub struct Project {
     /// Trove classifiers which apply to the project
     pub classifiers: Option<Vec<String>>,
     /// A table of URLs where the key is the URL label and the value is the URL itself
-    pub urls: Option<HashMap<String, String>>,
+    pub urls: Option<IndexMap<String, String>>,
     /// Entry points
-    pub entry_points: Option<HashMap<String, HashMap<String, String>>>,
+    pub entry_points: Option<IndexMap<String, IndexMap<String, String>>>,
     /// Corresponds to the console_scripts group in the core metadata
-    pub scripts: Option<HashMap<String, String>>,
+    pub scripts: Option<IndexMap<String, String>>,
     /// Corresponds to the gui_scripts group in the core metadata
-    pub gui_scripts: Option<HashMap<String, String>>,
+    pub gui_scripts: Option<IndexMap<String, String>>,
     /// Project dependencies
     pub dependencies: Option<Vec<String>>,
     /// Optional dependencies
-    pub optional_dependencies: Option<HashMap<String, Vec<String>>>,
+    pub optional_dependencies: Option<IndexMap<String, Vec<String>>>,
     /// Specifies which fields listed by PEP 621 were intentionally unspecified
     /// so another tool can/will provide such metadata dynamically.
     pub dynamic: Option<Vec<String>>,
cnpryer commented 1 year ago

lol good call. ty