LST: Add argument names to function completions

[Zhomart]

1. Adds brackets to fns
2. Adds argument names
3. Does not include 1st argument if used in the pipeline
4. Does not include the last argument if used in the "use" expression

Changes:

1. Add field "arg_names" to ValueConstructorVariant.ModuleFn
2. Separate "value_completion" into 2: "value_completion" and "value_completion_import"
3. Use "insert_text" instead of "text_edit", because it allows inserting snippets - https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#insertTextFormat
   • Note: we had an issue with "." previously, but it can be solved by "not" including module name when autocompleting (tested on vscode and nvim)
4. Updated the tests

before (video)
https://github.com/gleam-lang/gleam/assets/830536/26e9b25a-3c9b-41b6-85ac-c02a4409ec7d

after (video)
https://github.com/gleam-lang/gleam/assets/830536/89cbbceb-93f8-4912-a56b-785b2bd08dbb

gifs

[Zhomart]

As discussed in #3295

cc https://github.com/Olian04 https://github.com/movva-gpu

[Zhomart]

Actually, there's a bug : it doesn't work for std lib functions. I'm investigating...

[movva-gpu]

I agree, it's better like this already

[lpil]

What a cool PR!!

Judging the by the tests this PR reintroduces the duplication bug we just fixed by removing the text edits. We must continue to use that API, or a different solution must be found. The maintainers of the protocol told us we should use the text edit API so I believe that to be the best approach.

If it does not introduce the issue we need tests to demonstrate how that is the case. I think there's tests missing for the new functionality too?

[Olian04]

Does this PR also cover my final gripe with the previous PR?

We should also only add arguments IF the place where we are using the function isn't a function argument it self, and that function argument accepts a function with the same signature as the one we are completing for.

fn some_func(key, val, cb) -> String { todo } 

fn consumer(cb: fn(a, b, c) -> String) { todo }
consumer(some_func) // <- should not include any args nor parentheses

[lpil]

Yes I'm unsure myself how we can identify when to safely add parens to function calls. I wouldn't want the programmer to have to remove them.

[Olian04]

Yes I'm unsure myself how we can identify when to safely add parens to function calls. I wouldn't want the programmer to have to remove them.

Maybe we can have this feature be a code action instead? That way users can trigger it when it makes sense and not have to remove anything for most cases?

[Zhomart]

Yes, that make sense, let me see how other languages do that.

[Zhomart]

Question: I believe you use rust often. I noticed its LSP adds brackets and arguments. How "annoying" is this?

Do you know if another function as callback used a lot or not? If not often, I think this PR speeds up development. Can you please run this locally and check how useful is this?

[Zhomart]

It looks like Golang somehow knows how the completion is used and doesn't add brackets:

[Zhomart]

Update:

• Do not add parentheses (and argument names) when auto-completing in call arguments.

Full demo (video):
https://github.com/gleam-lang/gleam/assets/830536/85b663fc-5f53-4937-82f1-d108c20c8a7f

[Zhomart]

What a cool PR!!

Judging the by the tests this PR reintroduces the duplication bug we just fixed by removing the text edits. We must continue to use that API, or a different solution must be found. The maintainers of the protocol told us we should use the text edit API so I believe that to be the best approach.

If it does not introduce the issue we need tests to demonstrate how that is the case. I think there's tests missing for the new functionality too?

@lpil I believe it can be resolved without "text_edit" (I used is_module_select variable, I've tested this locally with various cases and couldn't reproduce that error.

[lpil]

Question: I believe you use rust often. I noticed its LSP adds brackets and arguments. How "annoying" is this?

Rust doesn't practically have higher order functions in this way, so it is not a problem.

How about instead for this feature is instead of adding parens and placeholder variables names automatically we instead complete the function name without parens, and then when they add parens we show in a tooltip the documentation, types, etc of each argument as they are added. This keeps the programmer in full control, does not add invalid variables to the code, doesn't use the internal names in external code, and the behaviour is uncontroversial.

Concurrently we can in the github issue we can discuss what heuristics would be appropriate to decide when (if ever) to add parens. I'm currently leaning towards never adding them as always adding them would mean doing the wrong thing half the time, and conditionally adding them means the programmer cannot operate purely by muscle memory and instead must pause to see if parens have been added or not before knowing how to continue typing.

[Zhomart]

Question: I believe you use rust often. I noticed its LSP adds brackets and arguments. How "annoying" is this?

Rust doesn't practically have higher order functions in this way, so it is not a problem.

How about instead for this feature is instead of adding parens and placeholder variables names automatically we instead complete the function name without parens, and then when they add parens we show in a tooltip the documentation, types, etc of each argument as they are added. This keeps the programmer in full control, does not add invalid variables to the code, doesn't use the internal names in external code, and the behaviour is uncontroversial.

Concurrently we can in the github issue we can discuss what heuristics would be appropriate to decide when (if ever) to add parens. I'm currently leaning towards never adding them as always adding them would mean doing the wrong thing half the time, and conditionally adding them means the programmer cannot operate purely by muscle memory and instead must pause to see if parens have been added or not before knowing how to continue typing.

Let me close this and let me experiment further with your proposal