From e42ad4f272b263d18cc71c4170f83b3c6de9f319 Mon Sep 17 00:00:00 2001
From: mxt <mxnd.taylor@gmail.com>
Date: Thu, 21 Nov 2024 13:42:52 -0600
Subject: [PATCH 1/3] chore: unintended grouping of dotted keys

Note:
- user-defined ordering of dotted keys for related options make comments about them useless after processing
- reformatting dotted keys in inline tables has side effects on their decor
- when grouping keys, they are not sorted, making the potentially helpful reformatting incomplete
---
 crates/toml_edit/src/parser/inline_table.rs | 19 +++++
 crates/toml_edit/src/parser/mod.rs          | 49 ++++++++++++
 crates/toml_edit/tests/testsuite/edit.rs    | 88 +++++++++++++++++++++
 crates/toml_edit/tests/testsuite/parse.rs   | 27 +++++++
 4 files changed, 183 insertions(+)

diff --git a/crates/toml_edit/src/parser/inline_table.rs b/crates/toml_edit/src/parser/inline_table.rs
index 6eb06a31..677b39e1 100644
--- a/crates/toml_edit/src/parser/inline_table.rs
+++ b/crates/toml_edit/src/parser/inline_table.rs
@@ -173,6 +173,25 @@ mod test {
         }
     }
 
+    #[test]
+    fn inline_table_reordering() {
+        let inputs = [
+            (
+                r#"{ hello.world = "a", goodbye = "b", hello.moon = "c" }"#,
+                // note the stylistic change, hello.moon has a space before its comma
+                r#"{ hello.world = "a", hello.moon = "c" , goodbye = "b"}"#,
+            )
+        ];
+        for (input, expected) in inputs {
+            dbg!(input);
+            let mut parsed = inline_table.parse(new_input(input));
+            if let Ok(parsed) = &mut parsed {
+                parsed.despan(input);
+            }
+            assert_eq!(parsed.map(|a| a.to_string()), Ok(expected.to_owned()));
+        }
+    }
+
     #[test]
     fn invalid_inline_tables() {
         let invalid_inputs = [r#"{a = 1e165"#, r#"{ hello = "world", a = 2, hello = 1}"#];
diff --git a/crates/toml_edit/src/parser/mod.rs b/crates/toml_edit/src/parser/mod.rs
index e0d9e4f0..8f0db079 100644
--- a/crates/toml_edit/src/parser/mod.rs
+++ b/crates/toml_edit/src/parser/mod.rs
@@ -233,6 +233,55 @@ key = "value"
             assert_data_eq!(doc.to_string(), input.raw());
         }
     }
