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

highlight calls to erlang modules as types #5

Merged
merged 2 commits into from
Oct 12, 2021
Merged

highlight calls to erlang modules as types #5

merged 2 commits into from
Oct 12, 2021

Conversation

the-mikedavis
Copy link
Member

👋 hello!

I'm integrating the new parser and highlight queries into the helix editor (see helix-editor/helix#830) and I noticed that a function call on an atom highlights differently than an alias (module). This PR adds a clause to the highlights to recognize that case and highlight the atom the same as an alias:

highlight-atom-modules-as-types

before | after

What do you think?

@jonatanklosko
Copy link
Member

jonatanklosko commented Oct 12, 2021

Hey @the-mikedavis! I intentionally didn't do it, because I'm not sure if atom should be colored differently based on context. On one side both tokens refer to a module, but in the end atoms and aliases are different beings. It's certainly subjective, but for some reference makup_elixir does the same.

@josevalim any preference?

@josevalim
Copy link
Member

I am thinking they could be the same. It is already syntactically distinct so having them visually the same can help drive the point home they are all modules?

@jonatanklosko
Copy link
Member

I'm not really opinionated here, so as you two agree it sounds perfectly fine to me!

@the-mikedavis
Copy link
Member Author

I think the consistency with makeup is pretty desirable. I'd be willing to try out a PR to makeup to align the behavior.

here are some more snippets from a real code-base for comparison...

crypto-compare

per-message-deflate-compare

before | after

@jonatanklosko
Copy link
Member

Ah, actually this matches the behaviour in nvim-treesitter/nvim-treesitter#1904 too :)

@jonatanklosko
Copy link
Member

Thanks! 🐱

@jonatanklosko jonatanklosko merged commit 24a9eae into elixir-lang:main Oct 12, 2021
@the-mikedavis the-mikedavis deleted the highlight-erlang-modules-as-types branch October 12, 2021 21:11
the-mikedavis added a commit to the-mikedavis/helix that referenced this pull request Oct 12, 2021
the-mikedavis added a commit to the-mikedavis/helix that referenced this pull request Oct 12, 2021
@connorlay
Copy link
Contributor

Ah, actually this matches the behaviour in nvim-treesitter/nvim-treesitter#1904 too :)

Yep, which was me following what I saw in exdocs snippets, bringing things around full circle 😆

the-mikedavis added a commit to the-mikedavis/helix that referenced this pull request Oct 16, 2021
the-mikedavis added a commit to the-mikedavis/helix that referenced this pull request Oct 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants