Skip to content

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

Merged
charliermarsh merged 4 commits intoastral-sh:mainfrom
VictorGob:format_check_better_message
Dec 18, 2023
Merged

Update format.rs to display correct message for already formatted files#9153
charliermarsh merged 4 commits intoastral-sh:mainfrom
VictorGob:format_check_better_message

Conversation

@VictorGob
Copy link
Copy Markdown
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?

@github-actions
Copy link
Copy Markdown
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.

Comment thread crates/ruff_cli/src/commands/format.rs Outdated
writeln!(
f,
"{} file{} {}, {} file{} left unchanged",
"{} file{} {}, {} file{} already formatted",
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@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