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

Fix and improved TypeScript types #18

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marc2332
Copy link
Contributor

@marc2332 marc2332 commented Jul 2, 2022

Hey, I am playing with some changes to the API that would fix #14 and make it slightly better.

  • Renamed languageServerWithTransport to languageServerWithClient, and instead of accepting a transport, directly accept a client.
  • Moved the client property from languageServer options, so if the dev wants to pass a client, he can use languageServerWithClient.

Examples:

WebSockets transport

import { languageServer } from "codemirror-languageserver";

const lsPlugin = languageServer({
  serverUri, // WebSocket server uri.
  rootUri: "file:///",
  documentUri: `file:///${filename}`,
  languageId: "cpp", // As defined at https://microsoft.github.io/language-server-protocol/specification#textDocumentItem.
});

const view = new EditorView({
  state: EditorState.create({
    extensions: [
      // ...
      lsPlugin,
      // ...
    ],
  }),
});

Re using the same client

import { languageServer } from "codemirror-languageserver";

const client = new LanguageServerClient({
  transport: new WebSocketTransport(serverUri),
  rootUri: "file:///",
});

const firstView = new EditorView({
  state: EditorState.create({
    extensions: [
      // ...
      languageServerWithClient({
        client,
        documentUri: `file:///${secondFileName}`,
        languageId: "cpp",
      }),
      // ...
    ],
  }),
});

const secondView = new EditorView({
  state: EditorState.create({
    extensions: [
      // ...
      languageServerWithClient({
        client,
        documentUri: `file:///${firstFileName}`,
        languageId: "cpp",
      }),
      // ...
    ],
  }),
});

Custom transport

import { languageServer } from "codemirror-languageserver";

const client = new LanguageServerClient({
  transport: new AwesomeCustomTransport(),
  rootUri: "file:///",
});

const lsPlugin = languageServerWithClient({
  client,
  documentUri: `file:///${filename}`,
  languageId: "cpp",
});

const view = new EditorView({
  state: EditorState.create({
    extensions: [
      // ...
      lsPlugin,
      // ...
    ],
  }),
});

Thoughts?

@hjr265
Copy link
Member

hjr265 commented Aug 6, 2022

@marc2332 Can we do this without removing languageServerWithTransport. The improvements suggested here are great. I just also want to make sure that the update won't require the users of this library to have to then "fix" their code.

We can mark languageServerWithTransport as deprecated, and then remove it eventually after a while.

How does that sound to you?

P.S: Sorry for taking so much time to get back to you.

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 this pull request may close these issues.

Wrong types
2 participants