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

LSP saga floating window design #87

Closed
kaykhan opened this issue Apr 8, 2023 · 9 comments · Fixed by sainnhe/everforest#137
Closed

LSP saga floating window design #87

kaykhan opened this issue Apr 8, 2023 · 9 comments · Fixed by sainnhe/everforest#137
Labels
bug Something isn't working

Comments

@kaykhan
Copy link

kaykhan commented Apr 8, 2023

Hi using im using lsp saga and hovering over a function a floating window appears. I want to remove the padding on this floating window and change the background color of it.

Any idea how i might go about doing that by modifying this theme?

image

image

@kaykhan kaykhan closed this as completed Apr 8, 2023
@antoineco
Copy link
Collaborator

That looks like a bug to me, see the following issue for the expected rendering of LSP Saga's pop-ups: sainnhe/everforest#88

You closed the issue almost immediately after opening it, does it mean you found a solution? If yes, could you please share your findings?

@kaykhan
Copy link
Author

kaykhan commented Apr 8, 2023

@antoineco My issue was completely different and i resolved it on my own.

But i realised what i had shown does resemble that issue in the referenced issue on everforest.

I was able to replicate that problem on sonokai.

image

@kaykhan kaykhan reopened this Apr 8, 2023
@antoineco
Copy link
Collaborator

Strange, we fixed that in #67. Maybe something changed in LSP Saga since that fix.

@antoineco
Copy link
Collaborator

OK looks like there were indeed a couple of breaking changes introduced in LSP Saga, and there is now a single highlight group for all diagnostic borders: https://github.com/glepnir/lspsaga.nvim/blob/44af2ab1/lua/lspsaga/highlight.lua#L60

Ref. nvimdev/lspsaga.nvim#586 (highlight.lua)

@kaykhan
Copy link
Author

kaykhan commented Apr 9, 2023

OK looks like there were indeed a couple of breaking changes introduced in LSP Saga, and there is now a single highlight group for all diagnostic borders: https://github.com/glepnir/lspsaga.nvim/blob/44af2ab1/lua/lspsaga/highlight.lua#L60

Ref. glepnir/lspsaga.nvim#586 (highlight.lua)

Will there be a fix made in this theme to support this change?

@antoineco
Copy link
Collaborator

Yes, and ported to all sainnhe's color schemes as soon as I test the potential fix locally.

@antoineco antoineco added the bug Something isn't working label Apr 11, 2023
@antoineco
Copy link
Collaborator

antoineco commented Apr 11, 2023

I just looked into this issue and things are a bit more granular than I though.

By default, LSP Saga has the option diagnostic.border_follow set to true. In this configuration, the borders of all diagnostic pop-ups are rendered with the highlight group of the corresponding diagnostic type.

For example, a Hint pop-up is created with a border which highlight group is linked to DiagnosticHint. If I re-link DiagnosticHint to Green, the border changes as follows:

image
image

This is bad for two reasons:

  1. LSP Saga hijacks highlight groups related to text for graphics.
  2. We render DiagnosticHint, etc. with undercurl to emulate the style adopted by most IDEs to highlight code related to a diagnostic.
    image

Before, we could get away with setting LspSagaDiagnosticHint, etc. to a non-undercurl style. Since the recent changes in LSP Saga, it is no longer possible to customize LSP Saga specifically. In my opinion this is a regression, and plugins should under no circumstances expect to be customized only through Vim's internal highlight groups, especially to render graphical elements.

Some plugins—like lualine for example—are graceful enough to allow the user to customize fallback groups. LSP Saga could adopt a similar approach and use DiagnosticHint by default, while honoring LspSagaDiagnosticHint if it exists.

We are most definitely not going to override internal highlight groups to accommodate one specific plugin, so unless the author of LSP Saga changes their approach to border rendering, LSP Saga won't be supported in Sonokai / Gruvbox material / Everforest / Edge moving forward.

@antoineco antoineco added the wontfix This will not be worked on label Apr 11, 2023
@kaykhan
Copy link
Author

kaykhan commented Apr 11, 2023

Okay i appreciate the effort looking into this. Maybe its something to bring up to the maintainer of LSPSaga.

@antoineco
Copy link
Collaborator

Cross referencing sainnhe/everforest#136 (same root cause)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants