fix(pyright): pyright.organizeimports command fails

[michelesr]

Fix error raised when running :LspPyrightOrganizeImports :

Language server `pyright` does not support command `pyright.organizeimports`. This command may require a client extension.

[justinmk]

not sure why you changed this (always need to mention that in the pr), but if your intention is to pass bufnr, that can be done as the 2nd arg to exec_cmd

client:exec_cmd({ ... }, { bufnr = bufnr })

[michelesr]

@justinmk It's explained in the PR description and commit msg: client:exec_cmd() was raising an error about that command not being supported by the server and aborting, so I got the old version from here but I assumed you needed to pass bufnr instead of 0; it works with 0 too, but it doesn't work if you change it back to client:exec_cmd().

[justinmk]

It's explained in the PR description and commit msg: client:exec_cmd() was raising an error about that command not being supported by the server and aborting,

That doesn't explain anything. client:exec_cmd() should work.

it doesn't work if you change it back to client:exec_cmd().

Did you pass bufnr in the 2nd dict arg as I suggested?

[michelesr]

Did you pass bufnr in the 2nd dict arg as I suggested?

      client:exec_cmd({
        command = 'pyright.organizeimports',
        arguments = { vim.uri_from_bufnr(bufnr) },
      }, {bufnr = bufnr})

This raises the same error.

[michelesr]

@justinmk I think it's because of microsoft/pyright#10406 ?

[justinmk]

? client:exec_cmd performs "workspace/executeCommand". The only difference is that exec_cmd currently is always asynchronous (but so is client.request).

So, there is some other problem, and the change you've made here only accidentally "fixes" a symptom. Need to find out what the actual problem is.

I wonder if client.request just doesn't show the error?

Based on microsoft/pyright#10406 , it seems like the server is literally telling us not to use this feature. So if your intent is to ignore that and use it anyway, that sure is worth mentioning in the PR description (and commit message) and a code comment.

Here's an outline of an improved approach:

1. use pcall or ignore the error in some other explicit manner
2. put a 1-line code comment mentioning that we are intentionally ignoring the server error and this could break in the future. also link to the upstream issue
3. actually mention this in the PR description and commit message

[michelesr]

So if your intent is to ignore that and use it anyway, that sure is worth mentioning in the PR description (and commit message) and a code comment.

Not really, I just found out about this, because I moved my config to use vim.lsp.config(), and noticed the difference between the code in lua/ and the one in lsp/. I don't rely on this feature and there are better alternatives (like using !ruff check --fix --select I % for example).

I'm not sure what's the best decision to take here, if the feature is discouraged from the pyright maintainers then is probably better to just remove the vim command altoghether rather than leaving it broken?

I agree that wrapping exec_cmd with pcall() seems an hack, and I've tried to do so but it still prints the message and doesn't perform the required operation.

[justinmk]

Let's wait a bit and see if others have comments.