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

Add sql-language-server #326

Merged
merged 6 commits into from Aug 30, 2020
Merged

Add sql-language-server #326

merged 6 commits into from Aug 30, 2020

Conversation

krassowski
Copy link
Member

@krassowski krassowski commented Aug 29, 2020

References

Closes #325 . Provides a default connection configuration using an empty in-memory SQLite3 database so that it works straight away. It's simple on features (completion + diagnostics) and the diagnostics do not really work on Node 10 as code uses features only implemented in Node 11 (see joe-re/sql-language-server#56), but we might be fine bumping the node requirement soon I think.

Code changes

None

User-facing changes

None

Backwards-incompatible changes

None

Chores

  • linted
  • tested
  • documented
  • changelog entry

@krassowski
Copy link
Member Author

Speaking of node, it seems that docs do not like the version bump of jest:

error jest@26.4.2: The engine "node" is incompatible with this module. Expected version ">= 10.14.2". Got "10.13.0"

@krassowski
Copy link
Member Author

And installation notebook is outdated saying that Node >= 8 is fine.

@krassowski
Copy link
Member Author

Jest has a weird dependency update cycle... they say that they dropped Node 8 in 26.0 but then they require 10.X in point releases and then they downgrade it (jestjs/jest#10352) to 10.13, but only in the worker, so here we are.

from .bash_language_server import BashLanguageServer
from .dockerfile_language_server_nodejs import DockerfileLanguageServerNodeJS
from .javascript_typescript_langserver import JavascriptTypescriptLanguageServer
from .pyls import PythonLanguageServer
from .r_languageserver import RLanguageServer
from .sql_langserver import SQLLanguageServer
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe make sql_language_server for consistency....

Copy link
Member Author

Choose a reason for hiding this comment

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

I was also thinking whether we need to have nodejs prefix on DockerfileLanguageServerNodeJS etc. I guess that it might have been used to distinguish potential alternative implementations, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

And more user-facing, we could change the display name of vscode-html-languageserver-bin to (at least) remove -bin suffix.

@bollwyvl
Copy link
Collaborator

Modulo the one comment regarding the spec module name, which is not a blocker, LGTM.

Heroic work on the RTD node stuff 👏 👏 👏 ... can't wait until Lab 3.0 makes it less of a life-ruining end-user experience, though of course we're rather wedded to it... for now. Before the next release, we should have a dedicated burn-down-the-yarn-lock party and figure out what is broken-by-default-unless-in-our-magic-garden.

Potential follow-ons:

  • fancy handling of the third-party magic seems like a separate PR, unless you've already snuck that in someplace.
  • a heavier-duty config test against e.g. postgres, a real .sqlite file, might be good in the future, as I'm a little unclear on where the config stuff needs to go.
  • explore if we can make it demo nicely with xeus-sqlite (which unfortunately can't be installed next to xeus-python for now)

@krassowski
Copy link
Member Author

Interestingly, the tectonic Zlib error issue broke Windows 3.8 here (but the two other Windows builds proceeded normally)... So this is because it installed h4f32bc4_3 (bot other Windows runs used h11def9f_0). I feel i understand the way conda/mambda resolves dependencies less and less... (though this is still an upstream bug...)

@krassowski krassowski closed this Aug 30, 2020
@krassowski krassowski reopened this Aug 30, 2020
@krassowski krassowski merged commit 574abcc into master Aug 30, 2020
@krassowski krassowski deleted the add-sql-server branch August 30, 2020 10:37
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.

Add support for an SQL langauge server
2 participants