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 yaml parser workaround for anchors #907

Merged
merged 4 commits into from
Apr 19, 2024

Conversation

bastianjoel
Copy link
Member

@bastianjoel bastianjoel added this to the 4.2 milestone Apr 5, 2024
@bastianjoel bastianjoel requested a review from ostcar April 5, 2024 12:36
Copy link
Contributor

@jsangmeister jsangmeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested

@ostcar
Copy link
Member

ostcar commented Apr 8, 2024

I am not a fan. I don't like, that the data it decoded, encoded again and decoded a second time.

I want to try something. If it doesn't work, we can do it this way

@jsangmeister
Copy link
Contributor

jsangmeister commented Apr 9, 2024

I am not a fan. I don't like, that the data it decoded, encoded again and decoded a second time.

Yes, it's not optimal, but it's the best we could come up with. Currently, the parser is faced with two problems:

  • There is no easy way to not parse the _meta key as a model, but as a separate attribute with a different interface
  • The YAML lib does not handle anchors/aliases with different indentations correctly when using custom unmarshalers

This approach solves both of these problems in one go (pun not intended). If you're solution solves those as well in a better way, I would be happy to accept it.

@ostcar
Copy link
Member

ostcar commented Apr 17, 2024

I was not able to find a better solution. So lets use it.

For the future, we could thing about changing the format of the models.yml so something like this:

meta: ...
config:...
other_fields: ...
collection:
motion:
...
agenda_item:
...

@bastianjoel
Copy link
Member Author

We tried something similar but then stumbled over goccy/go-yaml#431

@jsangmeister jsangmeister enabled auto-merge (squash) April 19, 2024 09:33
@jsangmeister jsangmeister merged commit ba53a47 into OpenSlides:main Apr 19, 2024
2 checks passed
@bastianjoel bastianjoel deleted the fix-yaml-meta branch April 19, 2024 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants