Skip to content

Conversation

@abdurrahman-nexthop
Copy link

No description provided.

Copy link

@augment-app-staging augment-app-staging bot left a comment

Choose a reason for hiding this comment

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

Thanks for the update to address deprecations. I left a couple of high-confidence notes where the changes could introduce runtime issues.

end

client.notify(method, params)
client:notify(method, params)

Choose a reason for hiding this comment

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

Switching to client:notify(...) will pass the client table as the first argument, but Neovim’s LSP client client.notify is typically called with dot notation (client.notify(method, params)) and does not expect self; this can shift arguments and break the call. Consider keeping the dot call here.

🤖 React with 👍 or 👎 to let us know if the comment was useful.

end

local _, id = client.request(method, params, function(err, result)
local _, id = client:request(method, params, function(err, result)

Choose a reason for hiding this comment

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

Similarly, client:request(...) will pass self and shift parameters; Neovim LSP usage expects client.request(method, params, handler, bufnr) with dot notation. This change is likely to cause request failures at runtime.

🤖 React with 👍 or 👎 to let us know if the comment was useful.

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.

1 participant