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

ruff server: Ruff configuration from client settings overrides project configuration #11062

Merged
merged 4 commits into from
Apr 23, 2024

Conversation

snowsignal
Copy link
Contributor

@snowsignal snowsignal commented Apr 21, 2024

Summary

This is a follow-up to #10984 that implements configuration resolution for editor configuration. By 'editor configuration', I'm referring to the client settings that correspond to Ruff configuration/options, like preview, select, and so on. These will be combined with 'project configuration' (configuration taken from project files such as pyproject.toml) to generate the final linter and formatter settings used by RuffSettings. Editor configuration takes priority over project configuration.

In a follow-up pull request, I'll implement a new client setting that allows project configuration to override editor configuration, as per this issue.

Review guide

The first commit, e38966d, is just doing re-arrangement so that we can pass the right things to RuffSettings::resolve. The actual resolution logic is in the second commit, 0eec9ee. It might help to look at these comments individually since the diff is rather messy.

Test Plan

For the settings to show up in VS Code, you'll need to checkout this branch: astral-sh/ruff-vscode#456.

To test that the resolution for a specific setting works as expected, run through the following scenarios, setting it in project and editor configuration as needed:

Set in project configuration? Set in editor configuration? Expected Outcome
No No The editor should behave as if the setting was set to its default value.
Yes No The editor should behave as if the setting was set to the value in project configuration.
No Yes The editor should behave as if the setting was set to the value in editor configuration.
Yes Yes (but distinctive from project configuration) The editor should behave as if the setting was set to the value in editor configuration.

An exception to this is extendSelect, which does not have an analog in TOML configuration. Instead, you should verify that extendSelect amends the select setting. If select is set in both editor and project configuration, extendSelect will only append to the select value in editor configuration, so make sure to un-set it there if you're testing extendSelect with select in project configuration.

@snowsignal snowsignal added configuration Related to settings and configuration server Related to the LSP server labels Apr 21, 2024
Copy link
Contributor

github-actions bot commented Apr 21, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@snowsignal
Copy link
Contributor Author

I'm working on adding some snapshot tests, but the PR should be ready for review regardless.

@snowsignal snowsignal requested a review from MichaReiser April 22, 2024 20:50
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Thank you

@snowsignal
Copy link
Contributor Author

I'll implement snapshot tests as a follow-up

@snowsignal snowsignal merged commit 35ca887 into main Apr 23, 2024
18 checks passed
@snowsignal snowsignal deleted the jane/server/settings/resolution branch April 23, 2024 18:19
snowsignal added a commit to astral-sh/ruff-vscode that referenced this pull request Apr 25, 2024
## Summary

This is a follow-up to astral-sh/ruff#10984,
which implemented support for common Ruff configuration options in
client settings. This PR exposes these new settings in the extension
configuration.

These settings are different from other extension settings, because they
don't have default values. Instead, if the user does not set them, they
will be sent as `null`, and the server will use project configuration
instead.

At the moment, `ruff server` does not actually resolve these settings
yet - settings resolution is introduced in
astral-sh/ruff#11062, which means that you'll
need to work from that branch to test this PR (and vice-versa).

## Test Plan

See the test plan in astral-sh/ruff#11062.
snowsignal added a commit that referenced this pull request Apr 26, 2024
…er editor configuration (#11086)

## Summary

This is intended to address
astral-sh/ruff-vscode#425, and is a follow-up
to #11062.

A new client setting is now supported by the server,
`prioritizeFileConfiguration`. This is a boolean setting (default:
`false`) that, if set to `true`, will instruct the configuration
resolver to prioritize file configuration (aka discovered TOML files)
over configuration passed in by the editor.

A corresponding extension PR has been opened, which makes this setting
available for VS Code:
astral-sh/ruff-vscode#457.

## Test Plan

To test this with VS Code, you'll need to check out [the VS Code
PR](astral-sh/ruff-vscode#457) that adds this
setting.

The test process is similar to
#11062, but in scenarios where the
editor configuration would take priority over file configuration, file
configuration should take priority.
snowsignal added a commit to astral-sh/ruff-vscode that referenced this pull request May 13, 2024
## Summary

This is a follow-up to astral-sh/ruff#10984,
which implemented support for common Ruff configuration options in
client settings. This PR exposes these new settings in the extension
configuration.

These settings are different from other extension settings, because they
don't have default values. Instead, if the user does not set them, they
will be sent as `null`, and the server will use project configuration
instead.

At the moment, `ruff server` does not actually resolve these settings
yet - settings resolution is introduced in
astral-sh/ruff#11062, which means that you'll
need to work from that branch to test this PR (and vice-versa).

## Test Plan

See the test plan in astral-sh/ruff#11062.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Related to settings and configuration server Related to the LSP server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants