-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Conside include
, extend-include
for the native server
#12252
Conversation
|
d83b0f2
to
9041186
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also need this for fix_all
?
ruff/crates/ruff_server/src/fix.rs
Lines 34 to 59 in e6e09ea
// If the document is excluded, return an empty list of fixes. | |
let package = if let Some(document_path) = document_path.as_ref() { | |
if let Some(exclusion) = match_any_exclusion( | |
document_path, | |
&file_resolver_settings.exclude, | |
&file_resolver_settings.extend_exclude, | |
Some(&linter_settings.exclude), | |
None, | |
) { | |
tracing::debug!( | |
"Ignored path via `{}`: {}", | |
exclusion, | |
document_path.display() | |
); | |
return Ok(Fixes::default()); | |
} | |
detect_package_root( | |
document_path | |
.parent() | |
.expect("a path to a document should have a parent path"), | |
&linter_settings.namespace_packages, | |
) | |
} else { | |
None | |
}; |
and format_text_document_range
?
ruff/crates/ruff_server/src/server/api/requests/format_range.rs
Lines 51 to 63 in 8338db6
// If the document is excluded, return early. | |
if let Some(file_path) = query.file_path() { | |
if let Some(exclusion) = match_any_exclusion( | |
&file_path, | |
&file_resolver_settings.exclude, | |
&file_resolver_settings.extend_exclude, | |
None, | |
Some(&formatter_settings.exclude), | |
) { | |
tracing::debug!("Ignored path via `{}`: {}", exclusion, file_path.display()); | |
return Ok(None); | |
} | |
} |
Should we have a unified helper to avoid the code repetion?
Yes, we'll require for both. I think it'd make sense to add a helper for that. |
8af9e89
to
3b3cf58
Compare
3b3cf58
to
83003dc
Compare
5ad936f
to
1bf727d
Compare
Summary
This PR updates the native server to consider the
include
andextend-include
file resolver settings.fixes: #12242
Test Plan
Note: Settings reloading doesn't work for nested configs which is fixed in #12253 so the preview here only showcases root level config.
Screen.Recording.2024-07-09.at.12.46.45.mov