elixir-editors / emacs-elixir

Emacs major mode for Elixir
446 stars 94 forks source link

Require as function name should not appear as blue #452

Open nicobao opened 4 years ago

nicobao commented 4 years ago

Special reserved words can actually be used as function name as well. For example with require. require_two_usages The first usage, require Foo shows that "require" is highlighted in blue. That's good. The second usage, require(fields, :title, &validate_title/1), is also displayed in blue. It should not. It is a normal function and should be displayed in white.

The same behaviour can be seen with function named import.

I'm happy to start working on a patch, but I'd need initial guidance.

axelson commented 4 years ago

A patch for require would be accepted, but unfortunately I can't give much guidance on how you'd accomplish this (or how much work it would be). It looks like require is identified as a keyword at https://github.com/elixir-editors/emacs-elixir/blob/231291ecadc479295d83fee619049030940bfbe5/elixir-mode.el#L123

nicobao commented 4 years ago

Hi @axelson! Thanks for your reply. I read on the Slack that people suggested having Semantic colouring and you talked about that PR: https://github.com/microsoft/language-server-protocol/issues/18 Indeed, I was too ambitious in suggesting I could help, I wish I could but I have no time for it right now :/. I don't think it is a very important problem anyway. I can live with it. Honestly, I am more bothered by the issue related to smartparens. If it is the same for all users, the smartparens issue should probably have higher priority.

jsmestad commented 4 years ago

@nicobao this should be a pretty easy fix because require cannot be followed by parens when it's a function versus a Kernel method (when it should show up as blue). Right?

If you can confirm that is right, I can put a patch together.

victorolinasc commented 3 years ago

@jsmestad unfortunately it is not so simple...

iex(1)> defmodule Test do
...(1)>   require(Logger)
...(1)> end
{:module, Test,
 <<70, 79, 82, 49, 0, 0, 3, 80, 66, 69, 65, 77, 65, 116, 85, 56, 0, 0, 0, 118,
   0, 0, 0, 12, 11, 69, 108, 105, 120, 105, 114, 46, 84, 101, 115, 116, 8, 95,
   95, 105, 110, 102, 111, 95, 95, 7, 99, ...>>, Logger}
iex(2)> 

Kernel.require is certainly a macro as any other in terms of syntax. What makes it even harder is that it can be scoped inside a function too:

iex(1)> defmodule Test do
...(1)>   def some_func do
...(1)>     require(Logger)
...(1)>   end
...(1)> end
{:module, Test,
 <<70, 79, 82, 49, 0, 0, 4, 36, 66, 69, 65, 77, 65, 116, 85, 56, 0, 0, 0, 142,
   0, 0, 0, 14, 11, 69, 108, 105, 120, 105, 114, 46, 84, 101, 115, 116, 8, 95,
   95, 105, 110, 102, 111, 95, 95, 7, 99, ...>>, {:some_func, 0}}
iex(2)>

Perhaps we should have a face for function calls with parentheses and another for function calls without parentheses. For example:

defmodule MyRouter do
  plug :my_plug # no parentheses call should
  plug(MyPlug, options: "an option")
end

Then at least this could be configurable. This is how the GitHub syntax highlight works. Not sure how it works in other editors though...