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

Stabilize disable_all_formatting #3481

Closed

Conversation

rchaser53
Copy link
Contributor

related: #3197, #3388

This options is a very simple and no bugs are reported.
So I think this options can be stabilized only tests addition.

@topecongiro
Copy link
Contributor

I would rather deprecate disable_all_formatting rather than stabilize it as we have ignore which is more flexible.

@rchaser53
Copy link
Contributor Author

rchaser53 commented Mar 30, 2019

I also think ignore is better than disable_all_formatting.
A good disable_all_formatting's points better than ignore is that can ignore every file.
But we can do same thing without disable_all_formatting option. Do not use rustfmt.
If we want to switch to use or not dynamically, just use shell.

Should I change the PR Stabilize disable_all_formatting to Deprecate disable_all_formatting?

cc: @nrc @scampi

@topecongiro
Copy link
Contributor

Originally disable_all_formatting was added to work around a not-so-good integration between an editor and rustfmt (cc #1297). I think the option is useful in that respect.

Other than replacing disable_all_formatting, ignore is useful when you have submodules in your crate that you do not want rustfmt to touch. A canonical example https://github.com/rust-lang/rust.

Should I change the PR Stabilize disable_all_formatting to Deprecate disable_all_formatting?

Before deprecating disable_all_formatting, I prefer to stabilize ignore first. It is blocked by #3413, but currently I do not have time to work on it. @rchaser53 If you want to push this further, it would be awesome if you could take it over and implement the feature (and add some tests, if possible)!

@rchaser53
Copy link
Contributor Author

I understand the context of disable_all_formatting.
Ok. I take #3413 over.

@scampi scampi added the blocked Blocked on rustc, an RFC, etc. label Mar 31, 2019
@topecongiro
Copy link
Contributor

@rchaser53 Thanks!

@topecongiro
Copy link
Contributor

Closing this as disable_all_formatting should be deprecated in favor of ignore (see #3481 (comment)). @rchaser53 thank you for all the work (including many other PRs to rustfmt)!

@rchaser53 rchaser53 deleted the stabilize-disable_all_formatting branch June 25, 2019 23:56
@Stargateur
Copy link

Warning: can't set ignore = IgnoreList { path_set: {"/"}, rustfmt_toml_path: "" }, unstable features are only available in nightly channel.

This should be an error - -, this was really unexpected that it format anyway !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Blocked on rustc, an RFC, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants