Skip to content

Ignore disabled diagnostics #1148

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

Merged
merged 2 commits into from
Sep 16, 2023
Merged

Ignore disabled diagnostics #1148

merged 2 commits into from
Sep 16, 2023

Conversation

sidequestboy
Copy link
Contributor

If you disable a diagnostic with vim.diagnostic.disable, don't count towards the diagnostic count.

@cseickel
Copy link
Contributor

I am hesitating because I wonder if we could instead filter them when pulling the diagnostics to begin with, rather than calling a function to check each one later. Have you looked into that?

@sidequestboy
Copy link
Contributor Author

sidequestboy commented Sep 12, 2023

I agree it doesn't seem great to check the namespace on every diagnostic. iirc, what we can do is filter by namespace to the vim.diagnostic.get and do the check only once per namespace usingvim.diagnostic.get_namespaces - I think we would need one call get for each namespace for each open buffer, compared with once per buffer currently. (still probably better than checking on each diagnostic) There didn't seem to be a built in way to filter the diagnostics by disabled status in the call toget though

I can rewrite this patch later today!

@sidequestboy
Copy link
Contributor Author

sidequestboy commented Sep 13, 2023

WIP - I changed the shape of the table returned from get_diagnostics_count without good reason I think (indexed counts with severity_number instead of severity_string by mistake, and also only stored the most severe diagnostic count unlike before)

@sidequestboy
Copy link
Contributor Author

ok, the most recent force-pushed branch is good to review. Let me know what you think.

Copy link
Contributor

@cseickel cseickel left a comment

Choose a reason for hiding this comment

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

I think this looks great. Let's just go ahead and remove the legacy lua vim global filter though.

Copy link
Contributor

@cseickel cseickel left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@cseickel cseickel merged commit e9edc00 into nvim-neo-tree:main Sep 16, 2023
@wookayin
Copy link
Contributor

wookayin commented Sep 19, 2023

Is this a breaking change or a regression? After this commit I keep getting

2023-09-19T01:56:50 Neo-tree  ERROR Error in seed function for vim_diagnostic_changed: .../neo-tree.nvim/lua/neo-tree/utils/init.lua:236: attempt to call field 'is_disabled' (a nil value)
2023-09-19T01:56:55 Neo-tree  ERROR Error in event handler for event state_created[buffers.state_created]: ...neo-tree.nvim/lua/neo-tree/utils/init.lua:236: attempt to call field 'is_disabled' (a nil value)
2023-09-19T01:56:55 Neo-tree  ERROR Error in event handler for event state_created[git_status.state_created]: .../neo-tree.nvim/lua/neo-tree/utils/init.lua:236: attempt to call field 'is_disabled' (a nil value)

for neovim 0.8.x.

Note that the API is_disabled() doesn't exist in 0.8:

vim.diagnostic.is_disabled() | neovim: +v0.9.0

@sidequestboy
Copy link
Contributor Author

apologies, it is a breaking change - I'll add a nil check before calling that function

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.

3 participants