search

OpenLiberty / liberty-tools-intellij

IntelliJ IDEA extension for Liberty
https://plugins.jetbrains.com/plugin/14856-open-liberty-tools
Eclipse Public License 2.0
14 stars 26 forks source link

Code Completion Errors in bootstrap.properties file #946

Closed vaisakhkannan closed 3 weeks ago

vaisakhkannan commented 2 months ago
  1. We have noticed a difference in code completion when typing a dot ('.') at the end of the text. Specifically, there is a difference in the suggestions for 'log' and 'log.' , the suggestions provided for 'log' are valid and correct, but the suggestions for 'log.' are invalid.

https://github.com/user-attachments/assets/a4831018-1e54-4952-90e1-878ec6e04b8e

The same situation arises when we try typing either 'COM.IBM.' or 'COM.' to view the list of suggestions. However, when a suggestion is selected, the initial 'COM.IBM.' or 'COM.' that was typed is not replaced. There is no issue with the lowercase letters text 'com.' and 'com.ibm.'

https://github.com/user-attachments/assets/66831f3f-728f-48b9-98e5-9ccf2d15f852

Environment

LTI Candidate driver 24.0.9-SNAPSHOT LSP4IJ version 0.4.1-20240904-172844 Intellij IDE 2024.1.6 Same behaviour in all os

TrevCraw commented 2 months ago

This bug was resolved previously in the MicroShed LSP4IJ: https://github.com/OpenLiberty/liberty-tools-intellij/issues/729

It seems it is now occurring with the Marketplace LSP4IJ.

angelozerr commented 2 months ago

This bug was resolved previously in the MicroShed LSP4IJ: https://github.com/OpenLiberty/liberty-tools-intellij/issues/729

I had commented your PR at https://github.com/OpenLiberty/liberty-tools-intellij/issues/729#issuecomment-2084538104 but as I had no answer I though it was resolved with our LSP4IJ.

As I said you in my comment your PR disabled resolve completion, which is a lot used in MicroProfile LS to improve completion performance.

I will try to investigate your problem and if I find a proper fix, I wil try to integrate it in 0.5.0 that we will release on Monday, next week, see https://github.com/redhat-developer/lsp4ij/discussions/509

angelozerr commented 2 months ago

Managing bootstrap.properties and server.env is very hard for LSP client, because your completion item doesn't generate textEdit and you let managing replacement to the LSP client.

I suggest you really that your LSP generates textEdit like MicroProfile LS does and you will be sure that you will have the same behavior than other LSP clients like vscode, Eclipse IDE, etc

But why it works on vscode but not in IJ?

Let me try to explain you. In your case as you don't define textEdit, the start offset used to replace content is get from https://github.com/redhat-developer/lsp4ij/blob/58dc589259bc8a34b999e520334841f8915e88ad/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionProposal.java#L551

The idea of this getWordRangeAt is to return a word:

log is a word, for instance.

When you write

log

and you apply completion, the start offset is before log, so it is working.

But if you write

log. the word in this case is empty, because getWorRangeAt collects word from left and right of the completion offset by stopping if the character is not a java part identifier.

It explains why it doesn't replace log. and you have the bug.

How vscode supports that? It uses the tokenized model, in other words the textmate grammar to know the start offset of the replacement.

In IJ context, we could try to do that by using the PsiElement (which is the property key in IJ), but it requires some investigation.

angelozerr commented 2 months ago

I created the issue https://github.com/redhat-developer/lsp4ij/issues/512 but as it is not trivial I will have no time to fix it.

I suggest really that you improve your LS to generate textEdit or wait for a future release of LSP4IJ when I will have time to fix it.

TrevCraw commented 2 months ago

Hi @angelozerr, thank you for your initial investigation and for opening an issue to address this bug in LSP4IJ. This issue is a blocker for our upcoming release (which is planned to adopt the LSP4IJ plugin). Unfortunately, updating the Liberty Config Language Server is not an option for our team.

  1. What is your outlook on being able to work on this bug further?
  2. Are there any pointers for our team to be able to assist with investigating and resolving this bug?
angelozerr commented 2 months ago

Hi @angelozerr, thank you for your initial investigation and for opening an issue to address this bug in LSP4IJ. This issue is a blocker for our upcoming release (which is planned to adopt the LSP4IJ plugin). Unfortunately, updating the Liberty Config Language Server is not an option for our team.

IMHO it is the best solutions. Almost language servers that I have tested manage textedit and you are sure that you will have the same behavior with any LSP client.

  1. What is your outlook on being able to work on this bug further?

I am very busy with our quarkus support, and I am trying to fix major problems in LSP4IJ according user issues.

I think this issue impacts only you and not other adopters which use language servers with textedit.

  1. Are there any pointers for our team to be able to assist with investigating and resolving this bug?

I have some ideas how to fix it but not very clear code to do for the moment and I need really to write more tests to avoid regression when fix will be done.

I repeat me but I think you should support textedit in your ls like almost ls does.

My fear is to fix this issue and discover new bugs and spend again a lot of time to support completion item without textedit although if you support textedit you will not have issues.

angelozerr commented 2 months ago

Please follow https://github.com/redhat-developer/lsp4ij/pull/539 which should fix your issue.

vaisakhkannan commented 1 month ago

I can confirm that the issue mentioned has been resolved in the latest LSP4IJ nightly build, version 0.6.0-20240926-013140.

vaisakhkannan commented 3 weeks ago

Again, I can confirm that the issue mentioned here has also been resolved in the latest LSP4IJ nightly build, version 0.7.1-20241101-013326. I am closing this issue now since its resolved