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

Cargo fmt should not run on a dirty repository by default #7509

Closed
dhardy opened this issue Oct 14, 2019 · 3 comments
Closed

Cargo fmt should not run on a dirty repository by default #7509

dhardy opened this issue Oct 14, 2019 · 3 comments
Labels
C-bug Category: bug

Comments

@dhardy
Copy link

dhardy commented Oct 14, 2019

Problem

  1. Hack on code, don't create a commit yet
  2. Run cargo fmt; find formatting changes to old code + new code all mixed up

(Yes, I am fully aware that some people frequently do this on purpose. More than once I have received PRs with commits mixing up significant amounts of reformatting and new code.)

Possible Solution(s)
cargo fix, cargo publish etc. refuse to run on a dirty repository by default (but allow override with --allow-dirty). Please do the same for cargo fmt.

The above is only really a problem when significant amounts of old code were not previously formatted, so possibly Cargo+rustfmt could try to detect this first.

An alternative solution might be to make it possible to undo cargo fmt, but I don't see a clean way to do this.

@dhardy dhardy added the C-bug Category: bug label Oct 14, 2019
@ehuss
Copy link
Contributor

ehuss commented Oct 14, 2019

Would you mind moving this to https://github.com/rust-lang/rustfmt? That's where cargo fmt lives.

Also, I think rust-lang/rustfmt#1324 might be close to what you want. I don't think aborting on a dirty repo is the right behavior, because that's the scenario it runs most often during development (that I've seen).

Some things that can help mitigate the issue:

  • Enforce rustfmt on CI. This will ensure everything stays formatted all the time. Changes due to rustfmt versions are rare.
  • Run rustfmt from an IDE/editor, where you can rustfmt only a region or single file. This also has the benefit of being easily undoable.

@dhardy
Copy link
Author

dhardy commented Oct 14, 2019

rust-lang/rustfmt#1324 would help. But your mitigations don't:

  • Enforce rustfmt on CI: not everyone wants to do this
  • Run rustfmt from an IDE/editor: if your editor does this, then great, my proposal won't inconvenience you. But it doesn't fix my use case at all (which was in fact not intending to format the project but mistakenly calling the wrong command).

@dhardy
Copy link
Author

dhardy commented Oct 14, 2019

Moved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

2 participants