search

neovim / pynvim

Python client and plugin host for Nvim
http://pynvim.readthedocs.io/en/latest/
Apache License 2.0
1.48k stars 118 forks source link

Add explicit API function definitions #493

Closed stevearc closed 11 months ago

stevearc commented 2 years ago

Depends on #492

With the addition of type annotations, there is now a reason to explicitly enumerate the API methods instead of relying on the magic __getattr__. If they are explicitly provided, plugin authors will get better autocompletion (via LSP) and more type safety guarantees.

Since this depends on #492, it has all of its commits. I'll rebase it once merged, but if anyone knows of a better Github workflow for dependent PR's, please let me know.

lgtm-com[bot] commented 2 years ago

This pull request introduces 2 alerts when merging 51fef93582bf7778b49a106d5b8265d223c26f60 into 581cba4fe027b4e02619ad69cc672c742e45cce1 - view on LGTM.com

new alerts:

justinmk commented 2 years ago

Is there a test that checks the types against the actual nvim --api-info schema? That is a minimum requirement imo.

Also would be great if we could generate this instead of manually maintaining it. Then we could have a CI bot that sends PRs automatically.

stevearc commented 2 years ago

@justinmk That's a great idea! I'm happy to take a crack at auto-generation, or at least auto-verification. The PR this depends on is just collecting dust at the moment and I haven't seen much indication that it will be merged, so I'll probably defer the work until after there's more progress there.

justinmk commented 2 years ago

I'm happy to take a crack at auto-generation, or at least auto-verification

just auto-verification is enough to move forward. I would even say, let's wait on auto-generation until we get this first phase merged.

I think we're ready to move forward with https://github.com/neovim/pynvim/pull/492#issuecomment-889478664

justinmk commented 11 months ago

Rebased after merging #492.

Failing Windows CI is a known issue, unrelated to this PR.

justinmk commented 11 months ago

Not in favor of this, for same reasoning as in https://github.com/neovim/node-client/pull/199#issuecomment-1628814410 : pynvim is a useful "reference implementation" Neovim client, but we need to be ruthless about keeping it extremely low-maintenance, because of the lack of ownership in the last 2+ years.

Most APIs should be autogenerated at runtime, if at all. Meanwhile, request('nvim_...') is always available to clients.