Skip to content

Add syntax highlight to function calls #506

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

Closed
wants to merge 6 commits into from
Closed

Add syntax highlight to function calls #506

wants to merge 6 commits into from

Conversation

w-sanches
Copy link
Contributor

Description

Function call highlight was added to elixir syntax highlight on Sublime Text and I thought that was a neat addition as it makes it easier to scan what are function vs variables in the code. I propose we can also add this to our Vim syntax highlight.

Result

image

This supports function calls without parenthesis only if prepended by
the module name, otherwise parenthesis are mandatory for a function call
to be detected
Copy link

@CaiqueMitsuoka CaiqueMitsuoka left a comment

Choose a reason for hiding this comment

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

Nice ✨

@jbodah
Copy link
Collaborator

jbodah commented Aug 29, 2019

I like this. The only things I'm not sure of:

  • The default highlight group is set to Function. This seems odd to me - I'd expect colorschemes to bind to elixirFunctionCall to get this behavior and for the default to be unhighlighted

  • Let's add more tests, particularly some of the interesting ones like ones with bangs, spaces, no parens, etc

@w-sanches
Copy link
Contributor Author

@CaiqueMitsuoka @jbodah just added more tests, please LMK if I forgot some important test.

I did set the default group to Function just so they are consistently coloured throughout the file (either call or definition, they are still functions).
One can always set different colours to elixirFunctionCall and elixirFunctionDefinition if they wish and overwrite this behaviour. IIRC this is the behaviour for C files, but not the same for Ruby so I don't think there's some consistency through different languages.


it 'does not highlight terms without a parentheresis' do
expect(<<~'EOF').not_to include_elixir_syntax('elixirFunctionCall', 'func')
func
Copy link
Collaborator

@jbodah jbodah Aug 30, 2019

Choose a reason for hiding this comment

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

This seems odd and I don't know what the solution here is. This causes only some function calls to be highlighted by default:

image

The first two hd instances come back as elixirId rather than elixirFunctionCall

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 have to confess I completely forgot we could use atoms as module names. Just pushed a commit to fix that.

Also, while at it I realised |> func should also be highlighted since if we are piping it is not ambiguous that we are calling a function.

@jbodah
Copy link
Collaborator

jbodah commented Sep 3, 2019

I want to give this a bit more manually testing; will get back to this by end of week

@denvaar
Copy link

denvaar commented Mar 8, 2020

Can this be merged?

@jbodah
Copy link
Collaborator

jbodah commented Mar 11, 2020

The only thing I'm iffy on in this branch is that functions without args and parens aren't detected which feels misleading. I am okay to merge this as long as we match these to elixirFunctionCall and don't link this to Function by default (due to the above inconsistency). Users can define a link themselves if they want this functionality and are okay with the inconsistency. Macros are also a weird grey area (especially structural ones like if and with)

The original branch was removed, so I will copy this over to a new branch, remove the link, and merge

@jbodah jbodah closed this Mar 11, 2020
jbodah added a commit that referenced this pull request Mar 11, 2020
Implementation from #506

cc: Thanks @w-sanches
@jbodah jbodah mentioned this pull request Mar 11, 2020
jbodah added a commit that referenced this pull request Mar 11, 2020
Implementation from #506

cc: Thanks @w-sanches
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.

4 participants