Conside `include`, `extend-include` for the native server by dhruvmanila · Pull Request #12252 · astral-sh/ruff

dhruvmanila · 2024-07-09T06:15:26Z

Summary

This PR updates the native server to consider the include and extend-include file resolver settings.

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

github-actions · 2024-07-09T06:36:30Z

`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

MichaReiser

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?

dhruvmanila · 2024-07-09T08:45:12Z

Yes, we'll require for both. I think it'd make sense to add a helper for that.

crates/ruff_server/src/server/api/requests/format.rs

dhruvmanila force-pushed the dhruv/server-include branch from d83b0f2 to 9041186 Compare July 9, 2024 07:13

dhruvmanila marked this pull request as ready for review July 9, 2024 07:18

dhruvmanila requested a review from MichaReiser July 9, 2024 07:18

dhruvmanila added bug Something isn't working server Related to the LSP server labels Jul 9, 2024

MichaReiser reviewed Jul 9, 2024

View reviewed changes

dhruvmanila force-pushed the dhruv/server-include branch 2 times, most recently from 8af9e89 to 3b3cf58 Compare July 9, 2024 12:35

Conside include, extend-include for the native server

83003dc

dhruvmanila force-pushed the dhruv/server-include branch from 3b3cf58 to 83003dc Compare July 9, 2024 13:21

MichaReiser approved these changes Jul 9, 2024

View reviewed changes

crates/ruff_server/src/server/api/requests/format.rs Outdated Show resolved Hide resolved

dhruvmanila enabled auto-merge (squash) July 10, 2024 03:37

Extract exclusion-inclusion to a helper function

1bf727d

dhruvmanila force-pushed the dhruv/server-include branch from 5ad936f to 1bf727d Compare July 10, 2024 04:09

dhruvmanila merged commit 0bb2fc6 into main Jul 10, 2024

dhruvmanila deleted the dhruv/server-include branch July 10, 2024 04:12

BrewTestBot mentioned this pull request Jul 15, 2024

ruff 0.5.2 Homebrew/homebrew-core#177352

Merged

This was referenced Jul 17, 2024

Improve logging in VSCode extension astral-sh/ruff-vscode#526

Closed

Native server does not work with Jupyter Notebook without configuration astral-sh/ruff-vscode#546

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Conside `include`, `extend-include` for the native server#12252

Conside `include`, `extend-include` for the native server#12252
dhruvmanila merged 2 commits intomainfrom
dhruv/server-include

dhruvmanila commented Jul 9, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Jul 9, 2024 •

edited

Loading

Uh oh!

MichaReiser left a comment •

edited

Loading

Uh oh!

dhruvmanila commented Jul 9, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	// 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
	};

	// 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);
	}
	}

Conversation

dhruvmanila commented Jul 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

github-actions bot commented Jul 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Formatter (stable)

Formatter (preview)

Uh oh!

MichaReiser left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhruvmanila commented Jul 9, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dhruvmanila commented Jul 9, 2024 •

edited

Loading

github-actions bot commented Jul 9, 2024 •

edited

Loading

`ruff-ecosystem` results

MichaReiser left a comment •

edited

Loading