-
Notifications
You must be signed in to change notification settings - Fork 1
feat(language server): sample implementation #216
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
base: feat/linter
Are you sure you want to change the base?
Conversation
4be032b to
7ab8616
Compare
7ab8616 to
b85501c
Compare
Update Rust version to use Rust 2024 edition
4013621 to
f9f7e39
Compare
| format_sql(&input, Some(&settings_json), config_path) | ||
| .map_err(|e| Error::new(Status::GenericFailure, format!("{e}"))) | ||
| format_sql(&input, Some(&settings_json), config_path).map_err(generic_error) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
runfmt()、runfmt_with_settings()は不要になった認識で合っていますか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
別言語のファイルに埋め込まれたSQLをフォーマットする用途のため、残しておいたほうがよい と考えています。
つまり、SQL以外のファイルで部分フォーマットする場合は Language Server にリクエストを投げずに拡張機能側で完結する構成にします。ここで runfmt()、runfmt_with_settings() を利用します。
Language Server を利用する構成も実現は可能ですが、その場合は拡張機能から textDocument/formatting または textDocument/rangeFormatting を呼ぶことになります。これらの機能は当該ドキュメントの内容をサーバ側が知っていることを前提としているため、高速な動作のためにはサーバがワークスペースに属するすべてのファイルの変更イベントを反映しておく必要があり、巨大なコードベースでは無駄が大きいと考えます。
そのため、以下のような構成を検討しています:
- SQL ファイルのフォーマットには LSP を経由する
textDocument/formattingまたはtextDocument/rangeFormattingを動作させるために、サーバは SQL ファイルの変更イベントを受け取りサーバ内のドキュメントに反映する
- SQL 以外のファイルでのフォーマットでは LSP を経由しない
- 専用のコマンドを用意しておく
- 拡張機能側で選択部分を取得、
runfmt()等に投げてフォーマットする - config の解決も拡張機能側で実施する(現状の拡張機能がもつ config 解決と同等の処理)
| }; | ||
| let text = rope.to_string(); | ||
|
|
||
| match format_sql(&text, None, None) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
現状はconfig無しでフォーマットしているようですが、今後クライアントも含めて対応するという認識で合っていますでしょうか?
実装するとしたら以下の流れですかね。(詳しく調べられていないので間違えていたらすみません)
- LSPサーバで
.uroborosqlfmtrc.jsonのパスを特定 (無ければNone) - LSPサーバからVSCode拡張クライアントへVSCodeユーザ設定を問合せ
- 1と2を与えて
format_sql()実行
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
現状はconfig無しでフォーマットしているようですが、今後クライアントも含めて対応するという認識で合っていますでしょうか?
共有が漏れておりすみません。 現状は config のことを考慮しない実装をしており、(クライアントを含め)今後対応していく予定です。
実装について、次に示すのが基本的な方針です:
- 設定値はサーバ側で保持し、それを実行時にも利用する
- 設定値が変更されたらそれを反映して同期する
ここで考慮すべき設定は 次の2種類あります:
- VS Code 拡張の設定(
settings.jsonで表現される) - ツール固有の設定(
.uroborosqlfmtrc.jsonで表現される)
これらを扱う基本的な方針は同じですが、設定値の取得方法と設定変更を知る方法が異なります。
- 設定値の取得方法
- 1 (VS Code 拡張の設定):workspace/configuration にて取得する
- 2(ツール固有の設定): 設定ファイルを特定して内容を読む
- 設定変更を知る方法
- 1 (VS Code 拡張の設定)
- 何もしなくても、設定変更時には workspace/didiChangeConfiguration にて変更がサーバへ通知される
- 2 (ツール固有の設定)
- サーバ起動時に
.uroborosqlfmtrc.jsonなどを workspace/didChangeWatchedFiles として登録する - するとファイル変更時に通知を受け取ることができる
- サーバ起動時に
- 1 (VS Code 拡張の設定)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
なるほど、.uroborosqlrc.json の変更は workspace/didChangeWatchedFilesで監視できるんですね。
ありがとうございます。
Summary
LSP の言語サーバを実装しました。
Node-API 版にサーバ実装を追加していますが、wasm 版は未対応です。
主なライブラリとして以下を追加しています:
Note
VS Code 側でこのサーバを利用する場合: future-architect/vscode-uroborosql-fmt#99