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

fix: Add coverage check and more samples for OpenSCAD #867

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

nbacquey
Copy link
Member

@nbacquey nbacquey commented Feb 5, 2025

Description

This PR adds the missing coverage checks for OpenSCAD, and adds extra code samples to complete query coverage.

Note: according to my LSP, the following snippet isn't correct OpenSCAD, but the tree-sitter grammar parses it correctly, so I still included it in the PR:

let (a = 1, b = 2,) {};

@mkatychev do you have an opinion on the matter?

Checklist

Checklist before merging, wherever relevant:

@nbacquey nbacquey requested a review from Xophmeister February 5, 2025 11:13
Copy link
Member

@Xophmeister Xophmeister left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Just waiting on the CI's opinion...

@nbacquey nbacquey merged commit d7a4250 into main Feb 5, 2025
10 checks passed
@nbacquey nbacquey deleted the nb/openscad_coverage branch February 5, 2025 12:25
@mkatychev
Copy link
Contributor

mkatychev commented Feb 5, 2025

@nbacquey thanks for the heads up, I added this coverage in #859 but the PR is being held back due to a technical issue.

Note: according to my LSP, the following snippet isn't correct OpenSCAD, but the tree-sitter grammar parses it correctly

I'm assuming you're talking about openscad-LSP, the grammar correctness for the lsp uses tree-sitter-openscad as a crate, I've since added many more edge cases to handle what is technically correct grammar, compiling openscad-lsp with
tree-sitter-openscad.git = https://github.com/mkatychev/tree-sitter-openscad in the Cargo.toml should fix the warnings.

Pinging @Leathong about the grammar changes in my fork of the grammar.

EDIT: filed an issue about it
Leathong/openscad-LSP#35

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.

3 participants