fix(serde): support deserializing empty scalars

[not-my-profile]

Previously attempting to deserialize the value of foo in:

foo:

failed when a mapping, sequence or unit was expected.

Note that this fix depends on a related bug fix in yaml-rust2: https://github.com/Ethiraric/yaml-rust2/pull/37. So that:

foo: null

or

foo: ~

can still result in errors when a mapping/sequence/unit is expected.

[kinnison]

Hi Martin,

Thank you for your contribution -- before I go further with review can you please give me some examples of where you want this kind of functionality? I've been trying to keep marked-yaml to a very clean explicit subset of YAML, and this kind of empty value feels iffy (I have, thus far, deliberately not included null as it were).

If you have a strong use-case though, I'm prepared to consider changing my stance -- I appreciate not everyone has my strict view of things; and the new(ish) may-coerce stuff does allow for this kind of thing - you used it well.

Thanks,

Daniel.

[not-my-profile]

Hi Daniel,

I'm working on a piece of software where I want to use YAML for the configuration file and I'm looking to use marked-yaml to provide spans along with my error messages. I'm migrating from serde-yaml where:

#[derive(Debug, Deserialize)]
struct Foo {
    x: Vec<u8>,
}
let foo: Foo = serde_yaml::from_str("x:").unwrap();
assert!(foo.x.is_empty())

just works. But most importantly that's also what users expect. If a software can work with:

x:
  - 1
  - 2
  - 3

and you can remove an item from the sequence by removing the corresponding line then the software shouldn't fail with:

invalid type: string "~", expected a sequence

when you remove all of the lines under x. This current error is particularly confusing because the config then doesn't even contain "~" ... but I submitted a fix to that bug in yaml-rust2. However with that fix invalid type: string "", expected a sequence isn't much better ... when it should work without an error.

Lastly I'd also like to draw a parallel to TOML ... cargo also doesn't complain when you have an empty section in your Cargo.toml, e.g. [features] without anything in it.

Cheers, Martin

[kinnison]

Your reasoning is sound and I approve of the idea. Can you let me know if/when yaml-rust2 gets the fix? In the meantime let's hit approve and see how the CI does :D

[not-my-profile]

Yeah sure I'll let you know if/when yaml-rust2 gets the fix :) I added a temporary git dependency to make the CI pass in the meantime.

Would you be ok with introducing a CHANGELOG.md?

[kinnison]

I'd be OK with a CHANGELOG - I'd also like to ensure that this behaviour is documented in the rustdoc too if you can think of a nice way to do that. Let's see if the CI runs with your YAML fork for now.

[kinnison]

As an addendum - when I think of CHANGELOG.md I generally speaking prefer something like NEWS.md since to me the changelog is the git history -- a NEWS file on the other hand is human-curated for important changes. If you're amenable then perhaps aim at a NEWS.md instead?

[not-my-profile]

Ad rustdoc: Sure, I added a sentence to the serde section :)

Ad changelog file naming: I'd be ok with NEWS.md though I'd like to point out that CHANGELOG.md is very much the de-facto standard name for the file that contains human-curated changes. Cases in point: hashlink, yaml-rust2 follow that convention as well as prominent crates such as tokio and regex.

[kinnison]

Sorry for the delay; rustconf got in the way 😀

I don't mind enough about the name, so sure, let's go for CHANGELOG.md :D

[not-my-profile]

Oh, I'm envious :D My bug fix in yaml-rust2 has been merged in the meantime. I'll update this MR when they release a new version.

[not-my-profile]

yaml-rust2 0.9 has been released with my fix. I updated this PR to update the dependency accordingly and also added a CHANGELOG.md.

[kinnison]

I've re-reviewed and enabled the workflows again - Assuming all is good I'll look at what else I want before releasing a new version of marked-yaml - thank you @not-my-profile for your efforts.