-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
feat: signature help #396
base: main
Are you sure you want to change the base?
feat: signature help #396
Conversation
404d8fd
to
64c53c7
Compare
64c53c7
to
9a4860a
Compare
Also I had the thought, this needs to work for standard library and Erlang functions as well. Which those aren't* traced by the compiler cuz they're already compiled. For those, I believe will need to read them from the runtime in some way (I'll leave you to figure that out) I think we can ship a first version without that tho Edit: typo "are" -> "aren't" |
c4df7ba
to
c693292
Compare
Played with it and realized that I'm not sure this path forward will work. this searched for the definition of the module/function before the cursor, but that implies the project has compiled and its in the references table. I think want we actually want to do is extract the module and function before the cursor and then fetch the docs with Code.fetch_docs/1 that should return the function docs which will actually have a "signature" field that we can use |
c693292
to
15e9b98
Compare
@mhanberg I switched approach. It works well for something like Enum.map(list, fun) It doesn't work for aliased or imported functions like alias NextLS.Formatter
Formatter.format(code) Because |
Gotcha, that should become available as I build out the "buffer environment". I'll try this out today |
Thank you, if you're happy with that limitation, I can cleanup the code later 👍 |
Yeah the limitation is fine for now, it's basically already a limitation for autocomplete. |
ee80f06
to
bd230ac
Compare
Also, I think we should go ahead and make the active parameter work before merging. I imagine you can take the ast you find and then find the parameter the cursor is inside and use that to determine the index to use. |
f68d626
to
c66fdfb
Compare
@mhanberg I added the active parameter, tests are working but I'm having trouble using it in my editor. I'll investigate more later. |
|
I think i need to make a patch to spitfire for this to work the way its currently coded. |
I was re reading the PR comments and I can't seem to remember what patch I needed to make to spitfire. |
06acca3
to
f4685d0
Compare
c66fdfb
to
6a3b1d8
Compare
6a3b1d8
to
92dfabc
Compare
Now spitfire has the function I think what you can do here is use that function, create a zipper, find the cursor, then traverse up until you see the first function call, and that should be the function that should be used for signature help. you might need to also "expand" the ast at the cursor like we do for completions to get the environment, which should help you resolve imports and aliases for that function. |
This could work but only for partial code like Enum.map( I think we also want to give Enum.map([1, 2], fn n -> n + 1 end) |
You make it into a partial code snippet by truncating the document after the cursor position. Basically if the cursor is the location line: 0, character: 30, you'd truncate it too look like
|
Oh right, It make a lot of sense, dumb question, my bad 😅. Thank you. I hope I have some time in the upcoming weeks to push this PR forward 🤞 |
No worries, its all volunteer work! Work on it when you have the time, energy, and interest. |
I've reduced the scope of this MR.
I've reused theDefinition
code and showed the signature.It doesn't work while you're writing code like
String.replace(
and it doesn't show the active parameter.Screen.Recording.2024-03-18.at.17.15.43.mov