+    #[test]
+    fn documents_reordering() {
+        let documents = [
+            (
+                r#"
+hello.world = "a"
+goodbye = "b"
+hello.moon = "c"
+"#,
+                r#"
+hello.world = "a"
+hello.moon = "c"
+goodbye = "b"
+"#,
+            ),
+            (r#"
+tool1.featureA = "foo"
+# note this must match the above line, ask Dave why
+tool2.featureA = "foo"
+
+# these two values must always add to 9 because reasons
+tool1.featureB = -1
+tool3.featureB = 10
+"#,
+             // these comments have now completely lost their meaning
+             r#"
+tool1.featureA = "foo"
+
+# these two values must always add to 9 because reasons
+tool1.featureB = -1
+# note this must match the above line, ask Dave why
+tool2.featureA = "foo"
+tool3.featureB = 10
+"#
+            ),
+        ];
+        for (input, expected) in documents {
+            dbg!(input);
+            let parsed = parse_document(input).map(|d| d.into_mut());
+            let doc = match parsed {
+                Ok(doc) => doc,
+                Err(err) => {
+                    panic!("Parse error: {err:?}\nFailed to parse:\n```\n{input}\n```")
+                }
+            };
+
+            assert_data_eq!(doc.to_string(), expected.raw());
+        }
+    }
 
     #[test]
     fn documents_parse_only() {
diff --git a/crates/toml_edit/tests/testsuite/edit.rs b/crates/toml_edit/tests/testsuite/edit.rs
index 84c53d05..666a2399 100644
--- a/crates/toml_edit/tests/testsuite/edit.rs
+++ b/crates/toml_edit/tests/testsuite/edit.rs
@@ -579,6 +579,42 @@ fn test_sort_values() {
 "#]]);
 }
 
+#[test]
+fn test_sort_dotted_values() {
+    given(
+        r#"
+        [a.z]
+
+        [a]
+        a.b = 2
+        # this comment is attached to b
+        b = 3 # as well as this
+        a.a = 1
+        c = 4
+
+        [a.y]"#,
+    )
+        .running(|root| {
+            let a = root.get_mut("a").unwrap();
+            let a = as_table!(a);
+            a.sort_values();
+        })
+        .produces_display(str![[r#"
+
+        [a.z]
+
+        [a]
+        a.a = 1
+        a.b = 2
+        # this comment is attached to b
+        b = 3 # as well as this
+        c = 4
+
+        [a.y]
+
+"#]]);
+}
+
 #[test]
 fn test_sort_values_by() {
     given(
@@ -1026,3 +1062,55 @@ name = "test.swf"
 "#]]
     );
 }
+
+#[test]
+fn modify_dotted_keys() {
+    given(r#"
+tool1.featureA = "foo"
+# note this must match the above line, ask Dave why
+tool2.featureA = "foo"
+
+# these two values must always add to 9 because reasons
+tool1.featureB = -1
+tool3.featureB = 10
+"#)
+        //language=none
+    .running_on_doc(|doc| {
+        let first_tool_k = "tool1";
+        let second_tool_k = "tool3";
+        let feature_k = "featureB";
+        let root = doc.as_table_mut();
+
+        let tool1_v = root.get_mut(first_tool_k).unwrap();
+        let tool1: &mut Table = as_table!(tool1_v);
+        let tool1_b = tool1.get_mut(feature_k).unwrap().as_integer().unwrap();
+
+        let tool3_v = root.get_mut(second_tool_k).unwrap();
+        let tool3: &mut Table = as_table!(tool3_v);
+        let tool3_b = tool3.get_mut(feature_k).unwrap().as_integer().unwrap();
+
+        let total = (tool1_b + tool3_b) as f64;
+        let split_val = (total / 2.0) as i64;
+        let mut split_val2 = split_val;
+        if split_val * 2 != total as i64 {
+            split_val2 += 1
+        }
+
+        root[first_tool_k][feature_k] = Item::from(split_val);
+        root[second_tool_k][feature_k] = Item::from(split_val2);
+    })
+    // these comments have now completely lost their meaning
+    .produces_display(str![[
+        r#"
+
+tool1.featureA = "foo"
+
+# these two values must always add to 9 because reasons
+tool1.featureB = 4
+# note this must match the above line, ask Dave why
+tool2.featureA = "foo"
+tool3.featureB = 5
+
+"#
+    ]]);
+}
diff --git a/crates/toml_edit/tests/testsuite/parse.rs b/crates/toml_edit/tests/testsuite/parse.rs
index 39e00cfd..63438e31 100644
--- a/crates/toml_edit/tests/testsuite/parse.rs
+++ b/crates/toml_edit/tests/testsuite/parse.rs
@@ -1605,6 +1605,33 @@ clippy.exhaustive_enums = "warn"
     assert_data_eq!(actual, expected.raw());
 }
 
+#[test]
+fn dotted_key_interspersed_roundtrip() {
+    let input = r###"
+rust.unsafe_op_in_unsafe_fn = "deny"
+
+clippy.cast_lossless = "warn"
+rust.explicit_outlives_requirements = "warn"
+
+clippy.doc_markdown = "warn"
+clippy.exhaustive_enums = "warn"
+"###;
+    let expected = r###"
+rust.unsafe_op_in_unsafe_fn = "deny"
+rust.explicit_outlives_requirements = "warn"
+
+clippy.cast_lossless = "warn"
+
+clippy.doc_markdown = "warn"
+clippy.exhaustive_enums = "warn"
+"###;
+
+    let manifest: DocumentMut = input.parse().unwrap();
+    let actual = manifest.to_string();
+
+    assert_data_eq!(actual, expected.raw());
+}
+
 #[test]
 fn string_repr_roundtrip() {
     assert_string_repr_roundtrip(r#""""#, str![[r#""""#]]);

From 25fbd07d0607c07e025c64aa8fe85c33bd0525be Mon Sep 17 00:00:00 2001
From: mxt <mxnd.taylor@gmail.com>
Date: Wed, 27 Nov 2024 17:30:45 -0600
Subject: [PATCH 2/3] chore: refactoring

- adding better failure reporting in testsuite table parsing
- centralize ordering logic into internal table to improve code re-use between Table and InlineTable
---
 crates/toml_edit/src/inline_table.rs     |  66 +--------
 crates/toml_edit/src/internal_table.rs   | 179 +++++++++++++++++++++++
 crates/toml_edit/src/lib.rs              |   1 +
 crates/toml_edit/src/table.rs            |  70 +--------
 crates/toml_edit/tests/testsuite/edit.rs |   6 +-
 5 files changed, 194 insertions(+), 128 deletions(-)
 create mode 100644 crates/toml_edit/src/internal_table.rs

diff --git a/crates/toml_edit/src/inline_table.rs b/crates/toml_edit/src/inline_table.rs
index 9b8e9c42..43a32e82 100644
--- a/crates/toml_edit/src/inline_table.rs
+++ b/crates/toml_edit/src/inline_table.rs
@@ -3,6 +3,7 @@ use std::iter::FromIterator;
 use crate::key::Key;
 use crate::repr::Decor;
 use crate::table::{Iter, IterMut, KeyValuePairs, TableLike};
+use crate::internal_table::{GetTableValues, SortTable, SortTableBy};
 use crate::{InternalString, Item, KeyMut, RawString, Table, Value};
 
 /// Type representing a TOML inline table,
@@ -51,30 +52,7 @@ impl InlineTable {
     ///
     /// For example, this will return dotted keys
     pub fn get_values(&self) -> Vec<(Vec<&Key>, &Value)> {
-        let mut values = Vec::new();
-        let root = Vec::new();
-        self.append_values(&root, &mut values);
-        values
-    }
-
-    pub(crate) fn append_values<'s>(
-        &'s self,
-        parent: &[&'s Key],
-        values: &mut Vec<(Vec<&'s Key>, &'s Value)>,
-    ) {
-        for (key, value) in self.items.iter() {
-            let mut path = parent.to_vec();
-            path.push(key);
-            match value {
-                Item::Value(Value::InlineTable(table)) if table.is_dotted() => {
-                    table.append_values(&path, values);
-                }
-                Item::Value(value) => {
-                    values.push((path, value));
-                }
-                _ => {}
-            }
-        }
+        GetTableValues::get_values(self)
     }
 
     /// Auto formats the table.
@@ -84,52 +62,18 @@ impl InlineTable {
 
     /// Sorts the key/value pairs by key.
     pub fn sort_values(&mut self) {
-        // Assuming standard tables have their position set and this won't negatively impact them
-        self.items.sort_keys();
-        for value in self.items.values_mut() {
-            match value {
-                Item::Value(Value::InlineTable(table)) if table.is_dotted() => {
-                    table.sort_values();
-                }
-                _ => {}
-            }
-        }
+        SortTable::sort_values(self)
     }
 
     /// Sort Key/Value Pairs of the table using the using the comparison function `compare`.
     ///
     /// The comparison function receives two key and value pairs to compare (you can sort by keys or
     /// values or their combination as needed).
-    pub fn sort_values_by<F>(&mut self, mut compare: F)
+    pub fn sort_values_by<F>(&mut self, compare: F)
     where
         F: FnMut(&Key, &Value, &Key, &Value) -> std::cmp::Ordering,
     {
-        self.sort_values_by_internal(&mut compare);
-    }
-
-    fn sort_values_by_internal<F>(&mut self, compare: &mut F)
-    where
-        F: FnMut(&Key, &Value, &Key, &Value) -> std::cmp::Ordering,
-    {
-        let modified_cmp =
-            |key1: &Key, val1: &Item, key2: &Key, val2: &Item| -> std::cmp::Ordering {
-                match (val1.as_value(), val2.as_value()) {
-                    (Some(v1), Some(v2)) => compare(key1, v1, key2, v2),
-                    (Some(_), None) => std::cmp::Ordering::Greater,
-                    (None, Some(_)) => std::cmp::Ordering::Less,
-                    (None, None) => std::cmp::Ordering::Equal,
-                }
-            };
-
-        self.items.sort_by(modified_cmp);
-        for value in self.items.values_mut() {
-            match value {
-                Item::Value(Value::InlineTable(table)) if table.is_dotted() => {
-                    table.sort_values_by_internal(compare);
-                }
-                _ => {}
-            }
-        }
+        SortTableBy::<Value>::sort_values_by(self, compare)
     }
 
     /// If a table has no key/value pairs and implicit, it will not be displayed.
diff --git a/crates/toml_edit/src/internal_table.rs b/crates/toml_edit/src/internal_table.rs
new file mode 100644
index 00000000..2785fb60
--- /dev/null
+++ b/crates/toml_edit/src/internal_table.rs
@@ -0,0 +1,179 @@
+use crate::{InlineTable, Item, Key, Table, Value};
+/// a place for helper methods supporting the table-like impls
+
+use crate::table::KeyValuePairs;
+
+/// GetTableValues provides the logic for displaying a table's items in their parsed order
+pub(crate) trait GetTableValues {
+    fn items(&self) -> &KeyValuePairs;
+
+    fn get_values(&self) -> Vec<(Vec<&Key>, &Value)> {
+        let mut values = Vec::new();
+        let root = Vec::new();
+        self.append_values(&root, &mut values);
+        values
+    }
+
+    fn append_values<'s>(
+        &'s self,
+        parent: &[&'s Key],
+        values: &mut Vec<(Vec<&'s Key>, &'s Value)>,
+    ) {
+        for (key, item) in self.items().iter() {
+            let mut path = parent.to_vec();
+            path.push(key);
+            match item {
+                Item::Table(table) if table.is_dotted() => {
+                    GetTableValues::append_values(table, &path, values)
+                }
+                Item::Value(Value::InlineTable(table)) if table.is_dotted() => {
+                    GetTableValues::append_values(table, &path, values)
+                }
+                Item::Value(value) => {
+                    values.push((path, value))
+                }
+                _ => {}
+            }
+        }
+    }
+}
+
+/// SortTable provides the logic for sorting a table's items by its keys
+pub(crate) trait SortTable {
+    fn items_mut(&mut self) -> &mut KeyValuePairs;
+
+    fn sort_values(&mut self) {
+        // Assuming standard tables have their doc_position set and this won't negatively impact them
+        self.items_mut().sort_keys();
+        assign_sequential_key_positions(self.items_mut(), |item| {
+            match item {
+                Item::Table(table) if table.is_dotted() => {
+                    SortTable::sort_values(table)
+                }
+                Item::Value(Value::InlineTable(table)) if table.is_dotted() => {
+                    SortTable::sort_values(table)
+                }
+                _ => {}
+            }
+        });
+    }
+}
+
+/// SortTableBy provides the logic for sorting a table by a custom comparison
+pub(crate) trait SortTableBy<It> : SortTable
+where
+    It: for<'a> TryFrom<&'a Item>
+{
+    fn sort_values_by<F>(&mut self, compare: F)
+    where
+        F: FnMut(&Key, &It, &Key, &It) -> std::cmp::Ordering,
+    {
+        // intended for `InlineTable`s, where some `Item`s might not be `Value`s,
+        // in the case of dotted keys mostly I expect.
+        // but for `Table`s the `(Some,Some)` will be the only one used.
+        self.sort_vals_by_direct(
+            &mut Self::generalize(compare)
+        )
+    }
+
+    /// no modification to the comparing Fn in this one,
+    /// allows for slightly improved recursion that does not continuously
+    /// re-modify the comparison function.
+    fn sort_vals_by_direct<F>(&mut self, compare: &mut F)
+    where
+        F: FnMut(&Key, &Item, &Key, &Item) -> std::cmp::Ordering
+    {
+        self.items_mut().sort_by(|key1, val1, key2, val2| {
+            compare(key1, val1, key2, val2)
+        });
+
+        assign_sequential_key_positions(self.items_mut(), |value| {
+            match value {
+                Item::Table(table) if table.is_dotted() => {
+                    SortTableBy::<Item>::sort_values_by(
+                        table,
+                        |k1, i1, k2, i2| {
+                            compare(k1, i1, k2, i2)
+                        }
+                    )
+                },
+                Item::Value(Value::InlineTable(table)) if table.is_dotted() => {
+                    SortTableBy::<Value>::sort_values_by(
+                        table,
+                        |k1, i1, k2, i2| {
+                            let s1 = &Item::from(i1);
+                            let s2 = &Item::from(i2);
+                            compare(k1, s1, k2, s2)
+                        }
+                    )
+                },
+                _ => {}
+            };
+        });
+    }
+
+    fn generalize<'a, F>(mut compare: F) -> Box<dyn FnMut(&Key, &Item, &Key, &Item) -> std::cmp::Ordering + 'a>
+    where
+        F: FnMut(&Key, &It, &Key, &It) -> std::cmp::Ordering + 'a,
+    {
+        Box::new(move |key1, s1, key2, s2| {
+            match (It::try_from(s1).ok(), It::try_from(s2).ok()) {
+                (Some(v1), Some(v2)) => compare(key1, &v1, key2, &v2),
+                (Some(_), None) => std::cmp::Ordering::Greater,
+                (None, Some(_)) => std::cmp::Ordering::Less,
+                (None, None) => std::cmp::Ordering::Equal,
+            }
+        })
+    }
+}
+
+fn assign_sequential_key_positions<F>(items: &mut KeyValuePairs, mut recursive_step: F)
+where
+    F: FnMut(&mut Item),
+{
+    use indexmap::map::MutableKeys;
+    for (pos, (key, value)) in items.iter_mut2().enumerate() {
+        key.set_position(Some(pos));
+        recursive_step(value);
+    }
+}
+
+impl TryFrom<&Item> for Value {
+    type Error = String;
+
+    fn try_from(value: &Item) -> Result<Self, Self::Error> {
+        let err = "cannot extract Value from Non-Value Item:";
+        match value {
+            Item::Value(v) => Ok((*v).clone()),
+            it => it.as_value().map(|v| v.clone()).ok_or(
+                format!("{err}: {it:?}")
+            ),
+        }
+    }
+
+}
+
+impl GetTableValues for Table {
+    fn items(&self) -> &KeyValuePairs {
+        &self.items
+    }
+}
+impl GetTableValues for InlineTable {
+    fn items(&self) -> &KeyValuePairs {
+        &self.items
+    }
+}
+
+impl SortTable for Table {
+    fn items_mut(&mut self) -> &mut KeyValuePairs {
+        &mut self.items
+    }
+}
+impl SortTable for InlineTable {
+    fn items_mut(&mut self) -> &mut KeyValuePairs {
+        &mut self.items
+    }
+}
+
+impl SortTableBy<Item> for Table {}
+impl SortTableBy<Value> for InlineTable {}
diff --git a/crates/toml_edit/src/lib.rs b/crates/toml_edit/src/lib.rs
index c47b902a..966e001c 100644
--- a/crates/toml_edit/src/lib.rs
+++ b/crates/toml_edit/src/lib.rs
@@ -85,6 +85,7 @@ mod error;
 mod index;
 mod inline_table;
 mod internal_string;
+mod internal_table;
 mod item;
 mod key;
 #[cfg(feature = "parse")]
diff --git a/crates/toml_edit/src/table.rs b/crates/toml_edit/src/table.rs
index 1ae02310..94b4cdcf 100644
--- a/crates/toml_edit/src/table.rs
+++ b/crates/toml_edit/src/table.rs
@@ -5,6 +5,7 @@ use indexmap::map::IndexMap;
 use crate::key::Key;
 use crate::repr::Decor;
 use crate::value::DEFAULT_VALUE_DECOR;
+use crate::internal_table::{GetTableValues, SortTable, SortTableBy};
 use crate::{InlineTable, InternalString, Item, KeyMut, Value};
 
 /// Type representing a TOML non-inline table
@@ -64,38 +65,7 @@ impl Table {
     ///
     /// For example, this will return dotted keys
     pub fn get_values(&self) -> Vec<(Vec<&Key>, &Value)> {
-        let mut values = Vec::new();
-        let root = Vec::new();
-        self.append_values(&root, &mut values);
-        values
-    }
-
-    fn append_values<'s>(
-        &'s self,
-        parent: &[&'s Key],
-        values: &mut Vec<(Vec<&'s Key>, &'s Value)>,
-    ) {
-        for (key, value) in self.items.iter() {
-            let mut path = parent.to_vec();
-            path.push(key);
-            match value {
-                Item::Table(table) if table.is_dotted() => {
-                    table.append_values(&path, values);
-                }
-                Item::Value(value) => {
-                    if let Some(table) = value.as_inline_table() {
-                        if table.is_dotted() {
-                            table.append_values(&path, values);
-                        } else {
-                            values.push((path, value));
-                        }
-                    } else {
-                        values.push((path, value));
-                    }
-                }
-                _ => {}
-            }
-        }
+        GetTableValues::get_values(self)
     }
 
     /// Auto formats the table.
@@ -107,48 +77,18 @@ impl Table {
     ///
     /// Doesn't affect subtables or subarrays.
     pub fn sort_values(&mut self) {
-        // Assuming standard tables have their doc_position set and this won't negatively impact them
-        self.items.sort_keys();
-        for value in self.items.values_mut() {
-            match value {
-                Item::Table(table) if table.is_dotted() => {
-                    table.sort_values();
-                }
-                _ => {}
-            }
-        }
+        SortTable::sort_values(self)
     }
 
     /// Sort Key/Value Pairs of the table using the using the comparison function `compare`.
     ///
     /// The comparison function receives two key and value pairs to compare (you can sort by keys or
     /// values or their combination as needed).
-    pub fn sort_values_by<F>(&mut self, mut compare: F)
+    pub fn sort_values_by<F>(&mut self, compare: F)
     where
         F: FnMut(&Key, &Item, &Key, &Item) -> std::cmp::Ordering,
     {
-        self.sort_values_by_internal(&mut compare);
-    }
-
-    fn sort_values_by_internal<F>(&mut self, compare: &mut F)
-    where
-        F: FnMut(&Key, &Item, &Key, &Item) -> std::cmp::Ordering,
-    {
-        let modified_cmp =
-            |key1: &Key, val1: &Item, key2: &Key, val2: &Item| -> std::cmp::Ordering {
-                compare(key1, val1, key2, val2)
-            };
-
-        self.items.sort_by(modified_cmp);
-
-        for value in self.items.values_mut() {
-            match value {
-                Item::Table(table) if table.is_dotted() => {
-                    table.sort_values_by_internal(compare);
-                }
-                _ => {}
-            }
-        }
+        SortTableBy::<Item>::sort_values_by(self, compare)
     }
 
     /// If a table has no key/value pairs and implicit, it will not be displayed.
diff --git a/crates/toml_edit/tests/testsuite/edit.rs b/crates/toml_edit/tests/testsuite/edit.rs
index 666a2399..5b0465a4 100644
--- a/crates/toml_edit/tests/testsuite/edit.rs
+++ b/crates/toml_edit/tests/testsuite/edit.rs
@@ -26,8 +26,10 @@ struct Test {
 
 fn given(input: &str) -> Test {
     let doc = input.parse::<DocumentMut>();
-    assert!(doc.is_ok());
-    Test { doc: doc.unwrap() }
+    match doc {
+        Err(e) => panic!("{}", e),
+        _ => Test { doc: doc.unwrap() }
+    }
 }
 
 impl Test {

From a83d2bdfb1639e8ce946268a05735b7934b3fdd9 Mon Sep 17 00:00:00 2001
From: mxt <mxnd.taylor@gmail.com>
Date: Wed, 27 Nov 2024 17:54:13 -0600
Subject: [PATCH 3/3] fix: add position field for value to preserve order of
 dotted keys

---
 crates/toml_edit/src/internal_table.rs      | 24 +++++++
 crates/toml_edit/src/key.rs                 | 19 ++++++
 crates/toml_edit/src/parser/inline_table.rs | 23 +------
 crates/toml_edit/src/parser/mod.rs          | 47 ++------------
 crates/toml_edit/src/parser/state.rs        |  7 ++
 crates/toml_edit/tests/testsuite/edit.rs    | 71 +++++++++++++++++++--
 crates/toml_edit/tests/testsuite/parse.rs   | 10 +--
 7 files changed, 127 insertions(+), 74 deletions(-)

diff --git a/crates/toml_edit/src/internal_table.rs b/crates/toml_edit/src/internal_table.rs
index 2785fb60..ed25a867 100644
--- a/crates/toml_edit/src/internal_table.rs
+++ b/crates/toml_edit/src/internal_table.rs
@@ -35,6 +35,7 @@ pub(crate) trait GetTableValues {
                 _ => {}
             }
         }
+        sort_values_by_position(values);
     }
 }
 
@@ -138,6 +139,29 @@ where
     }
 }
 
+fn sort_values_by_position<'s>(values: &mut [(Vec<&'s Key>, &'s Value)]) {
+    /*
+    `Vec::sort_by_key` works because we add the position to _every_ item's key during parsing,
+    so keys without positions would be either:
+       1. non-leaf keys (i.e. "foo" or "bar" in dotted key "foo.bar.baz")
+       2. custom keys added to the doc without an explicit position
+    In the case of (1), we'd never see it since we only look at the last
+    key in a dotted-key. So, we can safely return a constant value for these cases.
+
+    To support the most intuitive behavior, we return the maximum usize, placing
+    position=None items at the end, so when you insert it without position, it
+    appends it to the end.
+     */
+    values.sort_by_key(|(key_path, _)| {
+        return match key_path.last().map(|x| x.position) {
+            // unwrap "last()" -> unwrap "position"
+            Some(Some(pos)) => pos,
+            // either last() = None, or position = None
+            _ => usize::MAX
+        };
+    });
+}
+
 impl TryFrom<&Item> for Value {
     type Error = String;
 
diff --git a/crates/toml_edit/src/key.rs b/crates/toml_edit/src/key.rs
index 314870f5..75ae83d3 100644
--- a/crates/toml_edit/src/key.rs
+++ b/crates/toml_edit/src/key.rs
@@ -32,6 +32,7 @@ pub struct Key {
     pub(crate) repr: Option<Repr>,
     pub(crate) leaf_decor: Decor,
     pub(crate) dotted_decor: Decor,
+    pub(crate) position: Option<usize>,
 }
 
 impl Key {
@@ -42,6 +43,7 @@ impl Key {
             repr: None,
             leaf_decor: Default::default(),
             dotted_decor: Default::default(),
+            position: Default::default(),
         }
     }
 
@@ -76,6 +78,12 @@ impl Key {
         self
     }
 
+    /// While creating the `Key`, add a table position to it
+    pub fn with_position(mut self, position: Option<usize>) -> Self {
+        self.position = position;
+        self
+    }
+
     /// Access a mutable proxy for the `Key`.
     pub fn as_mut(&mut self) -> KeyMut<'_> {
         KeyMut { key: self }
@@ -158,6 +166,16 @@ impl Key {
         }
     }
 
+    /// Get the position relative to other keys in parent table
+    pub fn position(&self) -> Option<usize> {
+        self.position
+    }
+
+    /// Set the position relative to other keys in parent table
+    pub fn set_position(&mut self, position: Option<usize>) {
+        self.position = position;
+    }
+
     /// Auto formats the key.
     pub fn fmt(&mut self) {
         self.repr = None;
@@ -190,6 +208,7 @@ impl Clone for Key {
             repr: self.repr.clone(),
             leaf_decor: self.leaf_decor.clone(),
             dotted_decor: self.dotted_decor.clone(),
+            position: self.position,
         }
     }
 }
diff --git a/crates/toml_edit/src/parser/inline_table.rs b/crates/toml_edit/src/parser/inline_table.rs
index 677b39e1..21e1cc20 100644
--- a/crates/toml_edit/src/parser/inline_table.rs
+++ b/crates/toml_edit/src/parser/inline_table.rs
@@ -40,7 +40,8 @@ fn table_from_pairs(
     // Assuming almost all pairs will be directly in `root`
     root.items.reserve(v.len());
 
-    for (path, (key, value)) in v {
+    for (position, (path, (mut key, value))) in v.into_iter().enumerate() {
+        key.set_position(Some(position));
         let table = descend_path(&mut root, &path)?;
 
         // "Likewise, using dotted keys to redefine tables already defined in [table] form is not allowed"
@@ -162,6 +163,7 @@ mod test {
             r#"{a = 1e165}"#,
             r#"{ hello = "world", a = 1}"#,
             r#"{ hello.world = "a" }"#,
+            r#"{ hello.world = "a", goodbye = "b", hello.moon = "c" }"#,
         ];
         for input in inputs {
             dbg!(input);
@@ -173,25 +175,6 @@ mod test {
         }
     }
 
-    #[test]
-    fn inline_table_reordering() {
-        let inputs = [
-            (
-                r#"{ hello.world = "a", goodbye = "b", hello.moon = "c" }"#,
-                // note the stylistic change, hello.moon has a space before its comma
-                r#"{ hello.world = "a", hello.moon = "c" , goodbye = "b"}"#,
-            )
-        ];
-        for (input, expected) in inputs {
-            dbg!(input);
-            let mut parsed = inline_table.parse(new_input(input));
-            if let Ok(parsed) = &mut parsed {
-                parsed.despan(input);
-            }
-            assert_eq!(parsed.map(|a| a.to_string()), Ok(expected.to_owned()));
-        }
-    }
-
     #[test]
     fn invalid_inline_tables() {
         let invalid_inputs = [r#"{a = 1e165"#, r#"{ hello = "world", a = 2, hello = 1}"#];
diff --git a/crates/toml_edit/src/parser/mod.rs b/crates/toml_edit/src/parser/mod.rs
index 8f0db079..da7a7d68 100644
--- a/crates/toml_edit/src/parser/mod.rs
+++ b/crates/toml_edit/src/parser/mod.rs
@@ -217,38 +217,12 @@ key = "value"
 "#,
             r#"hello.world = "a"
 "#,
-            r#"foo = 1979-05-27 # Comment
-"#,
-        ];
-        for input in documents {
-            dbg!(input);
-            let parsed = parse_document(input).map(|d| d.into_mut());
-            let doc = match parsed {
-                Ok(doc) => doc,
-                Err(err) => {
-                    panic!("Parse error: {err:?}\nFailed to parse:\n```\n{input}\n```")
-                }
-            };
-
-            assert_data_eq!(doc.to_string(), input.raw());
-        }
-    }
-    #[test]
-    fn documents_reordering() {
-        let documents = [
-            (
-                r#"
+            r#"
 hello.world = "a"
 goodbye = "b"
 hello.moon = "c"
 "#,
-                r#"
-hello.world = "a"
-hello.moon = "c"
-goodbye = "b"
-"#,
-            ),
-            (r#"
+            r#"
 tool1.featureA = "foo"
 # note this must match the above line, ask Dave why
 tool2.featureA = "foo"
@@ -257,19 +231,10 @@ tool2.featureA = "foo"
 tool1.featureB = -1
 tool3.featureB = 10
 "#,
-             // these comments have now completely lost their meaning
-             r#"
-tool1.featureA = "foo"
-
-# these two values must always add to 9 because reasons
-tool1.featureB = -1
-# note this must match the above line, ask Dave why
-tool2.featureA = "foo"
-tool3.featureB = 10
-"#
-            ),
+            r#"foo = 1979-05-27 # Comment
+"#,
         ];
-        for (input, expected) in documents {
+        for input in documents {
             dbg!(input);
             let parsed = parse_document(input).map(|d| d.into_mut());
             let doc = match parsed {
@@ -279,7 +244,7 @@ tool3.featureB = 10
                 }
             };
 
-            assert_data_eq!(doc.to_string(), expected.raw());
+            assert_data_eq!(doc.to_string(), input.raw());
         }
     }
 
diff --git a/crates/toml_edit/src/parser/state.rs b/crates/toml_edit/src/parser/state.rs
index 6a234136..de126df9 100644
--- a/crates/toml_edit/src/parser/state.rs
+++ b/crates/toml_edit/src/parser/state.rs
@@ -7,6 +7,8 @@ pub(crate) struct ParseState {
     root: Table,
     trailing: Option<std::ops::Range<usize>>,
     current_table_position: usize,
+    // Current position within a table, to help order dotted keys
+    current_value_position: usize,
     current_table: Table,
     current_is_array: bool,
     current_table_path: Vec<Key>,
@@ -20,6 +22,7 @@ impl ParseState {
             root: Table::new(),
             trailing: None,
             current_table_position: 0,
+            current_value_position: 0,
             current_table: root,
             current_is_array: false,
             current_table_path: Vec::new(),
@@ -74,6 +77,9 @@ impl ParseState {
         if let (Some(existing), Some(value)) = (self.current_table.span(), value.span()) {
             self.current_table.span = Some((existing.start)..(value.end));
         }
+        key.set_position(Some(self.current_value_position));
+        self.current_value_position += 1;
+
         let table = &mut self.current_table;
         let table = Self::descend_path(table, &path, true)?;
 
@@ -161,6 +167,7 @@ impl ParseState {
         }
 
         self.current_table_position += 1;
+        self.current_value_position = 0;
         self.current_table.decor = decor;
         self.current_table.set_implicit(false);
         self.current_table.set_dotted(false);
diff --git a/crates/toml_edit/tests/testsuite/edit.rs b/crates/toml_edit/tests/testsuite/edit.rs
index 5b0465a4..59e7ea6e 100644
--- a/crates/toml_edit/tests/testsuite/edit.rs
+++ b/crates/toml_edit/tests/testsuite/edit.rs
@@ -1076,7 +1076,6 @@ tool2.featureA = "foo"
 tool1.featureB = -1
 tool3.featureB = 10
 "#)
-        //language=none
     .running_on_doc(|doc| {
         let first_tool_k = "tool1";
         let second_tool_k = "tool3";
@@ -1101,18 +1100,82 @@ tool3.featureB = 10
         root[first_tool_k][feature_k] = Item::from(split_val);
         root[second_tool_k][feature_k] = Item::from(split_val2);
     })
-    // these comments have now completely lost their meaning
     .produces_display(str![[
         r#"
 
 tool1.featureA = "foo"
+# note this must match the above line, ask Dave why
+tool2.featureA = "foo"
 
 # these two values must always add to 9 because reasons
 tool1.featureB = 4
-# note this must match the above line, ask Dave why
-tool2.featureA = "foo"
 tool3.featureB = 5
 
 "#
     ]]);
 }
+
+#[test]
+fn insert_key_with_position() {
+    given(r#"foo.bar = 1
+
+foo.baz.qwer = "a"
+goodbye = { forever="no" }
+
+foo.pub = 2
+foo.baz.asdf = "b"
+"#)
+    .running_on_doc(|doc| {
+        let root = doc.as_table_mut();
+        let (_, foo_v) = root.get_key_value_mut("foo").unwrap();
+        let foo = as_table!(foo_v);
+        let (_, foo_baz_v) = foo.get_key_value_mut("baz").unwrap();
+        let foo_baz = as_table!(foo_baz_v);
+
+        let foo_baz_asdf_v = foo_baz.remove("asdf").unwrap();
+        let foo_baz_asdf_k = Key::new("asdf").with_position(Some(0));
+
+        foo_baz.insert_formatted(&foo_baz_asdf_k, foo_baz_asdf_v);
+    })
+    .produces_display(str![[r#"foo.bar = 1
+foo.baz.asdf = "b"
+
+foo.baz.qwer = "a"
+goodbye = { forever="no" }
+
+foo.pub = 2
+
+"#]]);
+}
+
+#[test]
+fn insert_key_with_position_none() {
+    given(r#"foo.bar = 1
+
+foo.baz.qwer = "a"
+goodbye = { forever="no" }
+
+foo.pub = 2
+foo.baz.asdf = "b"
+"#)
+    .running_on_doc(|doc| {
+        let root = doc.as_table_mut();
+        let (_, foo_v) = root.get_key_value_mut("foo").unwrap();
+        let foo = as_table!(foo_v);
+        let (_, foo_baz_v) = foo.get_key_value_mut("baz").unwrap();
+        let foo_baz = as_table!(foo_baz_v);
+
+        let foo_baz_asdf_v = foo_baz.remove("qwer").unwrap();
+        let foo_baz_asdf_k = Key::new("qwer").with_position(None);
+
+        foo_baz.insert_formatted(&foo_baz_asdf_k, foo_baz_asdf_v);
+    })
+    .produces_display(str![[r#"foo.bar = 1
+goodbye = { forever="no" }
+
+foo.pub = 2
+foo.baz.asdf = "b"
+foo.baz.qwer = "a"
+
+"#]]);
+}
diff --git a/crates/toml_edit/tests/testsuite/parse.rs b/crates/toml_edit/tests/testsuite/parse.rs
index 63438e31..e437ca4e 100644
--- a/crates/toml_edit/tests/testsuite/parse.rs
+++ b/crates/toml_edit/tests/testsuite/parse.rs
@@ -1616,15 +1616,7 @@ rust.explicit_outlives_requirements = "warn"
 clippy.doc_markdown = "warn"
 clippy.exhaustive_enums = "warn"
 "###;
-    let expected = r###"
-rust.unsafe_op_in_unsafe_fn = "deny"
-rust.explicit_outlives_requirements = "warn"
-
-clippy.cast_lossless = "warn"
-
-clippy.doc_markdown = "warn"
-clippy.exhaustive_enums = "warn"
-"###;
+    let expected = input;
 
     let manifest: DocumentMut = input.parse().unwrap();
     let actual = manifest.to_string();