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

Adding async-request-handlers for lsp-json #1245

Merged
merged 2 commits into from
Dec 12, 2019

Conversation

kiennq
Copy link
Member

@kiennq kiennq commented Dec 12, 2019

Since json language server requests emacs to provide file/remote resource content that may take time to retrieve, we need to make the request response being able to not block typing via async callback,

This is per discussion in #1073

@kiennq kiennq force-pushed the bug/lsp-json-slow branch 2 times, most recently from 6e05b97 to a32f31e Compare December 12, 2019 08:06
Copy link
Member

@yyoncho yyoncho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! You may fix lsp-json.el header as well.

lsp-mode.el Outdated Show resolved Hide resolved
@yyoncho
Copy link
Member

yyoncho commented Dec 12, 2019

I had some troubles when testing on clean machine with emacs 27.

  1. The automatic install does not work on linux systems since it requires super user privileges. I suggest removing automatic installation until we make it work for all systems.
  2. The lists must be vectors because emacs 27 json serialization supports only vectors.

@kiennq
Copy link
Member Author

kiennq commented Dec 12, 2019

2. The lists must be vectors because emacs 27 json serialization supports only vectors.

Are you talking about lsp-json--schema-associations? Also I wonder where can you get the emacs-snapshot version that support json serialization?
I'm on Ubuntu (via WSL) and install emacs with this PPA https://launchpad.net/~ubuntu-elisp/+archive/ubuntu/ppa

  1. The automatic install does not work on linux systems since it requires super user privileges. I suggest removing automatic installation until we make it work for all systems.

Oh, it works great on Windows though.
In my Ubuntu, once I set npm config set prefix '~/.npm-global', then install npm i -g will not require sudo anymore.
I'm not sure if there's anyone really install some packages for everyone on their machines.
For now I will scoop down the automatic installation to Windows, is that fine?

@yyoncho
Copy link
Member

yyoncho commented Dec 12, 2019

  1. The lists must be vectors because emacs 27 json serialization supports only vectors.

Are you talking about lsp-json--schema-associations? Also I wonder where can you get the emacs-snapshot version that support json serialization?
I'm on Ubuntu (via WSL) and install emacs with this PPA https://launchpad.net/~ubuntu-elisp/+archive/ubuntu/ppa

I don't know - I am compiling it from source - Another option is to use lsp-docker, which has emacs 27.

lsp-json--schema-associations
lsp-json--extra-init-params

I fixed these locally but I am unable to get completion working.

In my Ubuntu, once I set npm config set prefix '~/.npm-global', then install npm i -g will not require sudo anymore.
I'm not sure if there's anyone really install some packages for everyone on their machines.
For now I will scoop down the automatic installation to Windows, is that fine?

In general, I prefer having #506 in before working on any automatic installations because it makes the task harder(e. g. whoever starts working on it will have to fix all automatic installations that are implemented). If you insist keep it for windows.

@kiennq
Copy link
Member Author

kiennq commented Dec 12, 2019

I fixed these locally but I am unable to get completion working.

Can you post the locally fix here? I don't have emacs27 with native json now, so it would be better to reuse your fix.

I fixed these locally but I am unable to get completion working.

The latest vscode-json-languageserver has problem, you may need to install the version before it.
npm i -g [email protected]

@yyoncho
Copy link
Member

yyoncho commented Dec 12, 2019

I fixed these locally but I am unable to get completion working.

Can you post the locally fix here? I don't have emacs27 with native json now, so it would be better to reuse your fix.

yyoncho@162c1d3
It works fine after this commit.

I fixed these locally but I am unable to get completion working.

The latest vscode-json-languageserver has problem, you may need to install the version before it.
npm i -g [email protected]

Can you document that?

@kiennq
Copy link
Member Author

kiennq commented Dec 12, 2019

Fixed!!!

@yyoncho yyoncho merged commit de67beb into emacs-lsp:master Dec 12, 2019
@yyoncho
Copy link
Member

yyoncho commented Dec 12, 2019

Thank you!

As a side note, when you mark a server with :multi-root t lsp-mode will run only one instance of that server but I am not sure whether it could cause issues if the server does not support workspace folders.

@kiennq kiennq deleted the bug/lsp-json-slow branch December 12, 2019 17:05
@kiennq
Copy link
Member Author

kiennq commented Dec 12, 2019

Thank you!

As a side note, when you mark a server with :multi-root t lsp-mode will run only one instance of that server but I am not sure whether it could cause issues if the server does not support workspace folders.

Oh, I didn't know about this. Certainly lsp-json could be benefit from this

patgro1 pushed a commit to patgro1/lsp-mode that referenced this pull request Dec 15, 2019
* lsp-json: adding async-request-handlers and public schemas url

* Document vscode-json-languageserver broken
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.

2 participants