Skip to content

Show changed files when running under --check#7788

Merged
charliermarsh merged 1 commit intomainfrom
charlie/check
Oct 3, 2023
Merged

Show changed files when running under --check#7788
charliermarsh merged 1 commit intomainfrom
charlie/check

Conversation

@charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Oct 3, 2023 •
edited
Loading

Summary

We now list each changed file when running with --check.

Closes #7782.

Test Plan

❯ cargo run -p ruff_cli -- format foo.py --check
   Compiling ruff_cli v0.0.292 (/Users/crmarsh/workspace/ruff/crates/ruff_cli)
rgo +    Finished dev [unoptimized + debuginfo] target(s) in 1.41s
     Running `target/debug/ruff format foo.py --check`
warning: `ruff format` is a work-in-progress, subject to change at any time, and intended only for experimentation.
Would reformat: foo.py
1 file would be reformatted

@zanieb
Copy link
Member

zanieb commented Oct 3, 2023

What's this look like for multiple files?

@charliermarsh
Copy link
Member Author

charliermarsh commented Oct 3, 2023

It shows one line (like the above) per file:

File is unformatted: bar.py
File is unformatted: foo.py
2 files would be reformatted

(The filenames are bolded.)

@zanieb
Copy link
Member

zanieb commented Oct 3, 2023

And we don't mention the files that are correctly formatted? Seems reasonable to me. Should we say "Would reformat: foo.py" instead?

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023 •
edited
Loading

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

@charliermarsh
Copy link
Member Author

charliermarsh commented Oct 3, 2023

Changed!

zanieb
zanieb approved these changes Oct 3, 2023
View reviewed changes
Copy link
Member

@zanieb zanieb left a comment

Choose a reason for hiding this comment

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

LGTM; why the refactor?

@charliermarsh
Copy link
Member Author

charliermarsh commented Oct 3, 2023

Which part? Overall, I needed to store the results with paths so that we could report on individual paths.

The impl<P: AsRef<Path>> From<P> change was necessary to allow passing &path to the SourceKind::from calls rather than path.as_path() -- the .as_path() shouldn't be necessary.

@charliermarsh charliermarsh enabled auto-merge (squash) October 3, 2023 18:45
@charliermarsh charliermarsh merged commit f71c80a into main Oct 3, 2023
@charliermarsh charliermarsh deleted the charlie/check branch October 3, 2023 18:50
@p-linnane p-linnane mentioned this pull request Oct 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zanieb zanieb zanieb approved these changes

@konstin konstin Awaiting requested review from konstin

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Formatter: include filenames in ruff format --check

2 participants

@charliermarsh @zanieb