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

Sign rendering code seems to have an inverted condition #448

Closed
therealprof opened this issue May 30, 2024 · 8 comments · Fixed by #445
Closed

Sign rendering code seems to have an inverted condition #448

therealprof opened this issue May 30, 2024 · 8 comments · Fixed by #445

Comments

@therealprof
Copy link

therealprof commented May 30, 2024

Hi,

I was briefly looking into why trouble fails to render the correctly configured sign icons and instead uses the first letter of the severity instead.

It occured to me that this condition is wrong

if vim.fn.has("nvim-0.10.0") == 1 then

since the else branch uses vim.fn.sign_getdefined() which does exist in nvim 0.10. Indeed flipping the condition makes trouble render the icon instead of the letter just fine.

However I'm not sure there's no more correct way to do version checks since that first branch is probably supposed to be used for all older versions instead of all versions not being 0.10 and I'm not sure how those nvim feature flags work.

@folke
Copy link
Owner

folke commented May 30, 2024

diagnostics signs on >= 0.10 should be configured through vim.diagnostic.config, not using sign_define

@folke
Copy link
Owner

folke commented May 30, 2024

#449

@folke folke closed this as not planned Won't fix, can't repro, duplicate, stale May 30, 2024
@therealprof
Copy link
Author

It seems rather odd to declare a non-deprecated API, literally everyone uses, obsolete and break existing configs for no apparent reason. gitsigns, lualine, trouble v2 and other plugins work just fine with sign_define, only trouble v3 looks rather dated now.

With your pointer and the help of https://neovim.io/doc/user/diagnostic.html#diagnostic-signs I managed to adjust the config to support old and new style but since I couldn't find any tutorials or example configs using the "recommded" API, I think it will take a while until the world catches up...

@folke folke reopened this May 30, 2024
@folke
Copy link
Owner

folke commented May 30, 2024

Allright, I'll see if I can just also use the old values if needed.

@folke folke closed this as completed in 36545cb May 30, 2024
@folke
Copy link
Owner

folke commented May 30, 2024

I think I fixed it

@therealprof
Copy link
Author

Doesn't look like it, with the latest main and after backing out the new configuration:

Screenshot 2024-05-31 at 19 29 37

@folke
Copy link
Owner

folke commented May 31, 2024

Can you check again?

@therealprof
Copy link
Author

Looking good now. 👍🏻

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 a pull request may close this issue.

2 participants