Skip to content

feat(lsp): deprecate non-method client functions #31207

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 20, 2024
Merged

feat(lsp): deprecate non-method client functions #31207

merged 1 commit into from
Nov 20, 2024

Conversation

lewis6991
Copy link
Member

Deprecated:

  • client.request() -> client:request()
  • client.request_sync() -> client:request_sync()
  • client.notify() -> client:notify()
  • client.cancel_request() -> client:cancel_request()
  • client.stop() -> client:stop()
  • client.is_stopped() client:is_stopped()
  • client.supports_method() -> client:supports_method()
  • client.on_attach() -> client:on_attach()

Fixed docgen to link class fields to the full function doc.

@github-actions github-actions bot added the lsp label Nov 14, 2024
@lewis6991 lewis6991 force-pushed the feat/lspmethoddep branch 2 times, most recently from b25e1a3 to ff72349 Compare November 14, 2024 12:05
@lewis6991 lewis6991 force-pushed the feat/lspmethoddep branch 6 times, most recently from 4e3d152 to bab2cd2 Compare November 14, 2024 12:43
MariaSolOs
MariaSolOs reviewed Nov 15, 2024
View reviewed changes
@lewis6991 lewis6991 force-pushed the feat/lspmethoddep branch 6 times, most recently from e7b1981 to 5e7a75a Compare November 19, 2024 12:24
@lewis6991
Copy link
Member Author

@mfussenegger did you want to give any feedback on this. I admit it's a big piece of churn, but something I've wanted to do for a long time.

mfussenegger
mfussenegger approved these changes Nov 19, 2024
View reviewed changes
Copy link
Member

@mfussenegger mfussenegger left a comment

Choose a reason for hiding this comment

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

Just to be sure: The methods already worked in 0.10, so plugins can switch to method calls without breaking 0.10?

Otherwise lgtm

@lewis6991
Copy link
Member Author

lewis6991 commented Nov 19, 2024 •
edited
Loading

Yes, both work. The worst thing is all the LuaLS errors every plugin/config will now get.

EDIT:

The methods already worked in 0.10

Oh wait, no. The methods in 0.10 were all prefixed with _. This change removes the underscore and adds some logic to automagically insert self.

If plugins want to remain compatible then they need to continue to use the . method and accept the LuaLS errors.

@mfussenegger
Copy link
Member

If plugins want to remain compatible then they need to continue to use the . method and accept the LuaLS errors.

But it would also cause deprecation warnings, right?
Could we maybe delay the deprecation to 0.12, so we've 0.11 where both options work so plugins have a good upgrade path? Otherwise we're kinda forcing them to add bwc logic that we'd already have

@lewis6991
Copy link
Member Author

lewis6991 commented Nov 19, 2024 •
edited
Loading

But it would also cause deprecation warnings, right?

No deprecation warnings (I don't think) since I set the version to 0.12. We can set that higher. I don't care how long we maintain these for since it's only 24 LOC.

EDIT: updated it to 0.13.

@lewis6991
feat(lsp): deprecate non-method client functions
5eb6c74 
Deprecated:
- `client.request()` -> `client:request()`
- `client.request_sync()` -> `client:request_sync()`
- `client.notify()` -> `client:notify()`
- `client.cancel_request()` -> `client:cancel_request()`
- `client.stop()` -> `client:stop()`
- `client.is_stopped()` `client:is_stopped()`
- `client.supports_method()` -> `client:supports_method()`
- `client.on_attach()` -> `client:on_attach()`

Fixed docgen to link class fields to the full function doc.
@lewis6991 lewis6991 merged commit 454ae67 into neovim:master Nov 20, 2024
30 of 31 checks passed
@lewis6991 lewis6991 deleted the feat/lspmethoddep branch November 20, 2024 08:51
@lithammer lithammer mentioned this pull request Nov 21, 2024
Closed
@MariaSolOs MariaSolOs mentioned this pull request Dec 6, 2024
Merged
4 tasks
@dylanarmstrong dylanarmstrong mentioned this pull request Mar 29, 2025
Closed
@Julian
Copy link
Contributor

Julian commented Mar 29, 2025

I might be missing something, but

The methods already worked in 0.10, so plugins can switch to method calls without breaking 0.10?

This doesn't seem to be the case, specifically in 0.11:

vim.lsp.get_clients()[1]:request_sync('textDocument/hover', vim.lsp.util.make_position_params(0, 'utf-16'), 1000, 0)

works fine, but in 0.10 produces:

E5108: Error executing lua ...eovim@0.10/0.10.4/share/nvim/runtime/lua/vim/lsp/rpc.lua:291: Cannot serialise function: type not supported                                                                                                                                                   
stack traceback:                                                                                                                                                                                                                                                                            
        [C]: in function 'encode'                                                                                                                                                                                                                                                           
        ...eovim@0.10/0.10.4/share/nvim/runtime/lua/vim/lsp/rpc.lua:291: in function 'encode_and_send'                                                                                                                                                                                      
        ...eovim@0.10/0.10.4/share/nvim/runtime/lua/vim/lsp/rpc.lua:338: in function 'request'                                                                                                                                                                                              
        ...im@0.10/0.10.4/share/nvim/runtime/lua/vim/lsp/client.lua:679: in function '_request'                                                                                                                                                                                             
        ...im@0.10/0.10.4/share/nvim/runtime/lua/vim/lsp/client.lua:753: in function 'request_sync'                                                                                                                                                                                         
        [string ":lua"]:1: in main chunk

Let me know if an issue is welcome.

@lewis6991
Copy link
Member Author

See #31207 (comment)

:request_sync() isn't available in 0.10, but :_request_sync() is.

@Julian
Copy link
Contributor

Julian commented Mar 29, 2025

Oh I'm bad at reading the edit... Thanks.

Julian added a commit to Julian/lean.nvim that referenced this pull request Mar 29, 2025
@Julian
Revert c8d6c38.
97173f4 
This isn't 0.10-compatible.

See neovim/neovim#31207 (comment)
@MariaSolOs MariaSolOs mentioned this pull request Mar 31, 2025
Merged
topaxi added a commit to topaxi/lsp-lens.nvim that referenced this pull request Apr 2, 2025
@topaxi
fix: use client method instead of non-method function
4820d13 
See: neovim/neovim#31207
@topaxi topaxi mentioned this pull request Apr 2, 2025
Open
@MariaSolOs MariaSolOs mentioned this pull request Apr 7, 2025
Merged
@phanen phanen mentioned this pull request Apr 17, 2025
Merged
@MariaSolOs MariaSolOs mentioned this pull request Apr 27, 2025
Merged
@wookayin wookayin added this to the 0.11 milestone May 4, 2025
@wookayin wookayin mentioned this pull request May 4, 2025
Closed
@MariaSolOs MariaSolOs mentioned this pull request Jun 29, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MariaSolOs MariaSolOs MariaSolOs left review comments

@mfussenegger mfussenegger mfussenegger approved these changes

@justinmk justinmk justinmk approved these changes

Assignees
No one assigned
Labels
lsp
Projects
None yet
Milestone
0.11
Development

Successfully merging this pull request may close these issues.
6 participants
@lewis6991 @mfussenegger @Julian @justinmk @MariaSolOs @wookayin