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 publish keyword #571

Merged
merged 19 commits into from
May 28, 2024
Merged

Add publish keyword #571

merged 19 commits into from
May 28, 2024

Conversation

georg-schwarz
Copy link
Member

@georg-schwarz georg-schwarz commented May 23, 2024

Partially addresses #563: Introduces publish keyword.

  • Extended the grammar with the keyword
  • Adjusted scoping to only make published elements available in other files
  • Added validation to avoid cyclic imports
  • Published all builtin elements
  • Adapted tests

As discussed in eclipse-langium/langium#1508 (reply in thread), the main logic resides in the JayveeScopeProvider instead of the JayveeScopeComputation.

Notes

  • Decide whether we should add caching (as suggested in the thread) right away or track it in a separate issue: already implemented, only 3 lines of code.
  • Validate code actions of Import Code Actions #565 still work
  • Validate that cyclic dependencies are prevented
  • Add user documentation

/**
* Follow an export statement to its original definition.
*/
protected followExportDefinitionChain(
Copy link
Member Author

Choose a reason for hiding this comment

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

If we should ever allow publish * from '{path}', we'd have to use recursion here I think to follow the chain.. Right now, the exported element is directly referenced.

@georg-schwarz georg-schwarz marked this pull request as ready for review May 26, 2024 16:29
@georg-schwarz georg-schwarz requested a review from rhazn May 26, 2024 16:49
@georg-schwarz georg-schwarz mentioned this pull request May 27, 2024
Copy link
Contributor

@rhazn rhazn left a comment

Choose a reason for hiding this comment

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

Had a high level look, overall looks good to me, great work 👍 . Just some small codestyle / optional comments.

@georg-schwarz georg-schwarz merged commit f5e5cc8 into main May 28, 2024
3 checks passed
@georg-schwarz georg-schwarz deleted the publish-elements branch May 28, 2024 08:46
@github-actions github-actions bot locked and limited conversation to collaborators May 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants