-
Notifications
You must be signed in to change notification settings - Fork 19
feat: handle commands on the client side #50
Conversation
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.
Thanks for implementing this feature! 🙏🏾
It is not yet working for me. But I think that can be reduced to one simple issue (see comments).
The second point is that you added it for workspace edits which are detected to be actually a command (legacy language servers), but for the actual Command
class you did not add it. Could you please do so? Don't worry about duplicated code here. Will be resolved in future anyway. 🙂
function BaseAction:new(data) | ||
vim.validate({ ['data'] = { data, 'table' } }) | ||
local instance = { server_data = data[1], client_id = data[2] } |
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.
Why did you rename this here. Is it somehow related to the feature? 🤔
@@ -80,7 +80,24 @@ function CodeAction:execute() | |||
if self:is_workspace_edit() then | |||
vim.lsp.util.apply_workspace_edit(self.server_data.edit, 'utf-8') | |||
elseif self:is_command() then | |||
vim.lsp.buf.execute_command(self.server_data.command) | |||
local client = vim.lsp.get_client_by_id(self.client_id) | |||
local fn = client.commands[self.server_data.command.command] |
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.
In my tests, this is not correct and also not how the native implementation does it. According to the specification this is just self.server_data.command
. Do you have a language server which formats that differently?
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.
Can you share your tests?
The navite implementation:
local fn = client.commands[command.command] or vim.lsp.commands[command.command]
local params = vim.lsp.util.make_range_params() | ||
params.context = context | ||
|
||
fn(self.server_data.command, { |
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.
Related to the above comment: we just need to pass self.server_data
here. The self.server_data.command
is just the string
.
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.
Nope, self.sever_data.command
is a table.
Add the following code at line 91
print(vim.inspect('here --> ' .. type(self.server_data.command)))
See the output #49 (comment)
Main Change
BaseAction:new
now takesclient_id
in the second index of the tableFeel free to make suggestions.
Close #49