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 LSP for groovy #4303

Closed
wants to merge 4 commits into from
Closed

Conversation

zephaniahong
Copy link

@zephaniahong zephaniahong commented Oct 16, 2022

#4211
Hi! @the-mikedavis
I'm trying to implement LSP and syntax highlighting for groovy. The LSP sorta works though it seems a little slow. But my main problem is trying to implement syntax highlighting. I tried copying the entire highlights.scm and injections.scm from java just to test things out but even that did not work. Any advice?

Thank you!

@archseer
Copy link
Member

There seems to be a separate grammar in development here? I'm not sure how different groovy is from Java https://github.com/codieboomboom/tree-sitter-groovy

file-types = ["groovy", "gvy", "gy", "gsh"]
comment-token = "//"
indent = { tab-width = 2, unit = " " }
language-server = { command = "java", args = ["-jar", "/Users/zephaniahong/Desktop/groovy-language-server/build/libs/groovy-language-server-all.jar"]}
Copy link
Member

Choose a reason for hiding this comment

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

This is hard coded to your setup and shouldn't be upstreamed in this way

Copy link
Author

Choose a reason for hiding this comment

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

Yup! I'm just testing things out to get a better understanding. But the main issue is that, I can't get the syntax highlighting to work

@archseer
Copy link
Member

Try using inherits instead in the same way this was done for purescript 2d958d6

@zephaniahong
Copy link
Author

zephaniahong commented Oct 16, 2022

Tried it but still does not seem to work.
Checked the logs and I'm getting this error:
helix_core::syntax [ERROR] Could not parse queries for language "groovy". Are your grammars out of sync? Try running 'hx --grammar fetch' and 'hx --grammar build'. This query could not be parsed: QueryError { row: 78, column: 1, offset: 1256, message: "comment", kind: NodeType }

I've ensured that I've sync all my grammars by running the mentioned commands but I still get the same error

@archseer
Copy link
Member

If you use tree-sitter-groovy then you need to write queries specific to that implementation and can't just reuse the Java ones.

@zephaniahong
Copy link
Author

zephaniahong commented Oct 16, 2022

I see! I'll give that a shot.
I've changed the tree sitter back to Java and tried using inherits. Any advice on why that implementation does not work? I checked the logs and it did not produce any error. But there is no syntax highlighting. My worry is that if I can't even get groovy to work with something that is already working then I can't begin to work on the custom highlight queries.

The only way I've gotten syntax highlighting to work for groovy is by add the groovy file extension to the java language.

Thank you!

Comment on lines +1851 to +1854
[[grammar]]
name = "groovy"
source = { git = "https://github.com/tree-sitter/tree-sitter-java", rev = "bd6186c24d5eb13b4623efac9d944dcc095c0dad" }

Copy link
Member

Choose a reason for hiding this comment

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

You either need to use grammar = "java" in the [[language]] block or use a tree-sitter-groovy parser - this will fail to load because we're expecting the parsing to expose symbols under the name tree_sitter_groovy but this parser uses tree_sitter_java:

let language_fn_name = format!("tree_sitter_{}", name.replace('-', "_"));

Copy link
Member

Choose a reason for hiding this comment

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

We could probably raise this log level so it shows up in the log by default, as long as this doesn't start showing up in test output:

.map_err(|e| log::info!("{}", e))

Copy link
Author

Choose a reason for hiding this comment

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

I see! I think I understand now(: Thanks!
Also, what do you mean by 'as long as this doesn't start showing up in test output'? Would changing it to log::warning!("{}", e) solve what you're saying?

Copy link
Member

Choose a reason for hiding this comment

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

Looking at the test suite, it isn't actually a problem. The integration tests will print log lines but there isn't an integration test where this returns Error

#4315

@the-mikedavis
Copy link
Member

I don't think groovy is close enough to java syntax-wise to use tree-sitter-java

@zephaniahong
Copy link
Author

Yupp! But I just wanted to play around to make sure I understood the mechanics of how things were working. Once I wrap my head around it, I'll swap it out with tree-sitter-groovy and write queries for it.

the-mikedavis added a commit that referenced this pull request Oct 16, 2022
Info logs don't show up in the log file by default, but this line
should: failures to load tree-sitter parser objects are useful errors.
A parser might fail to load it is misconfigured
(#4303 (comment))
or if the file does not exist.
sudormrfbin pushed a commit that referenced this pull request Oct 16, 2022
Info logs don't show up in the log file by default, but this line
should: failures to load tree-sitter parser objects are useful errors.
A parser might fail to load it is misconfigured
(#4303 (comment))
or if the file does not exist.
@kirawi kirawi added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 1, 2022
pathwave pushed a commit to pathwave/helix that referenced this pull request Nov 6, 2022
Info logs don't show up in the log file by default, but this line
should: failures to load tree-sitter parser objects are useful errors.
A parser might fail to load it is misconfigured
(helix-editor#4303 (comment))
or if the file does not exist.
herkhinah pushed a commit to herkhinah/helix that referenced this pull request Dec 11, 2022
Info logs don't show up in the log file by default, but this line
should: failures to load tree-sitter parser objects are useful errors.
A parser might fail to load it is misconfigured
(helix-editor#4303 (comment))
or if the file does not exist.
@pascalkuthe pascalkuthe added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 1, 2023
@zephaniahong zephaniahong deleted the groovy branch February 12, 2023 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants