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

Require as function name should not appear as blue #452

Open
nicobao opened this issue Mar 24, 2020 · 4 comments
Open

Require as function name should not appear as blue #452

nicobao opened this issue Mar 24, 2020 · 4 comments

Comments

@nicobao
Copy link

nicobao commented Mar 24, 2020

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
Copy link
Contributor

axelson commented Mar 29, 2020

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

(or "import" "require" "use" "alias")

@nicobao
Copy link
Author

nicobao commented Apr 2, 2020

Hi @axelson!
Thanks for your reply.
I read on the Slack that people suggested having Semantic colouring and you talked about that PR:
microsoft/language-server-protocol#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
Copy link
Contributor

@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
Copy link
Contributor

@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...

J3RN pushed a commit to J3RN/emacs-elixir that referenced this issue Apr 24, 2021
* remove legacy io_request handlers

we don't support OTP < R15B

* rescue MatchError in :int calls

Fixes elixir-editors#455

* make output device better conform to erlang I/O protocol

see https://erlang.org/doc/apps/stdlib/io_protocol.html for details

* return WireProtocol.send error to the caller

no need to IO.warn if write fails

* we are redirection stderr to stdout, use stdout as underlying device

* inspect error

* monitor debugged processes

add test for mix task exit
Fixes elixir-editors#454

* avoid debugger crashes when handling requests for no longer existing thread, frame and variable ids

Fixes elixir-editors#452

* add test

* forbid changes of underlying device opts

* refactor and add tests coverage to invalid requests

* Map.pop! is available since elixir 1.10

* run formatter
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

No branches or pull requests

4 participants