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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

moved generator API to a dedicated package export, solves #1285 #1287

Merged
merged 1 commit into from Nov 22, 2023

Conversation

sailingKieler
Copy link
Contributor

@sailingKieler sailingKieler commented Nov 10, 2023

I named the new export generating, as its content is to be used for generating code.
generators may also work.
generator is IMO a bit misleading as there is no langium generator, and generation means something different. 馃槈

I took the opportunity to:

  • refactored our generators to use 'expandToNode' and friends instead of instantiating 'CompositeGeneratorNodes' (in order to see how far we get with the tagged template based API)

@spoenemann
Copy link
Contributor

spoenemann commented Nov 13, 2023

Thanks! Refactoring langium-cli is great, I had that on my mind for quite some time.

Regarding the name: I agree generator is misleading, but I would propose generate, which is more consistent with test (otherwise it should be testing).

Then I'd also rename src/generator to src/generate.

@spoenemann spoenemann added this to the v3.0.0 milestone Nov 13, 2023
@sailingKieler
Copy link
Contributor Author

Regarding the name: I agree generator is misleading, but I would propose generate, which is more consistent with test (otherwise it should be testing).

Right, switched to generate.

Then I'd also rename src/generator to src/generate.

Done 馃憤

@Lotes
Copy link
Contributor

Lotes commented Nov 15, 2023

Nice work with replacing the CompositeGeneratorNodes with expandToNode.

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.

Thanks! Please modify the main package export test so it excludes the generate code (see #1269).

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.

There are lots of compile errors at the moment. I don't know why the GitHub Action wasn't triggered.

@sailingKieler sailingKieler force-pushed the cs/generatingPackageExport branch 3 times, most recently from 83292cd to a812c24 Compare November 22, 2023 10:01
@sailingKieler
Copy link
Contributor Author

There are lots of compile errors at the moment. I don't know why the GitHub Action wasn't triggered.

I missed to update an import of NEWLINE_REGEX, and a recent change in the yeoman generator test caused a conflict -> no actions where executed.

I would like to merge this PR ASAP, I left a hint in tsconfig.export-main.json, see packages/langium/test/tsconfig.export-main.json

* move NEWLINE_REGEXP from 'template-string.ts' to 'regex-util.ts',
* refactored our generators to use 'expandToNode' and friends instead of instantiating 'CompositeGeneratorNodes'
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.

馃殌

@sailingKieler sailingKieler merged commit 4a963b5 into main Nov 22, 2023
5 checks passed
@sailingKieler sailingKieler deleted the cs/generatingPackageExport branch November 22, 2023 11:23
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.

None yet

3 participants