Skip to content

Update server to return the debug info as string #16214

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

Merged
merged 1 commit into from
Feb 18, 2025
Merged

Update server to return the debug info as string #16214

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/ruff_server/src/server/api/requests/execute_command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ impl super::SyncRequestHandler for ExecuteCommand {
let output = debug_information(session);
notifier
.notify::<types::notification::LogMessage>(types::LogMessageParams {
message: output,
message: output.clone(),
typ: types::MessageType::INFO,
})
.with_failure_code(ErrorCode::InternalError)?;
Comment on lines 38 to 43
Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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

return Ok(None);
return Ok(Some(serde_json::Value::String(output)));
}

// check if we can apply a workspace edit
Expand Down
Loading