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 scala syntax highlights #1278

Merged
merged 6 commits into from
Dec 18, 2021
Merged

Conversation

dwat3r
Copy link
Contributor

@dwat3r dwat3r commented Dec 16, 2021

Hi!

I really like helix, so I'd like to add scala syntax highlight and LSP support for it.
Amazingly, LSP support with metals works out-of-the-box, although additional metals commands like import build and others obviously needs to be added.

I've copied the tree-sitter highlights and injections from nvim-treesitter, but found a couple of issues with it, and couldn't figure out it yet, so the syntax highlighting is partial currently. I'm happy to receive suggestions how to migrate the whole from nvim.

@dwat3r dwat3r changed the title Add partial scala syntax highlights Add scala syntax highlights Dec 16, 2021
runtime/queries/scala/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/scala/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/scala/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/scala/highlights.scm Outdated Show resolved Hide resolved
@amousset
Copy link
Contributor

I checked open PR before starting #1279 and it happened to be the day another identical PR is opened 😄

To make syntax highlighting work I had to update the tree-sitter-scala submodule.

runtime/queries/scala/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/scala/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/scala/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/scala/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/scala/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/scala/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/scala/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/scala/highlights.scm Outdated Show resolved Hide resolved
@dwat3r
Copy link
Contributor Author

dwat3r commented Dec 17, 2021

I checked open PR before starting #1279 and it happened to be the day another identical PR is opened smile

To make syntax highlighting work I had to update the tree-sitter-scala submodule.
Thanks, this was also required to make scala syntax highlight work :)

I've corrected the syntax, but I'm not sure why helix doesn't color things like objects and other stuff like neovim with the nvim-treesitter.

Here's a comparison screenshot (both are using their own onedark theme (hx: builtin, nvim: ful1e5/onedark.nvim)):
With neovim treesitter:
scala-nvim
helix with this PR:
scala-helix

Also apologies for spoiling some old advent of code for you :D

@archseer
Copy link
Member

It's probably the theme definitions not being defined for every syntax scope. I actually prefer it this way because it strikes a nice balance (is it really necessary to highlight every word? Not a single word in the first image isn't highlighted..)

I'll do some debugging locally though.

@dwat3r
Copy link
Contributor Author

dwat3r commented Dec 17, 2021

You're right, too much highlight is not very good. Although I've missed the functions being highlighted, but your fix made it perfect! :)

I personally think what is really bad in the neovim theme is the use of italics. It almost looks like a different font, and is distracting.

EDIT:
I've noticed indents were not working, so I've quickly copied what nvim-treesitter has in it's folds.scm, and it works most of the time, on multi-line parameters it indents too much, but otherwise it's much better than nothing.

@archseer archseer merged commit 0683f0a into helix-editor:master Dec 18, 2021
@dwat3r dwat3r deleted the scala-syntax branch December 18, 2021 09:54
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