Skip to content
This repository has been archived by the owner on Aug 11, 2024. It is now read-only.

fix(capabilities): replace variable #105

Merged
merged 5 commits into from
May 4, 2022
Merged

fix(capabilities): replace variable #105

merged 5 commits into from
May 4, 2022

Conversation

Avimitin
Copy link
Contributor

@Avimitin Avimitin commented May 2, 2022

Reference: neovim/neovim#14090 (comment)

Signed-off-by: Avimitin <avimitin@gmail.com>
@@ -16,7 +16,7 @@ local function check_server_support_signaturehelp()
end
local clients = vim.lsp.buf_get_clients()
for _, client in pairs(clients) do
if client.resolved_capabilities.signature_help == true then
if client.server_capabilities.signatureHelpProvider == true then
Copy link
Contributor

Choose a reason for hiding this comment

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

Here the server_capabilities.signatureHelpProvider is a table, so table will never be true

Suggested change
if client.server_capabilities.signatureHelpProvider == true then
if client.server_capabilities.signatureHelpProvider then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I use ~= nil as condition? For readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like somebody is already doing great job in #106 , maybe I should close this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should I use ~= nil as condition? For readability.

I'm not so sure about this one, I don't know if there's no provider it will be nil, are you sure about this?
if not, we could avoid that with this

if client.server_capabilities.signatureHelpProvider then

Signed-off-by: Avimitin <avimitin@gmail.com>
There is no guarantee that signatureHelpProvider will be nil.

Signed-off-by: Avimitin <avimitin@gmail.com>
@pedro757
Copy link
Contributor

pedro757 commented May 4, 2022

The maintainer of the project decided that we cannot remove support for previous versions, so make sure that this only runs in version 0.8, for the other version it should not be modified.

if vim.fn.has("nvim-0.8") then

@Avimitin
Copy link
Contributor Author

Avimitin commented May 4, 2022

The maintainer of the project decided that we cannot remove support for previous versions, so make sure that this only runs in version 0.8, for the other version it should not be modified.

if vim.fn.has("nvim-0.8") then

But the code will become too verbose.

@pedro757
Copy link
Contributor

pedro757 commented May 4, 2022

I know, but version 0.8 is still beta, you know, many breaking changes, it will probably break again, and most of the people they use previous versions.

#106 (comment)

Signed-off-by: Avimitin <avimitin@gmail.com>
Comment on lines 40 to 42
( is_nightly
and type(client.server_capabilities.codeActionProvider) == "table"
and client.server_capabilities.codeActionProvider.resolveProvider
Copy link
Owner

Choose a reason for hiding this comment

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

for sake of readability, client.server_capabilities should be abbreviated to capabilities

Signed-off-by: Avimitin <avimitin@gmail.com>
@kkharji
Copy link
Owner

kkharji commented May 4, 2022

Thanks @Avimitin, lgtm I'm going to merge it later on tonight in case of last suggestion ot changes

@kkharji kkharji merged commit fafb102 into kkharji:main May 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants