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

Null response for empty completion requests #17

Closed
LucasBullen opened this issue Sep 20, 2017 · 13 comments
Closed

Null response for empty completion requests #17

LucasBullen opened this issue Sep 20, 2017 · 13 comments

Comments

@LucasBullen
Copy link

Request:

{
  "jsonrpc":"2.0",
  "id":"2",
  "method":"textDocument/completion",
  "params":{
    "textDocument":{
      "uri":"file:///home/lbullen/runtime-EclipseApplication/NewDotnetProject1/NewDotnetProject1.csproj"
    },
    "uri":"file:///home/lbullen/runtime-EclipseApplication/NewDotnetProject1/NewDotnetProject1.csproj",
    "position":{
      "line":0,
      "character":0
    }
  }
}

Response:

{
  "protocolVersion":"2.0",
  "id":"2",
  "result":null
}

Looking at the LSP for completion-requests, the expected response should be CompletionItem[] | CompletionList.

null is not accepted or else the protocol would look like the Goto Definition Request Protocol: Location | Location[] | null

@tintoy
Copy link
Owner

tintoy commented Sep 23, 2017

Ooh, that's interesting - my understanding was that null means "completions not supported here" (VSCode seems to show an empty completion list if I return CompletionItem[], but no list at all if I return null). That signature comparison does seem compelling though.

If I start always returning non-null here it would break my extension (since I have to rely on "silently" triggering completion sometimes; VSCode may ask for completions, but shows nothing since I return null).

FWIW, my LSP client implementation is perfectly happy either way:

https://github.com/tintoy/dotnet-language-client/blob/master/src/LSP.Client/Clients/TextDocumentClient.Completions.cs#L32

How about if I add a feature flag to return empty completion lists instead of null - would that be good enough?

tintoy added a commit that referenced this issue Sep 23, 2017
…an null.

We can't always do this because our extension depends on VSCode's behaviour when null is returned vs an empty completion list (when null is returned, no completion list is displayed; when an empty completion list is returned, purely-textual completions are displayed based on current file contents).

#17
@tintoy
Copy link
Owner

tintoy commented Sep 23, 2017

Ok - after starting the language service, you can now get the behaviour you want by sending it a "workspace/didChangeConfiguration" message
with params along the lines of:

{
    "settings": {
        "msbuildProjectTools": {
            "language": {
                "enable": true,
                "logLevel": "Information",
                "experimentalFeatures": [
                    "empty-completion-lists"
                ]
            }
        }
    }
}

BTW, the full settings schema is:

{
    "settings": {
        "msbuildProjectTools": {
            "language": {
                "enable": true,
                "disableHover": false,
                "logLevel": "Information",
                "seqLogging": {
                    "url": "http://localhost:5341/",
                    "apiKey": "KN3CFcqcWmsirb3ztL9g",
                    "logLevel": "Verbose"
                },
                "completionsFromProject": [
                    "Property",
                    "ItemType",
                    "ItemMetadata",
                    "Target",
                    "Task"
                ],
                "experimentalFeatures": [
                    "expressions",
                    "empty-completion-lists"
                ]
            },
            "nuget": {
                "newestVersionsFirst": true,
                "disablePrefetch": true
            }
        }
    }
}

@tintoy
Copy link
Owner

tintoy commented Sep 23, 2017

BTW, sorry for the slow response - I didn't get the notification until this morning, for some reason!

@LucasBullen
Copy link
Author

Is there a possibility for the experimentalFeatures to be set within the initializationOptions of the initialize call? LSP4E does not support making workspace/didChangeConfiguration calls.

BTW, sorry for the very slow response, was busy with other tasks

@tintoy
Copy link
Owner

tintoy commented Oct 16, 2017

No problem :)
Let me do an experiment or 2 - will need to see the server-side parameter type. Should be doable, or at worst I could make it an environment variable. Will give it a go as soon as I get to work (an hour and a half)

@tintoy
Copy link
Owner

tintoy commented Oct 16, 2017

Ok, yep, looks like I can do it - give me a couple of minutes. Do you need a new package, or can you run from source?

tintoy added a commit that referenced this issue Oct 16, 2017
@tintoy
Copy link
Owner

tintoy commented Oct 16, 2017

Basically, just pass something like the following JSON in InitializationOptions:

{
    "msbuildProjectTools": {
        "logging": {
            "level": "Information"
        },
        "nuget": {
            "newestVersionsFirst": true,
            "disablePrefetch": true
        },
        "experimentalFeatures": [
            "empty-completion-lists"
        ],
        "schemaVersion": 1
    }
}

@LucasBullen
Copy link
Author

Perfect! That did it, no more errors on my end. We will need a new package if that is a possibility then we will be able to add your completion-suggestion work to aCute (Whoot!) and have the rest of the features functional once the initialization capabilities issue is solved.

@tintoy
Copy link
Owner

tintoy commented Oct 17, 2017

Woohoo! Ok, will put put out a new release sometime this morning then :)

@tintoy
Copy link
Owner

tintoy commented Oct 17, 2017

Ok, @LucasBullen, here you go - v0.2.15

@tintoy
Copy link
Owner

tintoy commented Oct 17, 2017

PS. Thanks for using this - it's great to see broader adoption :)

@tintoy
Copy link
Owner

tintoy commented Oct 17, 2017

BTW, not sure if you support snippet-style completions; if not, some types of completions (e.g. new elements) may not work correctly yet. Let me know if this is the case, and I can either offer a setting to turn off unsupported suggestion types (quick) or implement a fall-back to regular-style completions (may take a while, but definitely doable, although the UX is slightly less pleasant without snippet-style support we have no control over cursor positioning after inserting the completion).

@LucasBullen
Copy link
Author

Thanks! Yes lsp4e has snippet-style completion support and the new cut works great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants