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(diagnostic): show backtrace for deprecation warnings #28824

Merged
merged 1 commit into from
May 19, 2024

Conversation

wookayin
Copy link
Member

@wookayin wookayin commented May 17, 2024

Problem: On nvim 11.0-dev, deprecation warnings due to an use of hard-deprecated APIs such as:

  • vim.diagnostic.disable()
  • vim.diagnostic.is_disabled()

etc. are not accompanied by backtrace information.

It makes difficult for users to figure out which lines or which plugins are still using deprecated APIs.

Solution: use backtrace = true in vim.deprecate() call.

@wookayin
Copy link
Member Author

wookayin commented May 17, 2024

Note: The argument deprecate = false was first introduced in #26618, and I don't see any special reason to suppress backtrace here as well. For other places where vim.deprecate() is used usually, stacktrace information is not suppressed.

EDIT: seems it was intentional? https://github.com/neovim/neovim/pull/26618/files#r1429248469

@wookayin wookayin added this to the 0.11 milestone May 17, 2024
@github-actions github-actions bot requested a review from gpanders May 17, 2024 23:30
@gpanders
Copy link
Member

EDIT: seems it was intentional? https://github.com/neovim/neovim/pull/26618/files#r1429248469

That particular instance was intentional, because the backtrace there doesn't provide any useful information. It's not a call site of a specific function, it's warning about the use of sign_define for diagnostic signs, which could be happening anywhere in the user's config. The backtrace in this case would just be misleading.

@wookayin
Copy link
Member Author

I see, thanks for the explanation. This might be too niche, but in my specific configs it was a plugin (neo-tree) rather than user config:

vim.diagnostic.is_disabled() is deprecated, use vim.diagnostic.is_enabled() instead. :help deprecated
Feature will be removed in Nvim 0.12
stack traceback:
        ...neovim/runtime/lua/vim/diagnostic.lua:1597: in function 'is_disabled'
        .../neo-tree.nvim/lua/neo-tree/utils/init.lua:244: in function 'get_diagnostic_counts'
        .../neo-tree.nvim/lua/neo-tree/setup/init.lua:45: in function <.../neo-tree.nvim/lua/neo-tree/setup/init.lua:44>
        [C]: in function 'pcall'
        .../neo-tree.nvim/lua/neo-tree/events/queue.lua:65: in function 'fire_event_internal'
        .../neo-tree.nvim/lua/neo-tree/events/queue.lua:100: in function <....neo-tree.nvim/lua/neo-tree/events/queue.lua:99>
        [C]: in function 'pcall'
        .../neo-tree.nvim/lua/neo-tree/utils/init.lua:126: in function 'debounce'
        .../neo-tree.nvim/lua/neo-tree/utils/init.lua:71: in function ''
        vim/_editor.lua: in function ''
        vim/_editor.lua: in function <vim/_editor.lua:0>

So I think backtrace information could be still useful for this particular case as well as for user configs?

@gpanders
Copy link
Member

So I think backtrace information could be still useful for this particular case as well as for user configs?

For vim.diagnostic.disable() and vim.diagnostic.is_disabled() I agree. I don't know why the backtrace was omitted from those.

Problem: On nvim 11.0-dev, deprecation warnings due to an use of
hard-deprecated APIs such as:
- `vim.diagnostic.disable()`
- `vim.diagnostic.is_disabled()`
etc. are not accompanied by backtrace information. It makes difficult
for users to figure out which lines or which plugins are still using
deprecated APIs.

Solution: use `backtrace = true` in vim.deprecate() call.
@wookayin
Copy link
Member Author

That particular instance was intentional, because the backtrace there doesn't provide any useful information. It's not a call site of a specific function, it's warning about the use of sign_define for diagnostic signs, which could be happening anywhere in the user's config. The backtrace in this case would just be misleading.

This makes sense -- backtrace should be omitted for this. I've updated the PR accordingly.

@dundargoc dundargoc merged commit 9a43ec1 into neovim:master May 19, 2024
29 checks passed
@wookayin wookayin deleted the deprecate-trace branch May 19, 2024 22:04
@github-actions github-actions bot removed the request for review from gpanders May 19, 2024 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants