Add sqls language server#49
Conversation
I think adding support for a language server is substantial enough to warrant some credit
Oops, forgot to do that
Barely had to do much to hook this up so credit to valentinegb. Tested with sqlite
|
We require contributors to sign our Contributor License Agreement, and we don't have @NathanielH-snek on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
|
@cla-bot check' |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
@MrSubidubi Hey totally get you're busy, any chance I can get this reviewed or merged before I fully forget whatever this was doing? |
MrSubidubi
left a comment
MrSubidubi
left a comment
There was a problem hiding this comment.
Apologies for the delay! Few notes, overall looks fine. Can you perhaps add a small recording showing of you interacting with the server?
| let version_path = format!("{ls_path}{}/", latest_release.version); | ||
| let mut command_path = format!("{version_path}sqls"); | ||
|
|
||
| if current_platform().0 == Os::Windows { |
There was a problem hiding this comment.
We don't need this, this noops on Windows anyway
| set_language_server_installation_status, settings::LspSettings, | ||
| }; | ||
|
|
||
| struct Sql { |
There was a problem hiding this comment.
| struct Sql { | |
| struct SqlExtension { |
Is how we usually name these - sorry for the nitpick 😅
There was a problem hiding this comment.
Can we remove some of those that are not needed and make it a bit more barebones?
| schema_version = 1 | ||
| version = "1.1.7" | ||
| authors = ["nervenes", "notpeter", "phileix", "tammyxiong"] | ||
| authors = ["nervenes", "notpeter", "phileix", "tammyxiong", "valentinegb"] |
There was a problem hiding this comment.
| authors = ["nervenes", "notpeter", "phileix", "tammyxiong", "valentinegb"] | |
| authors = ["nervenes", "notpeter", "phileix", "tammyxiong"] |
They mentioned someone can take this over - we appreciate their work and already send them a little goodie for everything they did some time ago, but think let's leave them out here in their interest, please.
|
Will close for now, sorry for my initial slow reply, but happy to revisit this if you decide to pick it up again - we might want to consider disabling the language server by default since it is a bit noisy with that popup and thus might be better to keep that out of peoples faces. In any case, thanks for spending your time on this and happy to reopoen if you find some more for this. Thanks! |
3 participants
Needed a small change but seems to be working now. Had to wrap the passed settings with an outer sqls key. Tested with sqlite. One minor annoyance to note is that there's a notification claiming no db connections on launch.