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

Fix #183 idea #958

Closed

Conversation

OscarLahaie
Copy link
Contributor

Why don't we add in and do to snippets to facilitate their insertion ? With this, words like do_at_exit and in_channel_length would not be given priority. I think vscode's behaviour is better with these snippets.

@rgrinberg
Copy link
Contributor

What happens when I write:

let inner = 123 in
10 + in|

Where the cursor is at |. Does vscode suggest to insert a snippet here?

@OscarLahaie
Copy link
Contributor Author

Where the cursor is at |. Does vscode suggest to insert a snippet here?

Yes, it does.
image
Is that an issue?

@rgrinberg
Copy link
Contributor

Yes, it's essentially the same problem under different circumstances.

@OscarLahaie
Copy link
Contributor Author

Yes, it's essentially the same problem under different circumstances.

Yes, but I think this is preferable as I think this case is not annoying. Where the non-suggestion of the in is a real lack of comfort.

@rgrinberg
Copy link
Contributor

If you can convince the other maintainers it's a good idea then I won't block this.

@ulugbekna
Copy link
Collaborator

I'm in favor

@mnxn
Copy link
Collaborator

mnxn commented Jun 11, 2022

I don't think this is the correct solution.

The lack of keyword suggestions affects all editors that use ocaml-lsp, so it shouldn't only be fixed in this editor.
This also only handles two keywords, but there are other keywords that are not suggested. Users can have the same issues with other keywords if they open any additional modules that expose similar identifiers.

I don't mind if keywords are suggested when an identifier is expected, but I would prefer a solution in ocaml-lsp or merlin so we don't need to maintain a list of keywords just for VS Code.

@OscarLahaie
Copy link
Contributor Author

OscarLahaie commented Jun 12, 2022

I don't think this is the correct solution.

The lack of keyword suggestions affects all editors that use ocaml-lsp, so it shouldn't only be fixed in this editor. This also only handles two keywords, but there are other keywords that are not suggested. Users can have the same issues with other keywords if they open any additional modules that expose similar identifiers.

I don't mind if keywords are suggested when an identifier is expected, but I would prefer a solution in ocaml-lsp or merlin so we don't need to maintain a list of keywords just for VS Code.

I totally agree with you but I don't think it will be supported on ocaml-lsp soon. Also maintaining a word list won't be that complicated as the implementation is quite minimalist. Finally for the case of imported keywords it is clear that it should be supported by ocaml-lsp but in the meantime these few keywords are quite handy to be supported in this way.

@3Rafal
Copy link
Contributor

3Rafal commented Jun 12, 2023

This solution was already discussed in #1088 . IMO, it would be the best to solve keyword completion in merlin/ocaml-lsp.
The snippet workaround could be added to readme though, so that users can decide for themselves.

@bzy-debug
Copy link
Contributor

ocaml-lsp has already supported the completion of in keyword, see ocaml/ocaml-lsp#1217

@rgrinberg rgrinberg closed this Dec 14, 2023
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

Successfully merging this pull request may close these issues.

6 participants