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

Bump C3 tree-sitter parser to 0.2.3 #99

Merged
merged 12 commits into from
Jan 24, 2025

Conversation

PgBiel
Copy link
Contributor

@PgBiel PgBiel commented Jan 23, 2025

Keeping it as draft before I run some simple usage tests.

Fixes #91 (fix yet to be confirmed).

Applied suggestions from #91 (comment), except that I added a temporary fix for the AST convert function, where we parse initializer_list directly. That makes the test pass (although isn't a general fix for all cases).

All tests are passing.

@pherrymason
Copy link
Owner

Thanks! Just let me know when you feel is ready

@PgBiel
Copy link
Contributor Author

PgBiel commented Jan 23, 2025

The fix seems to work, however, for some reason, the LSP is now consuming much more CPU on completion compared to the latest version's binary (obtained through nixpkgs). Is there a way to make an optimized build that I'm missing? (I'm using make build-linux)

@pherrymason
Copy link
Owner

Mm now that you mention, go build says CGO could be optimized (cgo is the feature of compiling c code... which refers to the embedded treesitter library )

@PgBiel
Copy link
Contributor Author

PgBiel commented Jan 23, 2025

Now that I think about it, it's possible that it's just doing more work, because now it can parse the entirety of raylib.c3l properly. Before, each raylib file would just be a single error node due to <* *>.

In comparison, the CPU usage seems to be roughly the same for a very simple project.

Perhaps you can confirm this on your own projects? (I don't have many to test on)

Still, it'd be nice to somehow solve this problem, of course.

I wonder if it's somehow related to the usage of the .c3l format specifically, or whether it's constantly rebuilding C files or something, but I'm not sure.
We could consider delaying a proper solution to this problem if we can confirm that it's not that different from before.

@PgBiel
Copy link
Contributor Author

PgBiel commented Jan 24, 2025

I've found the problem :)
Opened a separate PR to fix it: #100

@pherrymason
Copy link
Owner

Is this PR ready then?

@PgBiel PgBiel force-pushed the bump-tree-sitter-0.2.3 branch from ebada1e to da28d4b Compare January 24, 2025 17:48
@PgBiel PgBiel marked this pull request as ready for review January 24, 2025 18:03
@PgBiel
Copy link
Contributor Author

PgBiel commented Jan 24, 2025

Added unit tests and tested a bit more locally as well. Should be ready for merge 😄

@pherrymason pherrymason merged commit 825a617 into pherrymason:main Jan 24, 2025
2 checks passed
@PgBiel PgBiel deleted the bump-tree-sitter-0.2.3 branch January 25, 2025 05:59
@pherrymason pherrymason mentioned this pull request Jan 28, 2025
32 tasks
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.

Go to definition doesn't work for functions with doc-strings
2 participants