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

Update format.rs to display correct message for already formatted files #9153

Merged
merged 4 commits into from
Dec 18, 2023

Conversation

VictorGob
Copy link
Contributor

Summary

New messages for "format" mode.
Fixes #9132

Test Plan

I ran the tests specified in CONTRIBUTING.md

cargo run -p ruff_cli -- check /path/to/some_files.py --no-cache
cargo run -p ruff_cli -- format --check /path/to/some_files.py --no-cache

cargo clippy --workspace --all-targets --all-features -- -D warnings
RUFF_UPDATE_SCHEMA=1 cargo test
pre-commit run --all-files --show-diff-on-failure

Note: In case no files are detected, either correctly formatted, changed, or unchanged, it does not display a message. Wouldn't it be better to show some message in this case?

Copy link
Contributor

github-actions bot commented Dec 16, 2023

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@@ -515,7 +515,7 @@ impl<'a> FormatResults<'a> {
if changed > 0 && unchanged > 0 {
writeln!(
f,
"{} file{} {}, {} file{} left unchanged",
"{} file{} {}, {} file{} already formatted",
Copy link
Member

Choose a reason for hiding this comment

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

I think we want to apply this change only in the event that we have FormatMode::Check | FormatMode::Diff -- do you mind tweaking, similar to "would be reformatted" below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't understand: Check and Diff here are like the lower branch of the 'if'

Copy link
Member

Choose a reason for hiding this comment

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

@VictorGob - I pushed an update to illustrate what I meant. What do you think of this approach?

Copy link
Contributor Author

@VictorGob VictorGob Dec 18, 2023

Choose a reason for hiding this comment

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

Chef kiss 👌

@charliermarsh charliermarsh merged commit 6feea86 into astral-sh:main Dec 18, 2023
17 checks passed
@charliermarsh charliermarsh added the cli Related to the command-line interface label Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command-line interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ruff format --check outputs "files left unchanged", which is slightly confusing
2 participants