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 the function call pattern to syntax #179

Merged

Conversation

CaiqueMitsuoka
Copy link
Contributor

@CaiqueMitsuoka CaiqueMitsuoka commented Aug 16, 2019

Description

Add the pattern to scope the function calls with parenthesis or dot and
parenthesis. Allowing themes to change the color of function calls.

Outdated, please look the comment
highlighted function calls

Motivation

Currently the Elixir syntax does not support function calls and it simple does not evaluate a scope for it. This happens mainly because in Elixir you can call a function without the parenthesis, so variables and function can look the same.

In this context we have some considerations:

The compile raise a ambiguity warning

In Elixir community we that the quote "Explicit is better than implicit" so the compiler will warn the developer about a call that looks like a variable but actually is a function call.
The warning raised by the compiler

The code formatter

By default it will add the parenthesis to ambiguous calls.
Formatter adding the parenthesis to a function call

Ambiguous calls will stay exactly the same

Thats it, if you enjoy the ambiguous calls they will stay the same.

Add the pattern to name the function calls with parenthesis or dot and
parenthesis.

There is a full explanation about why, on the GitHub PR of this commit.
TL;DR Since function calls without the parenthesis raise a compile
warning and the formatter add the parenthesis by default, we should
support a rich experience for those who follow the language
recomendation.
@josevalim
Copy link
Contributor

@CaiqueMitsuoka I would rather remove the support for ambiguous calls. They only exist today for backwards compatibillity and they will eventually be removed. Today there is no reason to keep them. What do you think?

@josevalim
Copy link
Contributor

To clarify the above: only call is ambiguous and it should always be treated as a variable. call() is always a call. Both Test.foo and Test.foo() are calls too.

@CaiqueMitsuoka
Copy link
Contributor Author

CaiqueMitsuoka commented Aug 27, 2019

Added support for the calls without parenthesis but with module and spaces between the parenthesis/function/module.

Orange are the valid function calls and the last 3 are cases that it should not kick the highlight.
function calls highlighted and some cases that it should not

@CaiqueMitsuoka
Copy link
Contributor Author

CaiqueMitsuoka commented Aug 27, 2019

Credits to @britto that suggested the refactors of the 81cfaa4.

@josevalim josevalim merged commit f61a00b into elixir-editors:master Aug 27, 2019
@josevalim
Copy link
Contributor

❤️ 💚 💙 💛 💜

@binaryseed
Copy link
Contributor

Hi folks, this PR has made a huge unexpected change in my editor... all of a sudden most of my code turned green!

After looking at this, I'm wondering if its the right choice for the scope for a function call to be the same as the function definition.

Having both use the same scope name overloads the "meaning" of entity.name.function, since defining and calling a function are quite different and the frequency of occurrence are quite different..

Any thoughts? Is there another common scope name for a function call?

@CaiqueMitsuoka
Copy link
Contributor Author

Hi @binaryseed
I see how it can impact, I would say that this is expected since functions are big part of our code bases.
It is expected to be a big change, as @josevalim said we only supported the ambiguous calls for backwards compatibility and the plan is to remove it, and our support was by not supporting it haha.

Here is the language grammar that is followed by most of the text editor and sublime included.
It does not include scopes for calls or that kind of code interaction. What it does is a meta scope for wrapping contexts and its tokens, so we could add a scope to like meta.function in the function declaration but it would require themes to have specific color for a token with the scope meta.function entity.functiona.name.

But I do expect the themes to adapt well to elixir, and in that sense imo we should push themes to grow and have better token support them rather than removing the possibility to have them correctly identified by the text editors.

@josevalim
Copy link
Contributor

josevalim commented Sep 8, 2019 via email

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