Skip to content
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.

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

Avoid erasing files when Ruff fails with an error #341

Merged
merged 1 commit into from
Dec 14, 2023
Merged

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Dec 14, 2023

Summary

A user reported an issue in the VS Code extension (astral-sh/ruff#9123) in which their files were being erased on-save. The issue is that we don't check the exit code of all commands, and so if ruff check fails with an error, then we end up wiping the file entirely.

This PR adds exit code-checking for all commands in the LSP. Further, we now surface such failures to the user directly (see bottom right):

Screen Shot 2023-12-13 at 10 57 20 PM

Closes astral-sh/ruff#9123

# For `ruff check`, 0 indicates successful completion with no remaining
# diagnostics, 1 indicates successful completion with remaining diagnostics,
# and 2 indicates an error.
if result.exit_code != 0 and result.exit_code != 1:
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I don't know what's more idiomatic Python

Suggested change
if result.exit_code != 0 and result.exit_code != 1:
if not result.exit_code in [0, 1]:

Copy link
Member

Choose a reason for hiding this comment

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

😬

if result.exit_code not in [0, 1]:
	...

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 not in (0, 1) is most idiomatic, probably.

Copy link
Member

Choose a reason for hiding this comment

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

😬

if result.exit_code not in [0, 1]:
	...

Python's hard

Copy link
Member Author

Choose a reason for hiding this comment

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

Haha both work!

ruff_lsp/server.py Show resolved Hide resolved
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix! It works on Notebook as expected.

@charliermarsh charliermarsh merged commit 4e02c2d into main Dec 14, 2023
20 checks passed
@charliermarsh charliermarsh deleted the charlie/error branch December 14, 2023 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ruff VSCode extension erases entire file on save
3 participants