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

Process inline comments with DictionaryDeserializer-2 #868

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

Conversation

evabalint
Copy link

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

  • ParserWithComments : Parser
  • ScannerWithComments : Scanner
    I've created a common base deserializer, which inherits from the base deserializer:
    • DictionaryDeserializerWithComments: DictionaryDeserializer
      and I've created two dictionary deserializer classes:
  • DictionaryNodeDeserializerWithComments : DictionaryDeserializerWithComments
  • StaticDictionaryNodeDeserializerWithComments : DictionaryDeserializerWithComments
    I've created a new value deserializer, called NodeCommentValueDeserializer, which does what the NodeValueDeserializer does, but handles comment-events differently.
    All these modifications can be called from the DeserializerBuilders calling its new function: BuildWithCommentsDeserializer.
    I know, that I did not answer your most important question, why would anyone process comments like data, but that is how we get our input from our clients :( I'm trying to find a solution for it...
    I hope this looks better :) What's your opinion?
    And many thanks,

@evabalint
Copy link
Author

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

@EdwardCooke
Copy link
Collaborator

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
Copy link
Collaborator

I haven’t forgotten about this.

@filippobottega
Copy link

filippobottega commented Feb 19, 2024

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

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.

3 participants