Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add YamlLoader::load_from_str_with_markers to provide AST with source markers #125

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hallettj
Copy link

fixes #103, replaces #105

The new function produces an alternative representation for YAML
documents where each YAML node is paired with a Marker to indicate the
corresponding line and column in the source markup. The new
representation takes the form of two new types, Node and YamlMarked.

Node is a pair of YamlMarked and Marker.

YamlMarked mimics the existing Yaml enum; the difference is that array
elements and hash keys and values are Node values instead of Yaml or
YamlMarked values. I created a new enum because I did not know of a way
to switch child nodes in Yaml between Yaml and Node types without
backward-incompatible changes to the Yaml enum.

The the behavior of the existing load_from_str function and Yaml
enum are unchanged, so pattern matching on results from load_from_str
will work as before.

To ensure consistent behavior for the Node and Yaml I moved methods
from the impl Yaml block to a new trait called YamlNode which is
implemented by Yaml, Node, and YamlMarked. This is a breaking
change since it means that consumers will have to import YamlNode to
use methods like .as_str() and .is_array().

I want to present this pull request as one proposal. I think there is also
an argument for changing the existing Yaml type to incorporate source
location markers instead of maintaining two parallel enums.

While making changes I split up yaml.rs into three nested modules.
I can put it back the way it was if that is preferable.

…ce markers

fixes chyh1990#103, replaces chyh1990#105

The new function produces an alternative representation for YAML
documents where each YAML node is paired with a `Marker` to indicate the
corresponding line and column in the source markup. The new
representation takes the form of two new types, `Node` and `YamlMarked`.

`Node` is a pair of `YamlMarked` and `Marker`.

`YamlMarked` mimics the existing `Yaml` enum; the difference is that array
elements and hash keys and values are `Node` values instead of `Yaml` or
`YamlMarked` values. I created a new enum because I did not know of a way
to switch child nodes in `Yaml` between `Yaml` and `Node` types without
backward-incompatible changes to the `Yaml` enum.

The the behavior of the existing `load_from_str` function and `Yaml`
enum are unchanged, so pattern matching on results from `load_from_str`
will work as before.

To ensure consistent behavior for the `Node` and `Yaml` I moved methods
from the `impl Yaml` block to a new trait called `YamlNode` which is
implemented by `Yaml`, `Node`, and `YamlMarked`. This is a breaking
change since it means that consumers will have to import `YamlNode` to
use methods like `.as_str()` and `.is_array()`.

I want to present this pull request as one proposal. I think there is also
an argument for changing the existing `Yaml` type to incorporate source
location markers instead of maintaining two parallel enums.

While making changes I split up `yaml.rs` into three nested modules.
I can put it back the way it was if that is preferable.
@hallettj
Copy link
Author

So I used the derivative crate to define Hash and PartialEq implementations for Node. Derivative uses procedural macros, which breaks compatibility with Rust v1.24. The reason I did that is that derivative makes it easy to ignore certain struct fields in the hash and equality implementations. Because Node values are used as hash keys, and YAML's hash keys do not differ based on source location, the source location marker in the Node struct should be ignored when comparing Node values.

If you want to maintain compatibility with earlier Rust versions I can make changes to remove derivative, and implement Hash and PartialEq by hand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Providing line and column numbers in parsed document
1 participant