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

Use document paths for json serializer references #787

Merged
merged 5 commits into from
Dec 9, 2022

Conversation

msujew
Copy link
Member

@msujew msujew commented Nov 28, 2022

To facilitate grammar loading without resorting to using the linker, we now use document paths for the json serializer. This makes some functions async which were previously sync, but these were used for testing purposes only anyway.

Also introduces a function to copy AST nodes, as we now need deep copies of AST nodes for the langium-cli to correctly embed referenced grammars.

@msujew msujew added the polish Some feature needs improvement label Nov 28, 2022
@msujew msujew requested a review from spoenemann November 28, 2022 12:19
Copy link
Contributor

@spoenemann spoenemann left a comment

Choose a reason for hiding this comment

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

This is a great step forward!

packages/langium/src/grammar/references/grammar-scope.ts Outdated Show resolved Hide resolved
packages/langium/src/serializer/json-serializer.ts Outdated Show resolved Hide resolved
packages/langium/src/serializer/json-serializer.ts Outdated Show resolved Hide resolved
packages/langium/src/serializer/json-serializer.ts Outdated Show resolved Hide resolved
packages/langium/src/serializer/json-serializer.ts Outdated Show resolved Hide resolved
packages/langium/src/utils/ast-util.ts Outdated Show resolved Hide resolved
packages/langium/src/utils/grammar-util.ts Outdated Show resolved Hide resolved
packages/langium/src/utils/grammar-util.ts Outdated Show resolved Hide resolved
@msujew msujew force-pushed the msujew/json-serialize-reference branch from bb8c724 to 888f719 Compare December 7, 2022 16:22
@msujew msujew requested a review from spoenemann December 8, 2022 12:31
Copy link
Contributor

@spoenemann spoenemann left a comment

Choose a reason for hiding this comment

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

Let's ship it! I have two more remarks though:

packages/langium/src/serializer/json-serializer.ts Outdated Show resolved Hide resolved
packages/langium/src/serializer/json-serializer.ts Outdated Show resolved Hide resolved
@msujew msujew force-pushed the msujew/json-serialize-reference branch from 3695a41 to 6b723bd Compare December 9, 2022 10:56
@msujew msujew merged commit b2b3404 into main Dec 9, 2022
@msujew msujew deleted the msujew/json-serialize-reference branch December 9, 2022 12:43
@spoenemann spoenemann added this to the v1.0.0 milestone Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
polish Some feature needs improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants