saphyr-rs / saphyr

A set of crates dedicated to parsing YAML.
58 stars 5 forks source link

Port 644af6a to this repository #3

Open Ethiraric opened 7 months ago

Ethiraric commented 7 months ago

yaml-rust2's commit which disallows duplicate keys in mappings can't be instantly merged here since it depends on a small change in ScanError which is in saphyr-parser. When saphyr-parser is released with the new method in ScanError, this can be ported here.

felipesere commented 6 months ago

Is there a chance to put it behind a feature flag? On some projects that handle external YAML I've been forced to pin my serde_yaml version exactly because of this.

Ethiraric commented 6 months ago

What is your use case exactly? There is no need for a feature flag. If I let duplicate keys, there just would be one that you cannot access, without any guarantee on which it would be.

Also, as per 3.2.1.3 in the YAML spec, duplicate keys are an error.

felipesere commented 6 months ago

In my particular case we post-process the outcome of a helm-chart for Kubernetes and there is a chance that annotations or labels keys may contain duplicates. As far as I know, in golang a few places simply use map[string]string for YAML hashes so the last value wins.

Could this maybe be something configurable and the default is to error?

Ethiraric commented 6 months ago

I'll have a look at this once I manage to rework the reader in the parser. As of now, it's difficult for me to make rustc understand how to properly optimize the rewritten code.

The change you suggest should not weigh too much on performance so it should be doable.

felipesere commented 6 months ago

Happy to help and contribute this 😄

bglw commented 1 month ago

+1 to putting this behind a parsing option. Use case is parsing files to extract metadata, where being lenient is preferable to throwing errors, and accuracy isn't critical.