erlang-ls / erlang_ls

The Erlang Language Server
https://erlang-ls.github.io/
Apache License 2.0
620 stars 136 forks source link

More precise 'pos' to 'MFA' type #1461

Closed shuying2244 closed 3 months ago

shuying2244 commented 8 months ago

Like player_pgup:get_pgup() click player_pgup will jump to the module; click get_pgup will jump to the function.

If it is before, clicking 'player_pgup' will jump to the function.

gomoripeti commented 8 months ago

thank you for your contribution!

my personal opinion is that to achieve the behaviour that you would like, not the pois and the parser but the "jump to definition" feature needs to be adjusted. (The module poi represents the -module attribute and could lead to confusion if there are a lot of them with different values from the same module. And the application poi has all the range info in the Data section.) However my knowledge of erlang_ls is quite dusty and outdated.

I can imagine that the new behaviour is what some would expect: if one sees a module name, just go to the module. However I'm not sure jumping to the beginning of a module is more useful than to a specific function of it. Do you think it is valuable? (For me a mod:fun expression is together a remote call, and find it convenient that I don't have to move the cursor on the function part but can lazily just reach the beginning of the module name to jump to the function definition)

shuying2244 commented 8 months ago

Hi,

I understand, and I agree that jumping to a function is more useful in most cases;

This change is mainly due to the following use cases: When writing some business logic, we often have to expand some functions, the common is to add function parameters, such as player_pgup:get_pgup() to player_pgup:get_pgup(Info), then for this remote call function, the jump function is ineffective, To enter the module 'player_pgup', you need to copy it and jump to the file. But now, no matter how you change function, you click on player_pgup to always jump to the module; Also, some times when I click on player_pgup is want to see the comments at the top of the module.

In conclusion, I think the old design is useful, but it does not cover a wide range of use cases, and it is also a little inconsistent with the common perception that what you see is what you get.

gomoripeti commented 5 months ago

Hi,

(If I understand your example correctly) what if the improved behaviour would be that

would this be an improvement?

shuying2244 commented 5 months ago

Hi, Your idea is a good one, But I don't approve of the design, The response to an action should be certain so as not to confuse the operator, When 'player_pgup:get_pgup/0' exists, there is also a need to go to the top of the 'player_pgup' file, sometimes just to read the module comments to quickly understand what the module is capable of.

gomoripeti commented 5 months ago

Hi,

I have strong opinion about the implementation that it should be done in the action not the poi parsing, but only a subjective view on the behaviour itself. Just wanted to chime in as I have contributed to the parser before. But I stop now and let the maintainer(s) decide. Thanks for the discussion so far.