Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is it possible to get keys in the same order? #1

Closed
xphoniex opened this issue Feb 8, 2022 · 1 comment
Closed

Is it possible to get keys in the same order? #1

xphoniex opened this issue Feb 8, 2022 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@xphoniex
Copy link

xphoniex commented Feb 8, 2022

File 1:

version: 1
config:
  name: app

File 2:

version: 999

Result:

config:
  name: app
version: 999

whereas I expected it to keep the order.

@qtfkwk qtfkwk self-assigned this Nov 21, 2023
@qtfkwk
Copy link
Owner

qtfkwk commented Nov 21, 2023

As you can tell, this crate hasn't had any attention in some time. Sorry about that. Working on it a bit now.

Was able to reproduce via the following failing slightly modified copies of the merge_multiple_string_string_with_conflict test:

#[test]
fn merge_multiple_string_string_with_conflict_2() {
    let mut hash = MergeYamlHash::new();
    let yaml1 = "apple: 1\nbanana: 2".to_string();
    let yaml2 = "apple: 3".to_string();
    let result = "apple: 3\nbanana: 2";
    hash.merge_vec(vec![yaml1, yaml2]);
    assert_eq!(hash.to_string(), result);
}
---- tests::merge_multiple_string_string_with_conflict_2 stdout ----
thread 'tests::merge_multiple_string_string_with_conflict_2' panicked at src/tests.rs:69:5:
assertion `left == right` failed
  left: "banana: 2\napple: 3"
 right: "apple: 3\nbanana: 2"
#[test]
fn merge_multiple_string_string_with_conflict_3() {
    let mut hash = MergeYamlHash::new();
    let yaml1 = "apple: 1\nbanana: 2\ncherry: 3".to_string();
    let yaml2 = "banana: 4".to_string();
    let result = "apple: 1\nbanana: 4\ncherry: 3";
    hash.merge_vec(vec![yaml1, yaml2]);
    assert_eq!(hash.to_string(), result);
}
---- tests::merge_multiple_string_string_with_conflict_3 stdout ----
thread 'tests::merge_multiple_string_string_with_conflict_3' panicked at src/tests.rs:79:5:
assertion `left == right` failed
  left: "apple: 1\ncherry: 3\nbanana: 4"
 right: "apple: 1\nbanana: 4\ncherry: 3"

It seems the readme is right when it claims that "insertion order is maintained" only when there is no conflict or the conflict in file2 is the last item in file1, which is to say it's mostly wrong actually.

This crate as of version 0.3.0 has been upgraded to edition 2021, dependencies updated, code/comments/etc revamped, the merge_hashes method has been moved to a function due to clippy's only_used_in_recursion lint, and its Yaml::Hash.insert approach has been modified to use the entry().and_modify().or_insert_with() pattern which should resolve this issue.

@xphoniex If you're still around and able to test this and confirm it resolves, please do and lmk!

@qtfkwk qtfkwk added the bug Something isn't working label Nov 21, 2023
@qtfkwk qtfkwk closed this as completed Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants