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 syntax highlighting and LSP (erlang_lsp) for Erlang #7093

Merged

Conversation

codeadict
Copy link
Contributor

@codeadict codeadict commented Jan 30, 2024

This pull request implements support for the Erlang Language.

It adds:

  • tree-sitter-erlang grammar
    highlights (Licensed under Apache-2 from WhatsApp which is compatible with Zed licensing model), folds and indents
  • Erlang file icon based on the official one
  • erlang_ls support

Fixes #4939, possibly a duplicate of #7085 with more features. Suppose @wingyplus wants to join efforts here.

To complete (out of scope for this PR):

  • Support for the ELP language server from WhatsApp. CC @robertoaloi
  • Better indentation handling, need something like indentNextLinePattern in VS Code

Screenshots:

Screenshot 2024-01-30 at 11 03 51 AM
Screenshot 2024-01-30 at 11 01 19 AM
Screenshot 2024-01-30 at 11 01 37 AM
Screenshot 2024-01-30 at 11 02 03 AM
Screenshot 2024-01-30 at 11 02 19 AM

Outline:
Screenshot 2024-01-31 at 9 09 36 AM

Release Notes:

Copy link

cla-bot bot commented Jan 30, 2024

We require contributors to sign our Contributor License Agreement, and we don't have @codeadict on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

1 similar comment
Copy link

cla-bot bot commented Jan 30, 2024

We require contributors to sign our Contributor License Agreement, and we don't have @codeadict on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jan 30, 2024
@codeadict codeadict mentioned this pull request Jan 30, 2024
1 task
"undef"
"unit"
"when"
"xor"] @keyword
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to move and, or, not, and xor to @keyword.operator since it's the same as andalso and orelse?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and, or, xor, and not are boolean operators and do not control flow. orelse and andalso are control operators. For the highlighter they look the same but they are semantically different which I think makes sense to differentiate :

Screenshot 2024-01-31 at 9 18 46 AM

@codeadict
Copy link
Contributor Author

codeadict commented Jan 31, 2024

This is ready for review now. Have tested it with a few big Erlang projects and is working great.

@codeadict codeadict force-pushed the erlang_syntax_highlighting branch 2 times, most recently from 0d2dda2 to 64052a0 Compare January 31, 2024 21:50
Copy link
Contributor

@SomeoneToIgnore SomeoneToIgnore left a comment

Choose a reason for hiding this comment

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

Great scm files, thank you for creating a PR.

There are a few style nits and one odd "gitmodules": "vcs", file association removal, which does not look right.
Let's fix all together and merge this?

assets/icons/file_icons/file_types.json Show resolved Hide resolved
crates/zed/Cargo.toml Show resolved Hide resolved
@SomeoneToIgnore
Copy link
Contributor

SomeoneToIgnore commented Feb 1, 2024

One other thing I'm interested about: I have tried to run the branch, with zero Erlang experience before.
I've installed erlang via brew and erl seem to work, cloned https://github.com/erlang/otp and tried to run Zed in its root.
And in the stderr, I see

[2024-02-01T15:06:03+02:00 ERROR project] failed to start language server "erlang_ls": No such file or directory (os error 2)
[2024-02-01T15:06:03+02:00 ERROR project] server stderr: Some("")

and no erlang-related entries in Zed's LSP logs.
I have ~/Library/Application Support/Zed/languages/erlang_ls directory, but it's empty.

What am I doing wrong? If nothing seems so, could you try removing that language directory locally, and recheck that part?

@wingyplus
Copy link
Contributor

One other thing I'm interested about: I have tried to run the branch, with zero Erlang experience before. I've installed erlang via brew and erl seem to work, cloned https://github.com/erlang/otp and tried to run Zed in its root. And in the stderr, I see

[2024-02-01T15:06:03+02:00 ERROR project] failed to start language server "erlang_ls": No such file or directory (os error 2)
[2024-02-01T15:06:03+02:00 ERROR project] server stderr: Some("")

and no erlang-related entries in Zed's LSP logs. I have ~/Library/Application Support/Zed/languages/erlang_ls directory, but it's empty.

What am I doing wrong? If nothing seems so, could you try removing that language directory locally, and recheck that part?

@SomeoneToIgnore As far as I know, the adaper didn't download the erlang_ls for you (see from https://github.com/zed-industries/zed/pull/7093/files#diff-20bc59f7eda9211db28f8a2d600c59bb7b2b6fa816c68d9a929a7b2e4f661a7dR32). So the erlang_ls binary needs to present in your PATH.

