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

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.

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 a pull request may close this issue.

2 participants