docs: add a warning for stringify vs. pyyaml#621
docs: add a warning for stringify vs. pyyaml#621jwhitaker-gridcog wants to merge 1 commit intoeemeli:mainfrom
Conversation
|
|
||
| `value` can be of any type. The returned string will always include `\n` as the last character, as is expected of YAML documents. If defined, the `replacer` array or function follows the [JSON implementation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#The_replacer_parameter). See [Options](#options) for more information on the last argument, an optional configuration object. For JSON compatibility, using a number or a string as the `options` value will set the `indent` option accordingly. | ||
|
|
||
| Note that the default output of `stringify` is not safe to pass to YAML 1.1 consumers, e.g. PyYAML. You should set [`compat: true`](#schema-options) in `options` if you are targeting these consumers. |
There was a problem hiding this comment.
I wouldn't specifically mention a certain library here.
It's true that PyYAML itself does not support 1.2 at this point in time, but there is a library for that which you can use on top of PyYAML, and at some point it might be merged upstream.
(just IMHO as a regular pyyaml contributor)
There was a problem hiding this comment.
fwiw I called out pyyaml here because (anecdotally) it's a very common consumer, and (anecdotally) its users often aren't even aware that versioned YAML specs like 1.1 and 1.2 exist, much less the differences in parsing behaviour between them and which one pyyaml uses. As such I suspected the more correct "only works for yaml 1.2 readers" wouldn't be sufficient to warn many people who would be tripped up by this.
eemeli
left a comment
There was a problem hiding this comment.
You're right that defaulting to YAML 1.2 should be highlighted a bit better in the docs, but I don't think this particular language is quite right.
Could you re-file this as an issue instead of a PR?
Heya, here's a proposal for a doco change c.f. #619 . I had read "full support for 1.1 and 1.2" in the project intro so had incorrectly assumed that the default settings would also meet those requirements, which isn't the case. imo it could be worth adding a warning at top-level
stringifydoco to try and make this clear?Cheers