Update server to return the debug info as string by dhruvmanila · Pull Request #16214 · astral-sh/ruff

dhruvmanila · 2025-02-17T15:42:02Z

Summary

This PR updates the ruff.printDebugInformation command to return the info as string in the response. Currently, we send a window/logMessage request with the info but that has the disadvantage that it's not visible to the user directly.

What rust-analyzer does with it's rust-analyzer/status request which returns it as a string which then the client can just display it in a separate window. This is what I'm thinking of doing as well.

Other editors can also benefit from it by directly opening a temporary file with this information that the user can see directly.

There are couple of options here:

Keep using the command, keep the log request and return the string
Keep using the command, remove the log request and return the string
Create a new request similar to rust-analyzer/status which returns a string

This PR implements (1) but I'd want to move towards (2) and remove the log request completely. We haven't advertised it as such so this would only require updating the VS Code extension to handle it by opening a new document with the debug content.

Test plan

For VS Code, refer to astral-sh/ruff-vscode#694.

For Neovim, one could do:

local function execute_ruff_command(command)
  local client = vim.lsp.get_clients({ 
    bufnr = vim.api.nvim_get_current_buf(), 
    name = name,
    method = 'workspace/executeCommand',
  })[1]
  if not client then
    return
  end
  client.request('workspace/executeCommand', {
    command = command,
    arguments = {
      { uri = vim.uri_from_bufnr(0) }
    },
    function(err, result)
      if err then
        -- log error
        return
      end
      vim.print(result)
      -- Or, open a new window with the `result` content
    end
  }

github-actions · 2025-02-17T15:48:26Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

dhruvmanila · 2025-02-18T04:34:14Z

crates/ruff_server/src/server/api/requests/execute_command.rs

            notifier
                .notify::<types::notification::LogMessage>(types::LogMessageParams {
-                    message: output,
+                    message: output.clone(),
                    typ: types::MessageType::INFO,
                })
                .with_failure_code(ErrorCode::InternalError)?;


I prefer to remove the logging part and I don't think it should break anything.

I'm in favor of removing it, but wouldn't it break old VS code clients that don't have the required code to open the document on the client side?

Hmm yeah. So, that would happen if the user is using a non-latest extension version with the latest ruff in which case I think we should recommend them to upgrade the extension version as it always include improvements and bug fixes. I'm fine keeping this here as well for the time being and we can remove it with 0.10.

Can we include a TODO or similar that it should be removed? Or do we want to keep it, considering that other clients can't easily call commands?

I think we should remove it in a later version, I'll create an issue for 0.10.

MichaReiser

Could you add a test plan showing how this works for clients without an editor extension (e.g. neovim)? How does the client know that it has to open a document? Is this the default behavior of a custom command?

dhruvmanila · 2025-02-18T08:44:05Z

Could you add a test plan showing how this works for clients without an editor extension (e.g. neovim)? How does the client know that it has to open a document? Is this the default behavior of a custom command?

We haven't documented the commands API anywhere nor is it easy to add the argument schema in the capabilities as the arguments are []LspAny (https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#executeCommandParams).

No, this is not the default behavior for a custom command. The user or the plugin author needs to know the argument schema for each command to implement it. As to how to pass in the document URI, that will be dependent on each editor. For example, in VS Code, it's just a vscode.window.activeTextEditor while in Neovim it would be vim.lsp.util.make_text_document_params.

Hmm, that makes me wonder why weren't we using the existing types from lsp_types like VersionedTextDocumentIdentifier instead of Argument. I'll make this as a follow-up change.

MichaReiser · 2025-02-18T09:34:27Z

crates/ruff_server/src/server/api/requests/execute_command.rs

            notifier
                .notify::<types::notification::LogMessage>(types::LogMessageParams {
-                    message: output,
+                    message: output.clone(),
                    typ: types::MessageType::INFO,
                })
                .with_failure_code(ErrorCode::InternalError)?;


Can we include a TODO or similar that it should be removed? Or do we want to keep it, considering that other clients can't easily call commands?

* main: (60 commits) [`refurb`] Manual timezone monkeypatching (`FURB162`) (#16113) [`pyupgrade`] Do not upgrade functional TypedDicts with private field names to the class-based syntax (`UP013`) (#16219) Improve docs for PYI019 (#16229) Refactor `CallOutcome` to `Result` (#16161) Fix minor punctuation errors (#16228) Include document specific debug info (#16215) Update server to return the debug info as string (#16214) [`airflow`] Group `ImportPathMoved` and `ProviderName` to avoid misusing (`AIR303`) (#16157) Fix unstable formatting of trailing end-of-line comments of parenthesized attribute values (#16187) Ignore source code actions for a notebook cell (#16154) Add FAQ entry for `source.*` code actions in Notebook (#16212) red-knot: move symbol lookups in `symbol.rs` (#16152) better error messages while loading configuration `extend`s (#15658) Format `index.css` (#16207) Improve API exposed on `ExprStringLiteral` nodes (#16192) Update Rust crate tempfile to v3.17.0 (#16202) Update cloudflare/wrangler-action action to v3.14.0 (#16203) Update NPM Development dependencies (#16199) Update Rust crate smallvec to v1.14.0 (#16201) Update Rust crate codspeed-criterion-compat to v2.8.0 (#16200) ...

dhruvmanila added the server Related to the LSP server label Feb 17, 2025

dhruvmanila force-pushed the dhruv/print-debug-info-return-string branch from ab63d88 to c700a50 Compare February 17, 2025 17:09

Update server to return the debug info as string

8abfba3

dhruvmanila force-pushed the dhruv/print-debug-info-return-string branch from c700a50 to 8abfba3 Compare February 17, 2025 17:18

dhruvmanila commented Feb 18, 2025

View reviewed changes

dhruvmanila marked this pull request as ready for review February 18, 2025 04:34

dhruvmanila requested a review from MichaReiser February 18, 2025 04:34

MichaReiser reviewed Feb 18, 2025

View reviewed changes

dhruvmanila requested a review from MichaReiser February 18, 2025 09:14

MichaReiser approved these changes Feb 18, 2025

View reviewed changes

dhruvmanila merged commit bb2a712 into main Feb 18, 2025
21 checks passed

dhruvmanila deleted the dhruv/print-debug-info-return-string branch February 18, 2025 09:46

dhruvmanila mentioned this pull request Feb 18, 2025

Remove log notification for server debug info #16225

Closed

BrewTestBot mentioned this pull request Feb 20, 2025

ruff 0.9.7 Homebrew/homebrew-core#208398

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Update server to return the debug info as string#16214

Update server to return the debug info as string#16214
dhruvmanila merged 1 commit intomainfrom
dhruv/print-debug-info-return-string

dhruvmanila commented Feb 17, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Feb 17, 2025 •

edited

Loading

Uh oh!

dhruvmanila Feb 18, 2025

Uh oh!

MichaReiser Feb 18, 2025

Uh oh!

dhruvmanila Feb 18, 2025

Uh oh!

MichaReiser Feb 18, 2025

Uh oh!

dhruvmanila Feb 18, 2025

Uh oh!

MichaReiser left a comment

Uh oh!

dhruvmanila commented Feb 18, 2025

Uh oh!

MichaReiser Feb 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

dhruvmanila commented Feb 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

github-actions bot commented Feb 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

dhruvmanila Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

dhruvmanila commented Feb 18, 2025

Uh oh!

MichaReiser Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dhruvmanila commented Feb 17, 2025 •

edited

Loading

github-actions bot commented Feb 17, 2025 •

edited

Loading

`ruff-ecosystem` results