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

Build settings index in parallel for the native server #12299

Merged
merged 2 commits into from
Jul 15, 2024

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jul 12, 2024

Summary

This PR updates the server to build the settings index in parallel using similar logic as python_files_in_path.

This should help with #11366 but ideally we would want to build it lazily.

Test Plan

cargo insta test

@dhruvmanila dhruvmanila added the server Related to the LSP server label Jul 12, 2024
Copy link
Contributor

github-actions bot commented Jul 12, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

warning: Detected debug build without --no-cache.
error: Failed to parse examples/chatgpt/gpt_actions_library/.gpt_action_getting_started.ipynb:11:1:1: Expected an expression
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_bigquery.ipynb:13:1:1: Expected an expression

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

ruff format --preview

warning: Detected debug build without --no-cache.
error: Failed to parse examples/chatgpt/gpt_actions_library/.gpt_action_getting_started.ipynb:11:1:1: Expected an expression
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_bigquery.ipynb:13:1:1: Expected an expression


let directory = entry.into_path();
let mut builder = WalkBuilder::new(root);
builder.standard_filters(true);
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to respect the respect_gitignore setting here?

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 so. Let me check.

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've added it where by default it's true and if we find any config file it'll use the value from that instead.

@MichaReiser
Copy link
Member

What's also important to note is that this will exclude directories that are ignored

@charliermarsh
Copy link
Member

Nice.

@dhruvmanila
Copy link
Member Author

What's also important to note is that this will exclude directories that are ignored

I don't think that's the case if there's no config file present in the root directory. This is because the first loop extracts the settings from the root config file:

for directory in root.ancestors() {
if let Some(pyproject) = settings_toml(directory).ok().flatten() {
let Ok(settings) = ruff_workspace::resolver::resolve_root_settings(
&pyproject,
Relativity::Parent,
&EditorConfigurationTransformer(editor_settings, root),
) else {
continue;
};
index.insert(
directory.to_path_buf(),
Arc::new(RuffSettings {
path: Some(pyproject),
file_resolver: settings.file_resolver,
linter: settings.linter,
formatter: settings.formatter,
}),
);
break;
}
}
// Add any settings within the workspace itself

This is then used when walking through the project files to determine whether a directory should be excluded or not:

Box::new(|result| {
let Ok(entry) = result else {
return WalkState::Continue;
};
// Skip non-directories.
if !entry
.file_type()
.is_some_and(|file_type| file_type.is_dir())
{
return WalkState::Continue;
}
let directory = entry.into_path();
tracing::debug!("Visiting: {}", directory.display());
// If the directory is excluded from the workspace, skip it.
if let Some(file_name) = directory.file_name() {
if let Some((_, settings)) = index
.read()
.unwrap()
.range(..directory.clone())
.rfind(|(path, _)| directory.starts_with(path))
{
if match_exclusion(&directory, file_name, &settings.file_resolver.exclude) {
tracing::debug!("Ignored path via `exclude`: {}", directory.display());
return WalkState::Continue;
} else if match_exclusion(
&directory,
file_name,
&settings.file_resolver.extend_exclude,
) {
tracing::debug!(
"Ignored path via `extend-exclude`: {}",
directory.display()
);
return WalkState::Continue;
}
}
}
if let Some(pyproject) = settings_toml(&directory).ok().flatten() {
let Ok(settings) = ruff_workspace::resolver::resolve_root_settings(
&pyproject,
Relativity::Parent,
&EditorConfigurationTransformer(editor_settings, root),
) else {
return WalkState::Continue;
};
index.write().unwrap().insert(
directory,
Arc::new(RuffSettings {
path: Some(pyproject),
file_resolver: settings.file_resolver,
linter: settings.linter,
formatter: settings.formatter,
}),
);
}
WalkState::Continue
})
});
let fallback = Arc::new(RuffSettings::fallback(editor_settings, root));

Maybe we can use the fallback settings if there is none in the root directory?

let fallback = Arc::new(RuffSettings::fallback(editor_settings, root));

@dhruvmanila dhruvmanila enabled auto-merge (squash) July 15, 2024 09:53
@dhruvmanila dhruvmanila merged commit ecd4b4d into main Jul 15, 2024
18 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/parallel-walk-builder branch July 15, 2024 09:57
dhruvmanila added a commit that referenced this pull request Jul 17, 2024
This was a leftover from #12299
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
server Related to the LSP server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants