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

languages: add svelte support #733

Merged
merged 2 commits into from
Sep 17, 2021
Merged

languages: add svelte support #733

merged 2 commits into from
Sep 17, 2021

Conversation

happysalada
Copy link
Contributor

I'm opening this prospecive PR just to see if I haven't made anything stupid.

I've added the tree-sitter gitmodule
added all the queries file that were in the tree-sitter repository.

I need to update my nix to include the latest PR that downloads the submodules automatically.
I should have time to test this in the next few days.

Copy link
Member

@kirawi kirawi left a comment

Choose a reason for hiding this comment

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

(By the way, you're missing the final newline on a few of these files).

runtime/queries/svelte/folds.scm Outdated Show resolved Hide resolved
@@ -0,0 +1,68 @@
; Special identifiers
Copy link
Member

Choose a reason for hiding this comment

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

Some of these need to be converted into Helix's scopes: https://docs.helix-editor.com/themes.html

Copy link
Member

@kirawi kirawi Sep 9, 2021

Choose a reason for hiding this comment

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

@archseer Do we want to also add the none scope?

runtime/queries/svelte/indents.scm Outdated Show resolved Hide resolved
@happysalada
Copy link
Contributor Author

I've added the newlines, and switched from scm to toml for the indent file.
Regarding the highlights changes, understood. I need to test this to make meaningful changes. I'll update as soon as I could try it.

Thank you for the review!

runtime/queries/svelte/injections.scm Show resolved Hide resolved
runtime/queries/svelte/indents.toml Outdated Show resolved Hide resolved
runtime/queries/svelte/highlights.scm Outdated Show resolved Hide resolved
languages.toml Outdated Show resolved Hide resolved
":"
"/"
"@"
] @punctionation.definition.tag
Copy link
Member

@kirawi kirawi Sep 9, 2021

Choose a reason for hiding this comment

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

Typo punctionation vs. punctuation. I also think the brackets should be punctuation.bracket. Not sure about the rest. I'm not familiar with JS/Svelte, so what do the others do? Depending on what it is, we may be able to include another scope for tag, variable, or keyword.

@happysalada
Copy link
Contributor Author

Just to give you more context, svelte is a bit like vue, it's a file that contains a mix of html and javascript (or typescript).
Here is how it looks like in a hypothetical inefficient editor
(Note that there are some specificities around { brackets in the html)

Screen Shot 2021-09-10 at 11 13 46

Screen Shot 2021-09-10 at 11 14 03

@happysalada
Copy link
Contributor Author

Ok, I just had time to test this, it doesn't work at all.
Svelte is mostly injections, so I'll have a look at fixing that file first.
I'm not sure why the lsp doesn't do anything though. Not sure how to activate/debug.

@archseer
Copy link
Member

LSP: Is the binary in your $PATH? Run helix with the -v flag and post the log file.

@happysalada
Copy link
Contributor Author

binary is in my path

❯ which svelteserver
/Users/raphael/.nix-profile/bin/svelteserver

it seems to run actually, it just doesn't display anything.
Here are the logs
helix.log

@archseer
Copy link
Member

binary is in my path

❯ which svelteserver
/Users/raphael/.nix-profile/bin/svelteserver

it seems to run actually, it just doesn't display anything.
Here are the logs
helix.log

It seems to load just fine, are you able to get any completions using ctrl-x in insert mode? Or k over some item to see some documentation etc. I can see the LSP initialization but no interaction with the server.

@happysalada
Copy link
Contributor Author

I wasn't aware of ctrl-x.
it does work indeed. ctrl-x generates completion. typing some wrong code also gives errors in the editor.
There are just the injections left then. Let me update this in the next few days.

@happysalada
Copy link
Contributor Author

I've modified the injections to be of the correct format (I think), however those are not working. a svelte file is not being syntax hyghlighted.
I've checked the logs with -v and there are no error or indication that something is going wrong.
Not really sure what to try next.

@archseer
Copy link
Member

Can you post a sample .svelte file? I'll give it a go.

@happysalada
Copy link
Contributor Author

index.svelte.txt

(the normal extension is just .svelte , I added a .txt to enable github upload, feel free to remove the .txt once you've downloaded it)
That should be a fairly standard one. Let me know if I can help in any way.

@archseer
Copy link
Member

Fixed in dd0b15e! It worked on css but not for typescript or javascript. I'll let you do a quick test, but other than that I think we're ready for merge.

@happysalada
Copy link
Contributor Author

The injections didn't work on a style of script attribute for me.
I'm happy to merge though, as having a language server is an infinite improvement.
Plus now that I know where the injections are happening in the code, it gives me a strong incentive to go have a look and figure it out.

Thanks a lot for the help and the time spent on this!

@archseer
Copy link
Member

The injections didn't work on a style of script attribute for me.

They don't work for you? <script lang="ts"> was highlighted correctly for me after the patch in master.

@happysalada
Copy link
Contributor Author

I rebased this branch on master this morning, made sure I had my branch running locally.
I can confirm that I don't get highlights inside script or style tags.
I've turned verbosity on and saw nothing in the logs either.
I'm planning on having a look at the commit you pointed out and how the highlighting works in the next few weeks.

@archseer
Copy link
Member

Make sure you cargo clean -p helix-syntax and clear out runtime/grammars/ before building, maybe that will fix it.

@happysalada
Copy link
Contributor Author

cool, let me check.

@happysalada
Copy link
Contributor Author

Oh no, just realized that can't be it.
I'm building with the flake directly. So everytime, it compiles from scratch (some of the dependencies are cached of course).

@archseer
Copy link
Member

pos

Here's the file you provided for example, after building on this branch from scratch.

@happysalada
Copy link
Contributor Author

this looks beautiful!

ok let me try to build without the flake then.

@archseer archseer merged commit b2195e0 into helix-editor:master Sep 17, 2021
@happysalada
Copy link
Contributor Author

Thanks a lot, local compilation did work, it must be a problem with the flake (or more probable, my usage of it).

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