Repository: mitsuhiko/minijinja (language: rust)

Issue:
Keys gets reordered, unlike with the Python implementation
Please consider the following input data:

```yaml
---
fruits:
  cantaloupe:
    colour: orange
  banana:
    colour: yellow
  apple:
    colour: green
```

and this template:

```jinja2
{%- for fruit in fruits -%}
{{ fruit }}: {{ fruits[fruit].colour }}
{% endfor -%}
```

Running `minijinja-cli --py-compat --no-newline fruits.txt.j2 properties.yaml` on those result in the following output:

```
apple: green
banana: yellow
cantaloupe: orange
```

The `--py-compat` flag makes no difference. The need for `--no-newline` is surprising as the closing brace is already instructed to strip whitespace, but without unintended side effects.

**Additional helpful information:**

- Version of minijinja: 2.4.0 (85c9682)
- Version of rustc: rustc 1.81.0 (eeb90cda1 2024-09-04) (built from a source tarball)
- Operating system and version: OpenBSD 7.6 GENERIC.MP#338 amd64

**Expected result**

Given the stated goal to _Stay as close as possible to Jinja2_  it is unexpected that the fruits gets reordered (i.e. sorted alphabetically?) when the Python implementation keeps their order as given in the yaml file.

Compare with the results from e.g. running either `j2 fruits.txt.j2 properties.yaml` or `jinja2-3.9 fruits.txt.j2 properties.yaml`. Where j2 comes from j2cli ([kolypto/j2cli](https://github.com/kolypto/j2cli)) in Debian, version 0.3.12b-4 and jinja2-3.9 comes from py39-jinja2-cli-0.8.2 ([mattrobenholt/jinja2-cli](https://github.com/mattrobenolt/jinja2-cli)) in FreeBSD.

```
cantaloupe: orange
banana: yellow
apple: green
```

I have not looked at minijinja's source code. Neither have I investigated nor reflected on whether yaml is giving guarantees on retaining order, but in practise it has always done so for years on multiple machines. Given that the behaviour deviating, filing a bug seems appropriate.


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f76e2b4c..aa57d11c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 All notable changes to MiniJinja are documented here.
 
+## 2.4.1
+
+- `minijinja-cli` now supports preservation of order in maps.  #611
+- Fixed an issue where CBOR was not correctly deserialized in
+  `minijinja-cli`.  #611
+
 ## 2.4.0
 
 - Updated version of `minijinja-cli` with support for better documentation,
diff --git a/minijinja-cli/Cargo.toml b/minijinja-cli/Cargo.toml
index 0dcb35e9..a7d82c11 100644
--- a/minijinja-cli/Cargo.toml
+++ b/minijinja-cli/Cargo.toml
@@ -12,7 +12,7 @@ readme = "README.md"
 rust-version = "1.65"
 
 [features]
-default = ["toml", "yaml", "querystring", "cbor", "datetime", "json5", "repl", "unicode", "contrib"]
+default = ["toml", "yaml", "querystring", "cbor", "datetime", "json5", "repl", "unicode", "contrib", "preserve_order"]
 yaml = ["serde_yml"]
 querystring = ["serde_qs"]
 cbor = ["ciborium"]
@@ -24,6 +24,7 @@ unicode = ["minijinja/unicode"]
 ini = ["configparser"]
 contrib = ["minijinja-contrib"]
 toml = ["dep:toml", "home"]
+preserve_order = ["minijinja/preserve_order"]
 
 [dependencies]
 anyhow = "1.0.74"
diff --git a/minijinja-cli/README.md b/minijinja-cli/README.md
index 08ddccff..6cea69a3 100644
--- a/minijinja-cli/README.md
+++ b/minijinja-cli/README.md
@@ -193,6 +193,7 @@ selected when the defaults are turned off:
 * `completions`: enables the generation of completions
 * `unicode`: enables the unicode identifier support
 * `contrib`: enables the `minijinja_contrib` based functionality including the `--py-compat` flag
+* `preserve_order`: enables order preservation for maps
 
 Additionally if the `ASSET_OUT_DIR` environment variable is set during
 compilation manpage (and optionally completions) are generated into that
diff --git a/minijinja-cli/src/cli.rs b/minijinja-cli/src/cli.rs
index 00025808..c4825ace 100644
--- a/minijinja-cli/src/cli.rs
+++ b/minijinja-cli/src/cli.rs
@@ -1,6 +1,6 @@
 use std::borrow::Cow;
 use std::collections::BTreeMap;
-use std::io::Write;
+use std::io::{Read, Write};
 use std::path::{Path, PathBuf};
 use std::sync::Mutex;
 use std::{fs, io};
@@ -67,13 +67,14 @@ fn load_data(
     selector: Option<&str>,
 ) -> Result<(BTreeMap<String, Value>, bool), Error> {
     let (contents, stdin_used) = if path == Path::new(STDIN_STDOUT) {
-        (
-            io::read_to_string(io::stdin()).context("unable to read data from stdin")?,
-            true,
-        )
+        let mut buf = Vec::<u8>::new();
+        io::stdin()
+            .read_to_end(&mut buf)
+            .context("unable to read data from stdin")?;
+        (buf, true)
     } else {
         (
-            fs::read_to_string(path)
+            fs::read(path)
                 .with_context(|| format!("unable to read data file '{}'", path.display()))?,
             false,
         )
@@ -89,24 +90,28 @@ fn load_data(
     };
 
     let mut data: Value = match format {
-        "json" => preferred_json::from_str(&contents)?,
+        "json" => preferred_json::from_slice(&contents)?,
         #[cfg(feature = "querystring")]
-        "querystring" => Value::from(serde_qs::from_str::<BTreeMap<String, Value>>(&contents)?),
+        "querystring" => Value::from(serde_qs::from_bytes::<BTreeMap<String, Value>>(&contents)?),
         #[cfg(feature = "yaml")]
         "yaml" => {
             // for merge keys to work we need to manually call `apply_merge`.
             // For this reason we need to deserialize into a serde_yml::Value
             // before converting it into a final value.
-            let mut v: serde_yml::Value = serde_yml::from_str(&contents)?;
+            let mut v: serde_yml::Value = serde_yml::from_slice(&contents)?;
             v.apply_merge()?;
             Value::from_serialize(v)
         }
         #[cfg(feature = "toml")]
-        "toml" => toml::from_str(&contents)?,
+        "toml" => {
+            let contents = String::from_utf8(contents).context("invalid utf-8")?;
+            toml::from_str(&contents)?
+        }
         #[cfg(feature = "cbor")]
-        "cbor" => ciborium::from_reader(contents.as_bytes())?,
+        "cbor" => ciborium::from_reader(&contents[..])?,
         #[cfg(feature = "ini")]
         "ini" => {
+            let contents = String::from_utf8(contents).context("invalid utf-8")?;
             let mut config = configparser::ini::Ini::new();
             config
                 .read(contents)


Test patch:
diff --git a/minijinja-cli/tests/test_basic.rs b/minijinja-cli/tests/test_basic.rs
index a9869562..4ddc654a 100644
--- a/minijinja-cli/tests/test_basic.rs
+++ b/minijinja-cli/tests/test_basic.rs
@@ -12,13 +12,13 @@ fn file_with_contents(contents: &str) -> NamedTempFile {
     file_with_contents_and_ext(contents, "")
 }
 
-fn file_with_contents_and_ext(contents: &str, ext: &str) -> NamedTempFile {
+fn file_with_contents_and_ext<X: AsRef<[u8]>>(contents: X, ext: &str) -> NamedTempFile {
     let mut f = tempfile::Builder::new()
         .prefix("minijinja-testfile--")
         .suffix(ext)
         .tempfile()
         .unwrap();
-    f.write_all(contents.as_bytes()).unwrap();
+    f.write_all(contents.as_ref()).unwrap();
     f
 }
 
@@ -232,6 +232,29 @@ fn test_querystring() {
     "###);
 }
 
+#[test]
+#[cfg(feature = "cbor")]
+fn test_cbor() {
+    let input = file_with_contents_and_ext(
+        [0xa1, 0x63, 0x66, 0x6f, 0x6f, 0x63, 0x62, 0x61, 0x72],
+        ".cbor",
+    );
+    let tmpl = file_with_contents(r#"Hello {{ foo }}!"#);
+
+    assert_cmd_snapshot!(
+        cli()
+            .arg(tmpl.path())
+            .arg(input.path()),
+        @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+    Hello bar!
+
+    ----- stderr -----
+    "###);
+}
+
 #[test]
 #[cfg(feature = "ini")]
 fn test_ini() {
@@ -270,6 +293,128 @@ fn test_ini() {
     "###);
 }
 
+#[test]
+#[cfg(feature = "preserve_order")]
+fn test_preserve_order_json() {
+    let input = file_with_contents_and_ext(r#"{"x": {"c": 3, "a": 1, "b": 2}}"#, ".json");
+    let tmpl =
+        file_with_contents("{% for key, value in x|items %}{{ key }}: {{ value }}\n{% endfor %}");
+
+    assert_cmd_snapshot!(
+        cli()
+            .arg(tmpl.path())
+            .arg(input.path()),
+        @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+    c: 3
+    a: 1
+    b: 2
+
+
+    ----- stderr -----
+    "###);
+}
+
+#[test]
+#[cfg(all(feature = "preserve_order", feature = "yaml"))]
+fn test_preserve_order_yaml() {
+    let input = file_with_contents_and_ext(
+        r#"
+x:
+  c: 3
+  a: 1
+  b: 2
+"#,
+        ".yaml",
+    );
+    let tmpl =
+        file_with_contents("{% for key, value in x|items %}{{ key }}: {{ value }}\n{% endfor %}");
+
+    assert_cmd_snapshot!(
+        cli()
+            .arg(tmpl.path())
+            .arg(input.path()),
+        @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+    c: 3
+    a: 1
+    b: 2
+
+
+    ----- stderr -----
+    "###);
+}
+
+#[test]
+#[cfg(all(feature = "preserve_order", feature = "toml"))]
+fn test_preserve_order_toml() {
+    let input = file_with_contents_and_ext(
+        r#"
+[x]
+c = 3
+a = 1
+b = 2
+"#,
+        ".toml",
+    );
+    let tmpl =
+        file_with_contents("{% for key, value in x|items %}{{ key }}: {{ value }}\n{% endfor %}");
+
+    assert_cmd_snapshot!(
+        cli()
+            .arg(tmpl.path())
+            .arg(input.path()),
+        @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+    c: 3
+    a: 1
+    b: 2
+
+
+    ----- stderr -----
+    "###);
+}
+
+#[test]
+#[cfg(all(feature = "preserve_order", feature = "cbor"))]
+fn test_preserve_order_cbor() {
+    let input = file_with_contents_and_ext(
+        [
+            0xa1, // map(1)
+            0x61, 0x78, // "x"
+            0xa3, // map(3)
+            0x61, 0x63, 0x03, // "c": 3
+            0x61, 0x61, 0x01, // "a": 1
+            0x61, 0x62, 0x02, // "b": 2
+        ],
+        ".cbor",
+    );
+    let tmpl =
+        file_with_contents("{% for key, value in x|items %}{{ key }}: {{ value }}\n{% endfor %}");
+
+    assert_cmd_snapshot!(
+        cli()
+            .arg(tmpl.path())
+            .arg(input.path()),
+        @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+    c: 3
+    a: 1
+    b: 2
+
+
+    ----- stderr -----
+    "###);
+}
+
 #[test]
 fn test_context_stdin() {
     let tmpl = file_with_contents(r#"Hello {{ foo }}!"#);