Skip to content
This repository was archived by the owner on Oct 16, 2020. It is now read-only.
This repository was archived by the owner on Oct 16, 2020. It is now read-only.

partialResults are sent when clientCapabilities do not support them #329

Closed

Description

@damieng

Hooking up this language server to Atom right now and finding a ton of out-of-spec partialResult messages being returned. Wondering if this is a cause of some slow perf we're seeing but am unable to switch them off to see.

It looks like the intent was that the clientCapabilities has a streaming property which defines when these messages are supposed to be sent but it isn't yet honored.

Activity

  1. tomv564 commented on Aug 8, 2017

    @tomv564
    Contributor

    I brought this up in #298 because I thought it would be a significant effort.

    Looking at the TODO mentioned in that issue (https://github.com/sourcegraph/javascript-typescript-langserver/blob/master/src/connection.ts#L257-L282) it does not look that hard to fix!

  2. felixfbecker commented on Aug 11, 2017

    @felixfbecker
    Contributor

    I don't think that sending these notifications are a problem. VS Code sends a lot of non-specced notifications like $/trace and the server also resends textDocument/publishDiagnostics and those are not a problem. Message handling in the client shouldn't be the bottleneck or there is more likely something wrong with the client. That said, I don't have a problem with checking the flag there and would accept a PR, that's what it's for after all.

  3. tomv564 commented on Aug 20, 2017

    @tomv564
    Contributor

    I think this specific issue can be closed, the performance discussion continues in #331

  4. damieng commented on Aug 20, 2017

    @damieng
    Author

    I still think this is an issue. Emitting unwanted out-of-spec messages is quite annoying from the client side especially if you have debugging on, it makes it a real pain to see what's going on. It's also a pretty easy fix - I have one incoming.

  5. felixfbecker commented on Aug 20, 2017

    @felixfbecker
    Contributor

    It is already fixed in 2b68b48

  6. damieng commented on Aug 21, 2017

    @damieng
    Author

    Great, thanks. I had that piece but didn't manage to get tests working.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions