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

Fix yaml parsing #110

Merged
merged 6 commits into from
Feb 2, 2024
Merged

Fix yaml parsing #110

merged 6 commits into from
Feb 2, 2024

Conversation

rquitales
Copy link
Member

@rquitales rquitales commented Nov 28, 2023

This is a stacked PR on top of #107 (the last 3 commits are the new additions) that modifies how we interact with yaml strings in our AST parsing. Instead of using the String() method on nodes, which requires mutating the nodes to handle comments properly, this PR updates our codebase to use the Value method of the ast nodes instead. This gives us better output with correct formatting, eg. correct handling of octal values of file permissions and better multiline string formatting.

Fixes: #109
Fixes #112
Fixes: #115

@rquitales rquitales requested review from EronWright and a team November 28, 2023 19:58
Copy link
Contributor

@EronWright EronWright left a comment

Choose a reason for hiding this comment

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

I believe you still need to migrate the code in resourceName and getHeader away from String().

Please update your testdata to include some yaml tags, to verify correct handling.

@rquitales rquitales force-pushed the rquitales/fix-yaml-parsing branch from c9f0ce4 to f29ebda Compare January 9, 2024 17:16
@rquitales rquitales requested a review from EronWright January 18, 2024 18:13
Copy link
Contributor

@EronWright EronWright left a comment

Choose a reason for hiding this comment

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

LGTM

@rquitales rquitales merged commit 2fcda0e into master Feb 2, 2024
2 checks passed
@rquitales rquitales deleted the rquitales/fix-yaml-parsing branch February 2, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants