tweag / nickel

Better configuration for less
https://nickel-lang.org/
MIT License
2.43k stars 93 forks source link

Parse error when importing toml files #2059

Closed axyz closed 1 month ago

axyz commented 1 month ago

Describe the bug toml files that use dot paths fail to parse.

error: toml parse error: TOML parse error at line 1, column 1
  |
1 | foo.bar = "qux"
  | ^^^^^^^^^^^^^^^
invalid type: string "bar", expected a borrowed string

on the other hand, when not using paths with simple top level keys (like in the example folder) e.g.

foo = "bar"

works correctly

even square bracket syntax seems to fail

[foo.bar]
qux = "42"

showing same error.

To Reproduce try any of the simple previous examples in a minimal ncl file like

let _data = import "test.toml" in

{ data = _data }

Expected behavior A clear and concise description of what you expected to happen.

Environment

Additional context Add any other context about the problem here.

yannham commented 1 month ago

I suspect this is the addition of span information to TOML files which landed in 1.8.

yannham commented 1 month ago

Confirmed: this repro example doesn't fail in 1.7.0

yannham commented 1 month ago

After some investigation (albeit a bit shallow), I think it's a potential issue with toml-rs. Reported here https://github.com/toml-rs/toml/issues/798.

In the meantime, @axyz : is building from source a possibility for you? In that case, you can checkout the 1.8.1-release branch and build Nickel with spanned-deser feature disabled in the following way:

$ cargo build --bin nickel --no-default-features --features "doc format repl" --release
[...]

And get a binary in ./target/release/nickel with span information for TOML disabled, which should work.

In any case, we might release a patch version where spanned deserialization is disabled by default, to fix the regression while the situation is sorted out.

axyz commented 1 month ago

@yannham yes, nothing urgent. I can use 1.7.0 or build a binary if needed, just wanted to report the issue.

Thanks for taking a look.

yannham commented 4 weeks ago

Has been actually fixed rather than just disabled in #2074, so that it will stay enabled and be fixed in the 1.9 release (coming very soon)