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 EBNF grammar as doc comments for TerminalKind and NonterminalKind #1169

Merged
merged 33 commits into from
Dec 12, 2024

Conversation

mjoerussell
Copy link
Contributor

@mjoerussell mjoerussell commented Nov 27, 2024

When generating documentation for TerminalKind and NonterminalKind, generate the EBNF grammar for the current variant and include that in the final output. This affects the Wit output and Rust output (using templates cst.wit.jinja2 and (terminal|nonterminal)_kind.rs.jinja2).

Closes #1165

… .d.ts files using the `jco types` command.

This command is better for our use case than `jco transpile` because it generates the declarations directly from the source .wit files, whereas `jco transpile` uses the WASM binaries to generate them. This means that `jco types` has access to the documentation that's written in the wit interfaces, and `jco transpile` does not.

Since we're generating the interfaces in a separate step, I'm disabling typescript in the original `jco transpile` step. This is just a precaution to avoid confusion or possible conflicting declaration files interferring with each other.
…is correctly propagating through the build process.
…rpose of this is so that both processes use the same `CodegenFileSystem` instance. If they used separate instances, then the files generated in one function would get deleted by the next.
…the variant ID as well as the EBNF form of the kind to use as documentation. Getting the EBNF form involves creating an instance of `SpecModel` and serializing the ID using that.
…guage` field to be a `&Language` instead of an `Rc<Language>` because of some weird issue getting `KindBuilder` to instantiate properly given that `KindsModel::from_language` received a `&Rc<Language>` param. However, I realized afterwards that I could just use `to_owned` when creating the SpecModel instead of changing SpecModel everywhere.
@mjoerussell mjoerussell self-assigned this Nov 27, 2024
@mjoerussell mjoerussell requested review from a team as code owners November 27, 2024 17:02
Copy link

changeset-bot bot commented Nov 27, 2024

⚠️ No Changeset found

Latest commit: ce50167

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

… for the clippy lints `doc_markdown` and `doc_link_with_quotes`. These are "pedantic" lints that we run into issues with now that we're outputting the EBNF docs for these kinds, but the errors aren't relevant to us.
Copy link
Contributor

@OmarTawfik OmarTawfik left a comment

Choose a reason for hiding this comment

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

Passing back to you, for a few suggestions, and for resolving the conflicts with main.

Thank you!

P
Put EBNF docs for TerminalKind and NonterminalKind in code blocks
Cargo.lock Show resolved Hide resolved
Copy link
Contributor

@OmarTawfik OmarTawfik left a comment

Choose a reason for hiding this comment

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

Updated the comments.
Thank you for looking into this!

@OmarTawfik OmarTawfik added this pull request to the merge queue Dec 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 12, 2024
@OmarTawfik OmarTawfik added this pull request to the merge queue Dec 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 12, 2024
@OmarTawfik
Copy link
Contributor

looks like this needs rebasing before merge.

@OmarTawfik OmarTawfik added this pull request to the merge queue Dec 12, 2024
Merged via the queue into NomicFoundation:main with commit 8eea9e5 Dec 12, 2024
1 check passed
github-merge-queue bot pushed a commit that referenced this pull request Dec 12, 2024
Updates our JCO submodule to include the changes in
NomicFoundation/jco#5. Includes one other change to fully integrate
these changes, which is to change `NodeVariant` to `NodeType` in
`codegen/../cst/index.mts` so that the expected imports align with the
new generated exports.

* [x] Depends on #1169
* [x] Depends on NomicFoundation/jco#5

Closes #1166 
Closes #1171
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.

add EBNF grammar to node kinds
2 participants