repro for #417

[Halkcyon]

No description provided.

[matthiasbeyer]

Nice. Could you also add one with yaml-rust, so that we have all three cases?

I will then take these patches and continue based on them... not sure how, but I guess I will think of something.

[Halkcyon]

@matthiasbeyer not sure I understand; the default case is yaml-rust with the second test case being for serde_yaml.

[matthiasbeyer]

Yes, I meant add another case where you use only yaml-rust, without config-rs in between. This way we can verify that we don't mess things up in the config-rs implementation!

[Halkcyon]

@matthiasbeyer Done. Surprising result, the new test passes

[matthiasbeyer]

The changes itself look good.

CI fails because the files in tests/Settings are not updated to match the expected Settings structure!

The commit linter also fails. Please:

• Rewrite the commits in imperative mood ("Add stuff" instead of "Adding stuff"). Don't hesitate to include details in the message body!
• Ensure the length of the subject line does not exceed 50 characters
• Make sure to sign off your commits

In general: https://cbea.ms/git-commit/ 😉

---

Or ping me if you don't care anymore, then I will take over this PR! 😉

[matthiasbeyer]

This test fails because the files in tests/Settings are not updated for the new key ("192.168.1.1")!

[Halkcyon]

It is updated. It fails due to #417

[matthiasbeyer]

This test fails here because the tests/Settings files are not updated.

[Halkcyon]

@matthiasbeyer It was updated, but due to #417 it fails. This is the bug reproduced.

https://github.com/Halkcyon/config-rs/blob/cdb945f8a2b2fd7223d518eec874e28e38e491fa/tests/Settings.yaml#L18

[matthiasbeyer]

Ah, I guess I was not fully awake when reviewing this. Sorry about that.

[matthiasbeyer]

So if I understand correctly:

• Plain yaml-rust succeeds
• config-rs (which uses yaml-rust) fails

Okay, so we have to investigate why that is.

[polarathene]

This is not specific to YAML parsing, but feature of config-rs with map lookup keys using dot notation.

---

Okay, so we have to investigate why that is.

Failure is due to . not being a permitted raw identifier char (accepting . here will fix this):

config-rs/src/path/parser.rs

Lines 15 to 25 in 6946069

	fn raw_ident(i: &str) -> IResult<&str, String> {
	map(
	is_a(
	"abcdefghijklmnopqrstuvwxyz \
	ABCDEFGHIJKLMNOPQRSTUVWXYZ \
	0123456789 \
	_-",
	),
	ToString::to_string,
	)(i)
	}

Instead . is used to designate a child field (accepting a . above will cause the test for this feature to fail):

config-rs/src/path/parser.rs

Lines 38 to 49 in 6946069

	fn postfix<'a>(expr: Expression) -> impl FnMut(&'a str) -> IResult<&'a str, Expression> {
	let e2 = expr.clone();
	let child = map(preceded(tag("."), raw_ident), move |id| {
	Expression::Child(Box::new(expr.clone()), id)
	});
	let subscript = map(delimited(char('['), integer, char(']')), move |num| {
	Expression::Subscript(Box::new(e2.clone()), num)
	});
	alt((child, subscript))
	}

That above code is called from this method during config build:

config-rs/src/source.rs

Lines 30 to 38 in 6946069

	fn set_value(cache: &mut Value, key: &str, value: &Value) {
	match path::Expression::from_str(key) {
	// Set using the path
	Ok(expr) => expr.set(cache, value.clone()),
	// Set diretly anyway
	_ => path::Expression::Identifier(key.to_string()).set(cache, value.clone()),
	}
	}

which multiple locations will call via this method:

config-rs/src/source.rs

Lines 20 to 27 in 6946069

	/// Collects all configuration properties to a provided cache.
	fn collect_to(&self, cache: &mut Value) -> Result<()> {
	self.collect()?
	.iter()
	.for_each(|(key, val)| set_value(cache, key, val));
	Ok(())
	}

---

I'm not too familiar with config-rs, but vaguely recall some feature to access fields with dot notation. I assume it's for the value map as a string key lookup feature. Thus you probably can't have both here unless there was support to make the distinction 🤷‍♂️

I understand the intent was to deserialize with the serde rename feature to the desired field. This won't work AFAIK since the builder is creating this cache prior to deserializing which splits up the key:

# Example input
hello.world: example
192.168.1.1: a string value

Built config before deserializing to target config from user

A config: Config {
    defaults: {},
    overrides: {},
    sources: [],
    cache: Value {
        origin: None,
        kind: Table(
            {
                "hello": Value {
                    origin: None,
                    kind: Table(
                        {
                            "world": Value {
                                origin: Some(
                                    "hello.yaml",
                                ),
                                kind: String(
                                    "example",
                                ),
                            },
                        },
                    ),
                },
                "192": Value {
                    origin: None,
                    kind: Table(
                        {
                            "168": Value {
                                origin: None,
                                kind: Table(
                                    {
                                        "1": Value {
                                            origin: None,
                                            kind: Table(
                                                {
                                                    "1": Value {
                                                        origin: Some(
                                                            "hello.yaml",
                                                        ),
                                                        kind: String(
                                                            "a string value",
                                                        ),
                                                    },
                                                },
                                            ),
                                        },
                                    },
                                ),
                            },
                        },
                    ),
                },
            },
        ),
    },
}

Config format deserializes correctly before handed to builder as source input

Value {
    origin: Some(
        "hello.yaml",
    ),
    kind: Table(
        {
            "hello.world": Value {
                origin: Some(
                    "hello.yaml",
                ),
                kind: String(
                    "example",
                ),
            },
            "192.168.1.1": Value {
                origin: Some(
                    "hello.yaml",
                ),
                kind: String(
                    "a string value",
                ),
            },
        },
    ),
}