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

Allow setting pylsp.plugins.ruff.format to ["ALL"] #101

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

willjow
Copy link

@willjow willjow commented Oct 25, 2024

I'm a new user of ruff, so apologies if this is not intended usage!

When setting up my ruff config, I thought it might be fun to aggressively format using all of ruff's rules in the absence of any project settings.

However, it appears that the ignore option dominates the select option for ruff check if the same rule code is specified in each. Thus, the plugin will not currently work as expected if pylsp.plugins.ruff.format is set to ["ALL"].

In this case, the ignore will take precedence and setting
`pylsp.plugins.ruff.format` to `["ALL"]` will do nothing.
Update existing test text to be compatible with the case that a format
is run with `format` set to `["ALL"]`.
@@ -142,7 +142,7 @@ def pylsp_format_document(workspace: Workspace, document: Document) -> Generator
# specifying `format = ["I"]` to get import sorting as part of formatting.
new_text = run_ruff(
settings=PluginSettings(
ignore=["ALL"],
ignore=["ALL"] if "ALL" not in settings.format else [],
Copy link
Author

Choose a reason for hiding this comment

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

This is admittedly a bit of a quick and dirty patch; would it be preferred to track this condition when the settings are parsed?

@@ -121,3 +127,22 @@ def test_ruff_format_and_sort_imports(workspace):
)
got = run_plugin_format(workspace, doc)
assert want == got


def test_ruff_format_via_all(workspace):
Copy link
Author

Choose a reason for hiding this comment

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

This is somewhat of a fragile thing to test since it's at the mercy of whatever rules ruff might add. The tested text might be reasonably robust, but it also might be best to just leave this out.

@willjow willjow marked this pull request as ready for review October 25, 2024 04:44
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.

1 participant