@SomeoneToIgnore
Copy link
Contributor

Oh, I see that https://github.com/erlang-ls/erlang_ls/releases/tag/0.50.0 does not have any mac binaries, so that's why, most probably: Zed does not have the code to download LSP server binaries elsewhere.
Thank you for the explanation!

(callback fun: (atom) @function)

;; wild attribute
(wild_attribute name: (attr_name name: (atom) @keyword))
Copy link
Contributor

Choose a reason for hiding this comment

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

I found a bit of weird highlighting for wild_attribute node. It got highlighted as an atom (string.special.symbol) like screenshot below.

Screenshot 2567-02-01 at 21 45 38

You can see that -dialyzer should be highlight as same as -record and friends. After moving the wild_attribute node to under atom at line 229, it works.

I'm not sure if it's the right fix. My tree-sitter knowledge is limited.

Copy link
Contributor

Choose a reason for hiding this comment

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

Submit a fix at codeadict#1. 🙇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the problem here is that dialyzer is not defined as a keyword and acts as an atom

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would not put primitives so high in the file and rather move the wild attributes to override them. I have done that and here is the result:
Screenshot 2024-02-01 at 11 29 54 AM

@codeadict
Copy link
Contributor Author

Thanks for the help @wingyplus and @SomeoneToIgnore . I'll rebase with main now and fix the conflicts. You need to clone erlang_ls and run make, then make install to have it in the path. I'll submit a patch to Erlang_ls to release OSX binaries , when that is done I will submit another PR here to fetch the LS

@wingyplus
Copy link
Contributor

LGTM! 👌

Copy link
Contributor

@SomeoneToIgnore SomeoneToIgnore left a comment

Choose a reason for hiding this comment

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

Awesome to see such cooperative desire to bring this in, so let's do that.
Good luck with the macOs langserver releases!

@SomeoneToIgnore SomeoneToIgnore merged commit 97be0a9 into zed-industries:main Feb 1, 2024
5 checks passed
@codeadict
Copy link
Contributor Author

Thank you @SomeoneToIgnore and @wingyplus for the help. More things to come for Erlang soon...

@robertoaloi
Copy link

Thanks for the heads up @codeadict . Anything you'd need from us to get WhatsApp's ELP integration rolling?

@eproxus
Copy link

eproxus commented Mar 13, 2024

I've used the Erlang support for a while, and it works really well so far! I have a few comments on the syntax highlighting though (not sure what the root cause is, so just posting here hoping someone can at least explain it) 🙂

  • Module names, function definitions, function names, atoms and "keys" (atoms as keys in maps, for example) are all highlighted as atom which makes it not very useful
  • Macros and variables are both highlighted as var which also makes it harder to distinguish between them

Technically in Erlang module and function names are atoms, but in terms of syntax highlighting it is not very useful. Would it be possible to further specify subtypes or enhance this in someway? Other languages have e.g. special types for both function definitions and calls which makes them highlight very nicely.

Here's an example where pretty much only two colors are used (not counting the arity integer):

Screenshot 2024-03-13 at 10 38 36

With some themes (such as Ayu Dark) they are even set to the same color (!).

@wingyplus
Copy link
Contributor

Hi @eproxus, sorry for a bit late. I think we can modify the highlight query to support the function. I remember I have been doing this before this PR opened. So I will find the time in this week to make it better. 🙇‍♂️

@eproxus
Copy link

eproxus commented Apr 15, 2024

@wingyplus Do you know if it is possible with tree sitter and Zed to highlight all the differences mentioned?

  1. Module names
  2. Function definitions
  3. Function calls
    1. Internal (local calls)
    2. External (fully qualified calls, e.g. module:function(...), even to the same module)
  4. Atoms as map keys

@codeadict
Copy link
Contributor Author

@eproxus - I have been out for a while, busy with work. I saw your message on the Erlang Slack, I'll be looking into highlighting the differences you mentioned. Will also add support for the ELP LS that has semantic highlighting support and can help with these things too.

@eproxus
Copy link

eproxus commented May 14, 2024

@codeadict Not stress ☺️ Let me know if you need help with testing.

;;


;; First match wins in this file
Copy link

Choose a reason for hiding this comment

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

Seems like in zed topmost queries have lower priority. I.e. (atom) @string.special.symbol in this file overrides color for (call expr: (atom) @function)

@notpeter

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Erlang support
7 participants