Skip to content

Removed rustfmt_skip guards from all files. #71

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

Merged
merged 1 commit into from
Sep 19, 2020

Conversation

Zenithsiz
Copy link
Contributor

This PR removes all #![cfg_attr(rustfmt, rustfmt_skip)] from files so that files are automatically formatted by IDEs and using cargo fmt when working on this crate.
It might also be worth considering adding a github CI action to make sure the crate is fully formatted, this can be done with cargo fmt --verbose -- --check.

The reason for this PR is to make it easier for people to contribute without worrying about formatting. Recently I created a local branch to make a PR, but since I use hard tabs, I had to reformat my code manually every few lines, which became annoying quickly.
I believe it is standard for rust projects to use rustfmt to format their code. This can also be customized with a rustfmt.toml file if you wish to enforce a specific format.
It is also possible to put a #[rustfmt::skip] attribute on functions / blocks that shouldn't be formatted for some reason (such as the full ascii table in AsciiChar::new) to manually avoid this.

Added a `rustfmt::skip` to the char table in `AsciiChar::new`.
@tomprogrammer
Copy link
Owner

I fully agree that the code should be formatted by default. I'll add an editorconfig file, editors that understand the file will use spaces for this project even if rustfmt is not used.

@tomprogrammer tomprogrammer merged commit c9aca47 into tomprogrammer:master Sep 19, 2020
@Zenithsiz Zenithsiz deleted the rustfmt branch September 19, 2020 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants