Update `ruff check` and `ruff format` to default to the current directory #8791

zanieb · 2023-11-20T16:35:58Z

Closes #7347
Closes #3970 via use of include

We could update examples in our documentation, but I worry since we do not have versioned documentation users on older versions would be confused. Instead, I'll open an issue to track updating use of ruff check . in the documentation sometime in the future.

…tory

zanieb · 2023-11-20T16:41:27Z

I'm looking into the behavior this exhibits with include per #3970 — it looks like more changes will be needed to properly support the desired behavior there.

zanieb · 2023-11-20T17:17:21Z

crates/ruff_cli/tests/integration_test.rs

    ----- stderr -----
+    warning: Ignoring file . in favor of standard input.


I'm not enthused about this change. I'm unsure how best to fix it. I'm not sure I want to deal with changing the default for files later so we can detect this case but it also feels kind of wrong to just avoid raising the warning if the file is ..

Source:

ruff/crates/ruff_cli/src/lib.rs

Lines 87 to 95 in 7b4b004

if stdin_filename.is_some() {

if let Some(file) = files.iter().find(|file| file.as_path() != Path::new("-")) {

warn_user_once!(

"Ignoring file {} in favor of standard input.",

file.display()

);

}

return true;

}

:/ bde7712

zanieb · 2023-11-20T17:59:27Z

docs/configuration.md

+!!! warning
+    Paths provided to `include` _must_ match files. For example, `include = ["src"]` will fail since it
+matches a directory.


This really feels like it should not be the case. We should join our default inclusions to directories. I'd prefer to tackle that separately though.

What would be examples of use-cases in which one would want to include an entire directory?

As in #3970 e.g. ruff check with include = ["src", "tests"]

github-actions · 2023-11-20T18:28:20Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

konstin

🎉

charliermarsh · 2023-11-21T01:24:13Z

crates/ruff_cli/resources/test/fixtures/include-test/pyproject.toml

@@ -0,0 +1,2 @@
+[tool.ruff]
+include = ["a.py", "subdirectory/c.py"]


Nit: do you mind adding a newline here, so it isn't automatically added next time someone edits the file?

charliermarsh · 2023-11-21T01:26:22Z

crates/ruff_cli/src/lib.rs

+        if let Some(file) = files
+            .iter()
+            // We allow `.` in addition to `-` since it is the default file selector
+            .find(|file| file.as_path() != Path::new("-") && file.as_path() != Path::new("."))


Does this mean we now allow cat foo.py | ruff check --stdin-filename="foo.py" ., without warning? That feels like a bug. I'm wondering if we should instead represent this default as the empty slice? I.e., at the relevant sites, if files is empty, use the current working directory?

Yeah... I brought this up at #8791 (comment)

I don't like either solution honestly. . as the Clap default is nice for the CLI help menu and such and it seems like a hassle to make sure the behavior is correct in the call sites. It does feel bad to omit the warning though so I guess I'll do the extra work.

Ahh sorry, I missed your comment! I defer to you as the author, I see there are tradeoffs here.

Okay kind of awkward but still fine e92000d

charliermarsh · 2023-11-21T01:28:46Z

docs/configuration.md

+!!! warning
+    Paths provided to `include` _must_ match files. For example, `include = ["src"]` will fail since it
+matches a directory.


What would be examples of use-cases in which one would want to include an entire directory?

…in filename

Update ruff check and ruff format to default to the current direc…

8611c67

…tory

zanieb added the cli Related to the command-line interface label Nov 20, 2023

Add test coverage

19736f1

zanieb commented Nov 20, 2023

View reviewed changes

Avoid warning when stdin filename is provided

bde7712

zanieb marked this pull request as ready for review November 20, 2023 17:22

zanieb added 7 commits November 20, 2023 11:25

Regenerate schema

af7ef6d

Add documentation

74ed369

Add test case for child working directory

62d688d

Move docs to configuration

e0b8307

Add note to ruff check docs

6f9518c

Add note to ruff format docs

42d6eff

Docs touchups

3e1ce2d

zanieb commented Nov 20, 2023

View reviewed changes

zanieb added 2 commits November 20, 2023 12:10

Fix path determinisim

1ad6968

oopsie

1260157

konstin approved these changes Nov 20, 2023

View reviewed changes

Attempt to fix Windows tests

fab5932

zanieb force-pushed the zanie/default branch from db2e157 to fab5932 Compare November 20, 2023 20:54

zanieb requested a review from charliermarsh November 20, 2023 21:13

charliermarsh reviewed Nov 21, 2023

View reviewed changes

Move default to ensure warnings show when user provides confusing std…

df58ad6

…in filename

zanieb force-pushed the zanie/default branch from e92000d to df58ad6 Compare November 21, 2023 17:10

zanieb merged commit d9151b1 into main Nov 21, 2023
17 checks passed

zanieb deleted the zanie/default branch November 21, 2023 17:34

miccal mentioned this pull request Dec 5, 2023

ruff 0.1.7 Homebrew/homebrew-core#156391

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `ruff check` and `ruff format` to default to the current directory #8791

Update `ruff check` and `ruff format` to default to the current directory #8791

zanieb commented Nov 20, 2023 •

edited

Loading

zanieb commented Nov 20, 2023

zanieb Nov 20, 2023

zanieb Nov 20, 2023

zanieb Nov 20, 2023

charliermarsh Nov 21, 2023

zanieb Nov 21, 2023

github-actions bot commented Nov 20, 2023 •

edited

Loading

konstin left a comment

charliermarsh Nov 21, 2023

charliermarsh Nov 21, 2023

zanieb Nov 21, 2023

charliermarsh Nov 21, 2023

zanieb Nov 21, 2023 •

edited

Loading

charliermarsh Nov 21, 2023

		----- stderr -----
		warning: Ignoring file . in favor of standard input.

	if stdin_filename.is_some() {
	if let Some(file) = files.iter().find(\|file\| file.as_path() != Path::new("-")) {
	warn_user_once!(
	"Ignoring file {} in favor of standard input.",
	file.display()
	);
	}
	return true;
	}

		@@ -0,0 +1,2 @@
		[tool.ruff]
		include = ["a.py", "subdirectory/c.py"]

Update ruff check and ruff format to default to the current directory #8791

Update ruff check and ruff format to default to the current directory #8791

Conversation

zanieb commented Nov 20, 2023 • edited Loading

zanieb commented Nov 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 20, 2023 • edited Loading

ruff-ecosystem results

Linter (stable)

Linter (preview)

konstin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zanieb Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Update `ruff check` and `ruff format` to default to the current directory #8791

Update `ruff check` and `ruff format` to default to the current directory #8791

zanieb commented Nov 20, 2023 •

edited

Loading

github-actions bot commented Nov 20, 2023 •

edited

Loading

`ruff-ecosystem` results

zanieb Nov 21, 2023 •

edited

Loading