aaubry / YamlDotNet

YamlDotNet is a .NET library for YAML
MIT License
2.48k stars 466 forks source link

Process inline comments with DictionaryDeserializer-2 #868

Open evabalint opened 7 months ago

evabalint commented 7 months ago

Hello again, I've structured my modifications in separate classes under separate namespace (Core/ParsingComments/)

evabalint commented 7 months ago

Hi guys, Do you have any thoughts about this request, please?

EdwardCooke commented 7 months ago

I’ll try and take another look later today. When I went through it the first time it felt like a lot code duplication which isn’t great for maintaining it. Give me a few days.

EdwardCooke commented 7 months ago

I haven’t forgotten about this.

filippobottega commented 4 months ago

Hi @evabalint and @EdwardCooke , I've briefly tried to understand the PR implementation but I'm not confident that the overall design is consistent. I suggest you to think about yaml comments as a new layer on top of the Representation Model. Comments could be inline or block and they could be linked with the corresponding yaml node. In the Representation Model each node could be a dotnet object instance, a tag or a property value of an instance. Let me know if I'm wrong. I think that we can leave the Representation Model untouched and we can create a list for each type of yaml node with comments linked to corresponding entities:

  1. List<(object, string)> for comments of instances.
  2. List<(string, string)> for comments of tags.
  3. List<(PropertyInfo, object, string)> for comments of property values.

During serialization we can create these lists and pass theme to the serializer to emit comments accordingly. During deserialization the deserializer could return comments, with others out parameters, generating the corresponding lists. We can wrap the lists in a YamlComments class if you like to use less out parameters as possibile.

This is just a suggestion to enrich the library without changing the current behaviuor.

Thanks for your useful library, Filippo