Add workspace_folders parameter to LSP::Client#190
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Textbringer LSP client initialization handshake to send workspaceFolders (so servers can index the correct project root) and advertises workspace.workspaceFolders support in client capabilities.
Changes:
- Pass
workspace_foldersfromServerRegistrywhen constructing an LSP client. - Add
workspaceFoldersto the LSPinitializerequest params. - Flip
client_capabilities[:workspace][:workspaceFolders]fromfalsetotrue.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lib/textbringer/lsp/server_registry.rb | Passes [root_path] as workspace_folders when creating a client per project root. |
| lib/textbringer/lsp/client.rb | Adds workspace_folders support to initialize, sends workspaceFolders in initialize params, and advertises workspace folder support in capabilities. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/textbringer/lsp/client.rb
Outdated
| def initialize(command:, args: [], root_path:, server_name: nil, workspace_folders: [Dir.pwd]) | ||
| @command = command | ||
| @args = args | ||
| @root_path = root_path | ||
| @server_name = server_name || command | ||
| @workspace_folders = workspace_folders |
There was a problem hiding this comment.
The default workspace_folders: [Dir.pwd] can diverge from the required root_path argument (e.g., callers pass root_path different from the current working directory). That would cause rootUri and workspaceFolders to disagree and can lead servers to index the wrong root. Consider defaulting workspace_folders to [root_path] (or setting @workspace_folders ||= [@root_path] in the body) and normalizing to an array of paths.
| def initialize(command:, args: [], root_path:, server_name: nil, workspace_folders: [Dir.pwd]) | |
| @command = command | |
| @args = args | |
| @root_path = root_path | |
| @server_name = server_name || command | |
| @workspace_folders = workspace_folders | |
| def initialize(command:, args: [], root_path:, server_name: nil, workspace_folders: nil) | |
| @command = command | |
| @args = args | |
| @root_path = root_path | |
| @server_name = server_name || command | |
| @workspace_folders = Array(workspace_folders || @root_path) |
| workspaceFolders: @workspace_folders.map { |path| | ||
| { uri: "file://#{path}", name: File.basename(path) } | ||
| }, |
There was a problem hiding this comment.
workspaceFolders URIs are constructed via string interpolation ("file://#{path}"), which produces invalid DocumentUri values for paths containing spaces or other characters requiring percent-encoding. Consider generating file URIs via Ruby's URI helpers (and ensuring paths are absolute/expanded) before sending them to the server.
| }, | ||
| workspace: { | ||
| workspaceFolders: false | ||
| workspaceFolders: true |
There was a problem hiding this comment.
client_capabilities now advertises workspace.workspaceFolders: true, but the client does not handle the server request workspace/workspaceFolders (it will currently respond "Method not found"). Servers that use this capability may fail or degrade. Either implement handling for workspace/workspaceFolders (returning the current workspace folders) or keep this capability disabled.
| workspaceFolders: true | |
| workspaceFolders: false |
| params: { | ||
| processId: Process.pid, | ||
| rootUri: "file://#{@root_path}", | ||
| rootPath: @root_path, | ||
| workspaceFolders: @workspace_folders.map { |path| | ||
| { uri: "file://#{path}", name: File.basename(path) } | ||
| }, | ||
| capabilities: client_capabilities, |
There was a problem hiding this comment.
The new workspaceFolders initialization behavior isn’t covered by tests. Since this file already has LSP client tests, it would be good to add a test that initialize params include the expected workspaceFolders array (and that it matches the configured workspace folder paths).
Send workspaceFolders in the LSP initialize request so servers can index the correct project root. The server registry passes [root_path], and the parameter defaults to [Dir.pwd] for direct Client usage. Also advertise workspace.workspaceFolders: true in client capabilities. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
shugo
force-pushed
the
feature/lsp-workspace-folders
branch
from
1bad27f to
30279df
Compare
- Implement workspace/workspaceFolders server request handler so that advertising workspaceFolders: true actually works correctly - Default workspace_folders to root_path instead of Dir.pwd to avoid rootUri/workspaceFolders disagreement - Remove redundant workspace_folders: [root_path] from server_registry since the default now matches root_path - Add tests for workspace_folders defaulting and explicit values Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Send workspaceFolders in the LSP initialize request so servers can index the correct project root. The server registry passes [root_path], and the parameter defaults to [Dir.pwd] for direct Client usage.
Also advertise workspace.workspaceFolders: true in client capabilities.