Skip to content

Conversation

@pgavlin
Copy link
Member

@pgavlin pgavlin commented Apr 24, 2025

These changes add a new node type, DocumentNode, to the syntax package. This node allows concrete syntaxes to attach document-level syntactical trivia (e.g. comments, metadata, etc.) that is not otherwise associated with a node.

These changes also replace *RotationResult with RotationResult, as the result is already nilable. This fixes an issue when running the eval tests with PULUMI_ACCEPT set.

Fixes #260.

Copy link

@borisschlosser borisschlosser left a comment

Choose a reason for hiding this comment

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

lgtm! Still asking others who are more familiar with the code to approve.

pgavlin and others added 3 commits April 24, 2025 13:50
These changes add a new node type, DocumentNode, to the syntax package.
This node allows concrete syntaxes to attach document-level syntactical
trivia (e.g. comments, metadata, etc.) that is not otherwise associated
with a node.

These changes also replace `*RotationResult` with `RotationResult`, as
the result is already nilable. This fixes an issue when running the
eval tests with `PULUMI_ACCEPT` set.

Fixes #260.
Co-authored-by: Boris Schlosser <[email protected]>
@pgavlin pgavlin force-pushed the pgavlin/document-syntax branch from 6f14e52 to fa8ef86 Compare April 24, 2025 20:50
@pgavlin
Copy link
Member Author

pgavlin commented May 6, 2025

So I haven't merged this yet because I'm not totally sure that it's the right approach.

Part of me feels like this is shoehorning a YAML concept into the generic syntax layer, and that a better approach would be to somehow capture this in YAMLSyntax. Any thoughts here?

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.

Top level comments are dropped during auto-formatting

3 participants