Skip to content
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

fix: moved hover and signatureHelp to signature.lua from lsp.lua #188

Merged
merged 2 commits into from
Sep 26, 2023
Merged

fix: moved hover and signatureHelp to signature.lua from lsp.lua #188

merged 2 commits into from
Sep 26, 2023

Conversation

yoplat
Copy link
Contributor

@yoplat yoplat commented Sep 26, 2023

Moved hover and signature help setup from lsp.lua to signature.lua to fix problem with noice

@siduck siduck merged commit 190a9a0 into NvChad:v3.0 Sep 26, 2023
@yoplat
Copy link
Contributor Author

yoplat commented Sep 26, 2023

@siduck This didn't fix anything, by putting it outside the setup function there is no check for the chadrc signature option, so it will always get executed no matter what. That's why I had put it inside there.

siduck added a commit to NvChad/NvChad that referenced this pull request Sep 26, 2023
@siduck
Copy link
Member

siduck commented Sep 26, 2023

@carlobonandriniunimi check now

@yoplat
Copy link
Contributor Author

yoplat commented Sep 26, 2023

@carlobonandriniunimi check now

Doesn't work neither. I tried using a fresh install of nvchad (with the example_config provided in the install) and it STILL complains. Only solution is what I had done. I doesn't seem to be an issue with my lspconfig, maybe NvChad, maybe Noice, maybe my Noice config but I don't think so.

@lucario387
Copy link
Collaborator

Because this essentially still replaces noice's handlers with nvchad's

@yoplat
Copy link
Contributor Author

yoplat commented Sep 26, 2023

Because this essentially still replaces noice's handlers with nvchad's

That's what I was saying! It should be an option like with signature in the chadrc, it shouldn't even be that bad to do, just an if that surrounds those two commands

@siduck
Copy link
Member

siduck commented Sep 26, 2023

Because this essentially still replaces noice's handlers with nvchad's

That's what I was saying! It should be an option like with signature in the chadrc, it shouldn't even be that bad to do, just an if that surrounds those two commands

there is an option in chadrc, disable that

@lucario387
Copy link
Collaborator

image
There isn't it seems

@siduck
Copy link
Member

siduck commented Sep 26, 2023

@lucario387
Copy link
Collaborator

so why is it locked behind v3.0 and not at least backported to v2.0?

@yoplat
Copy link
Contributor Author

yoplat commented Sep 26, 2023

@lucario387 https://github.com/NvChad/NvChad/blob/v3.0/lua/plugins/configs/lspconfig.lua#L18

DOES NOT WORK STILL. Even with example config from fresh install WITH lsp.signature.disabled = true.

@siduck
Copy link
Member

siduck commented Sep 26, 2023

@lucario387 https://github.com/NvChad/NvChad/blob/v3.0/lua/plugins/configs/lspconfig.lua#L18

DOES NOT WORK STILL. Even with example config from fresh install WITH lsp.signature.disabled = true.

wait for some minutes, i'm rewriting signature.lua

@yoplat
Copy link
Contributor Author

yoplat commented Sep 26, 2023

@lucario387 https://github.com/NvChad/NvChad/blob/v3.0/lua/plugins/configs/lspconfig.lua#L18

DOES NOT WORK STILL. Even with example config from fresh install WITH lsp.signature.disabled = true.

wait for some minutes, i'm rewriting signature.lua

Also, in lspconfig.lua you are checking conf.signature:

if conf.signature and client.server_capabilities.signatureHelpProvider then
   require("nvchad.signature").setup(client)
end

But from the default_config (lsp.signature.disabled) it should be:

if not conf.signature.disabled and client.server_capabilities.signatureHelpProvider then
   require("nvchad.signature").setup(client)
end

So you need to change the defualt_config or how you are checking that to be enabled

@siduck
Copy link
Member

siduck commented Sep 26, 2023

@carlobonandriniunimi check now

@yoplat
Copy link
Contributor Author

yoplat commented Sep 26, 2023

@carlobonandriniunimi check now

FIXED IT! Thanks! I think the biggest problem that I was having is that I was still using lsp.signature.disabled = false insted of lsp.signature = false.

@siduck
Copy link
Member

siduck commented Sep 26, 2023

@carlobonandriniunimi check now

FIXED IT! Thanks! I think the biggest problem that I was having is that I was still using lsp.signature.disabled = false insted of lsp.signature = false.

yea ik, although this problem led me into having a clean re-write of the signature module! 38782b6 😬

roy-ren pushed a commit to roy-ren/NvChad that referenced this pull request Oct 13, 2023
roy-ren pushed a commit to roy-ren/NvChad that referenced this pull request Jan 4, 2024
chkg2a pushed a commit to chkg2a/neovim that referenced this pull request Feb 14, 2024
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.

None yet

3 participants