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

Allow comments #451

Open
philippe-lavoie opened this issue Nov 8, 2019 · 7 comments
Open

Allow comments #451

philippe-lavoie opened this issue Nov 8, 2019 · 7 comments
Labels
enhancement feedback-needed More feedback is required help-wanted If you want to help with this issue, let me know!

Comments

@philippe-lavoie
Copy link

I'm updating a node with YamlMapping inside a potentially large Yaml file that has comments. I quickly realized that the library is not handling this correctly right now even for a simple read/write test.

        [Fact]
        public void HandlingComments()
        {
            var yamlContent = @"# Name your package! Package names should contain only lowercase characters
name: 'test_package'
target-path: target  # directory which will store compiled SQL files
models:
  test_package:
      # Applies to all files under models/example/
      example:
          materialized: view";

            var input = new StringReader(yamlContent);
            var yaml = new YamlStream();
            yaml.Load(input);
            var document = (YamlMappingNode)yaml.Documents[0].RootNode;

            var newYaml = new YamlDocument(document);
            var yamlStream = new YamlStream(newYaml);
            var buffer = new StringBuilder();
            using (var writer = new StringWriter(buffer))
            {
                yamlStream.Save(writer, assignAnchors: false);
                var yamlText = writer.ToString();
                Assert.Equal(yamlContent, yamlText);
            }
        }

I understand the standard is vague on comments, however in practice comments are very useful. I'm not asking for full serialiazation / de-serializaton support, but a basic read/update/write flow leveraging the YamlMappingNode directly should work.

Is it possible ?

@aaubry
Copy link
Owner

aaubry commented Nov 23, 2019

It is certainly possible to do that, but it will require a change in YamlDotNet. It is possible to request that comments are returned by the Parser, but the RepresentationModel does not have a way to store them.

I don't which would be the best design for this feature. Two options come to my mind:

  1. Add a YamlComment type, that could appear inside a YamlDocument as any other YamlNode.
  2. Add a Comments property to YamlNode that would capture all comments associated with a node.

I feel that the second option would be cleaner because it wouldn't subvert the YamlNode type hierarchy with something that is not specified as a node on the YAML spec.
Do you think that this would work for you ?

@aaubry aaubry added enhancement feedback-needed More feedback is required help-wanted If you want to help with this issue, let me know! labels Nov 23, 2019
@smaillet
Copy link

I think this is dangerous feature as it is a shift in core functionality of YamlDotNet as an object serializer/Deserializer. Comments should have no place is such operations. YamlDotNet isn't a generalized YAML parser. While it could be built on top of one, there is generally a lot more functionality required of a general parser than is needed for serialize/deserialize and that could increase code bloat. If a generalized parsing is desired, I'd suggest it should be done as a distinct project that this one then consumes.

@aaubry
Copy link
Owner

aaubry commented Mar 21, 2020

That's incorrect, @smaillet . YamlDotNet consists of multiple parts. You should read the documentation to get a better understanding of the library. There's a parser / emitter that can parse or emit any valid YAML, a representation model that provides a higher-level representation of the YAML stream and again supports the whole set of YAML features. Finally, there's the serialization / deserialization components that build on the parser and emitter to do their job.

@cyrildurand
Copy link

Any news on this ?

Add a Comments property to YamlNode that would capture all comments associated with a node.

Is that idea still valid ?

How can we help on this ? I have not look at the source code yet, do you have a rough estimate on how long it would take to implement this ?

@flobernd
Copy link

Is there any news regarding this topic? It's a big showstopper for my current project 😞

@lonix1
Copy link

lonix1 commented Sep 7, 2022

From here:

public class Test
{
    [YamlMember(Description = "A comment")]
    public int Value { get; set; }
}

Maybe good enough for some peoples' use cases.

@ravensorb
Copy link

It is certainly possible to do that, but it will require a change in YamlDotNet. It is possible to request that comments are returned by the Parser, but the RepresentationModel does not have a way to store them.

I don't which would be the best design for this feature. Two options come to my mind:

  1. Add a YamlComment type, that could appear inside a YamlDocument as any other YamlNode.
  2. Add a Comments property to YamlNode that would capture all comments associated with a node.

I feel that the second option would be cleaner because it wouldn't subvert the YamlNode type hierarchy with something that is not specified as a node on the YAML spec. Do you think that this would work for you ?

Give than comments can appear as part of a line or as line on their own (and may also be multi-line) it might make sense to go with option 1 as it would allow more flexibility (including positional notation)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feedback-needed More feedback is required help-wanted If you want to help with this issue, let me know!
Projects
None yet
Development

No branches or pull requests

7 participants