Repository navigation
Update ruff check and ruff format to default to the current directory - #8791
Conversation
|
I'm looking into the behavior this exhibits with |
| ----- stderr ----- | ||
| warning: Ignoring file . in favor of standard input. |
There was a problem hiding this comment.
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
| !!! warning | ||
| Paths provided to `include` _must_ match files. For example, `include = ["src"]` will fail since it | ||
| matches a directory. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
What would be examples of use-cases in which one would want to include an entire directory?
There was a problem hiding this comment.
As in #3970 e.g. ruff check with include = ["src", "tests"]
|
db2e157 to
fab5932
Compare
| @@ -0,0 +1,2 @@ | |||
| [tool.ruff] | |||
| include = ["a.py", "subdirectory/c.py"] No newline at end of file | |||
There was a problem hiding this comment.
Nit: do you mind adding a newline here, so it isn't automatically added next time someone edits the file?
| 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(".")) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ahh sorry, I missed your comment! I defer to you as the author, I see there are tradeoffs here.
| !!! warning | ||
| Paths provided to `include` _must_ match files. For example, `include = ["src"]` will fail since it | ||
| matches a directory. |
There was a problem hiding this comment.
What would be examples of use-cases in which one would want to include an entire directory?
e92000d to
df58ad6
Compare
Closes #7347
Closes #3970 via use of
includeWe 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.