Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

CompletionItem of completions response always have start = 0 and text prefix inclusion is mixed #524

Open
mfussenegger opened this issue Dec 7, 2023 · 1 comment · May be fixed by #526

Comments

@mfussenegger
Copy link
Contributor

mfussenegger commented Dec 7, 2023

I noticed recently that in nvim-dap, if you complete com. and select an entry you get com.com... inserted, so today I took a closer look and noticed that the responses from java-debug are somewhat odd - and I think incorrect.

With a client that specified columnsStartAt1 = true, and a completions payload like:

{
    frameId = <frameId>,
    text = "List.",
    column = 6
}

The responses include:

  }, {
    label = "of(E e1, E e2, E e3, E e4) : List<E>",
    number = 0,
    sortText = "999999179",
    start = 0,
    text = "of()",
    type = "function"
  }, {

The specification says:

/**

  • Start position (within the text attribute of the completions request)
  • where the completion text is added. The position is measured in UTF-16 code
  • units and the client capability columnsStartAt1 determines whether it is
  • 0- or 1-based. If the start position is omitted the text is added at the
  • location specified by the column attribute of the completions request.
    */
    start?: number;

The expected result for the user is to have List.of() if the completion candidate is selected. Now, start=0 is already odd given the columnsStartAt1, so a possible interpretation in the client is that it's absent, and that the client should just append .of()

This is kinda what I did in nvim-dap so far, and it works for the List.of case, and also for variables, but with a payload like:

{
  column = 5,
  frameId = <frameId>,
  text = "com."
}

I get responses like:

  }, {
    label = "com.sun.tools.example",
    number = 0,
    sortText = "999999183",
    start = 0,
    text = "com.sun.tools.example",
    type = "module"
  }, {

Opposed to the List. result, here text includes the prefix com. and it's again start=0. This led to com.com.sun.tools.example

I suspect vscode does some kind of prefix matching on the client side again, so this isn't noticable there?
As far as I can tell, based on the specification the current behavior is wrong.

I used JDK 21 in my tests - in case it matters.
I can also provide some sample project if needed - but I tried to use examples that should behave similar with only the JDK as dependency

mfussenegger added a commit to mfussenegger/java-debug that referenced this issue Dec 8, 2023
Fixes microsoft#524
Ensures completion text can be appended by clients

E.g.

For `com.|` the completion proposal has:

    completion: com.sun.jndi.ldap.pool
    completionLocation: 3
    replaceEnd: 4
    replaceStart: 0

`insertText` will be `sun.jndi.ldap.pool`

For `List.|` the completion proposal has:

    completion: of()
    completionLocation: 4
    replaceStart: 5
    replaceEnd: 5

`insertText` will be `of()`
mfussenegger added a commit to mfussenegger/java-debug that referenced this issue Dec 8, 2023
Fixes microsoft#524
Ensures completion text can be appended by clients

E.g.

For `com.|` the completion proposal has:

    completion: com.sun.jndi.ldap.pool
    completionLocation: 3
    replaceEnd: 4
    replaceStart: 0

`insertText` will be `sun.jndi.ldap.pool`

For `List.|` the completion proposal has:

    completion: of()
    completionLocation: 4
    replaceStart: 5
    replaceEnd: 5

`insertText` will be `of()`
@testforstephen
Copy link
Contributor

public static class CompletionItem {
public String label;
public String text;
public String type;
/**
* A string that should be used when comparing this item with other items.
*/
public String sortText;
public int start;
public int number;

start = 0 should be a bug. We don't assign a value to start attribute of CompletionItem intentionally, and it's the Java compiler that gives it a default value 0 during json serialization. Our purpose is to always insert, I think we should remove start and number fields from our protocol data structure.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants