Produce valid TOML for non-map root values - #551
Conversation
Bare scalars and arrays are not valid TOML roots. Wrap them into a single-key map to emit `value = ...` instead. Fixes TomWright#550
|
Hey @guilherme-puida , I missed this and opened this PR. Happy to take your contribution if you could update the root value handling here to exclude |
|
By excluding "value = ", dasel would produce invalid TOML documents. For example, when running on current And on this branch: Is this really what is expected? I would assume that dasel always emits valid TOML documents. |
|
It's a fair question, and you are right about the output not being directly readable by a TOML parser, but I'm cautious of anyone already using a toml output with a single value and this change breaking their pipelines. Quite often people look to extract the value itself without any TOML document for use with other scripts, or even for use in another dasel executable. I am torn though since ini, hcl and csv writers already have requirements around the output data. |
|
It's a fair question, and you are right about the output not being
directly readable by a TOML parser, but I'm cautious of anyone already
using a toml output with a single value and this change breaking their
pipelines.
Quite often people look to extract the value itself without any TOML
document for use with other scripts, or even for use in another dasel
executable.
Hm, this makes sense. I didn't consider this usecase. Something like
`echo 'value = 1' | dasel -i toml '$root.value'`
would just echo back the input instead of outputting just `1`.
I am torn though since ini, hcl and csv writers already have
requirements around the output data.
Tough spot. IMHO it would be nice to make this consistent: always output
a valid document in the format requested, but I understand if you want
to keep the current behavior instead. Feel free to close this if you
decide to go that route.
|
|
I think we can take the middle road. If you could keep the current (incorrect) behaviour in-place, then open up a new PR with the change. We can let the people raise any issue they have with it, otherwise let it go in. I do need to create a "plain" output writer which could substitute the problematic use-case I mentioned above in the future. |
Might be easier to merge #553 first, then I'll adapt this PR to just introduce the
+1 |
|
OK will do, thanks. I've just merged the plain output writer to set this PR up for success |
A document like the one below (or anything that is not a key-value pair):
is not valid TOML. This PR fixes this by making it a key-value pair instead.
Do note that this is a breaking change, but I guess this is fine since the previously emitted documents were just invalid.
I chose
valuehere arbitrarily, it could be changed to another key. This fixes #550, and also bumps to-toml to the latest version.