ocamllabs / vscode-ocaml-platform

Visual Studio Code extension for OCaml
ISC License
344 stars 74 forks source link

`in` keyword completion #1088

Open 3Rafal opened 1 year ago

3Rafal commented 1 year ago

Scenario:

  1. User types a line of code that ends with in(very common OCaml pattern)
  2. Tries to open a new line with enter key
  3. Gets the autocompletion instead

The problem has already been discussed:

183

https://github.com/ocaml/ocaml-lsp/issues/420 The consensus seems that it takes more work to resolve this problem systemically for all OCaml keywords.

However, I have found a simple hack that resolves this specific case. It was inspired by this Stack Overflow issue.

This workaround for in can be implemented as:

    "in": {
        "body": "in",
        "prefix": "in",
        "description": "this is to prevent irrelevant suggestions for the keyword in"
    }

Which provides in autocompletion and seems much more intuitive.

image

We can either add this snippet to vscode-ocaml-platform list of built-in snippets or explain this workaround in README.

mshinwell commented 1 year ago

It would be nice to see this fixed completely properly, but the above gives a really significant improvement, to what seems a fairly serious usability issue.

abbysmal commented 1 year ago

Agreeing this would be a very nice improvement.

rgrinberg commented 1 year ago

If it's a snippet, should we just suggest it from the lsp server?

Also, what is the difference between this workaround and just adding the keyword as a normal completion candidate?

3Rafal commented 1 year ago

@rgrinberg , as I understand it, the keyword completion was expected to be context aware. This comment suggests that adding keyword completions without it is too noisy. Do you think of adding completion just for in? Or should we add all keywords?

My "solution" is a cheap attempt at helping with a common pain point that was reported by Mark and seems to affect a lot of users. I wanted to add it in the topmost "layer" of the completion stack and then wait for a proper solution. I'm unsure if hiding this kind of hack inside the lsp server is better.

rgrinberg commented 1 year ago

Does this issue affect every editor? If yes, then a solution in ocamllsp seems more appropriate.

Do you think of adding completion just for in? Or should we add all keywords?

No strong opinion here. We could experiment and see which ad-hoc solution works better in practice.

My "solution" is a cheap attempt at helping with a common pain point that was reported by Mark and seems to affect a lot of users. I wanted to add it in the topmost "layer" of the completion stack and then wait for a proper solution. I'm unsure if hiding this kind of hack inside the lsp server is better.

I don't mind a temporary hack, but I don't see the advantage of doing it in vscode if the issue affects multiple editors. Unless of course the hack proposed here is somehow different than doing it right in the server.

lukstafi commented 1 year ago

The completion exception cannot easily be patched in a vscode extension, because it seems to be happening between VSCode and the LSP without the LSP client having a say. So it would be easiest to intervene somewhere in the completion logic. Except it would be kinda ugly.

smorimoto commented 1 year ago

I don't think we can even say that this is a problem that only happens with VSCode. In other words, it should ideally and definitely be addressed on the LSP side.