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

Enable Rustfmt & Clippy for cg_gcc #457

Closed
2 tasks done
tempdragon opened this issue Feb 28, 2024 · 11 comments
Closed
2 tasks done

Enable Rustfmt & Clippy for cg_gcc #457

tempdragon opened this issue Feb 28, 2024 · 11 comments

Comments

@tempdragon
Copy link
Contributor

tempdragon commented Feb 28, 2024

It takes a long time to manually adjust the code format and style, so I advice to enable the Rustfmt for this project to avoid potential bad styling.
As for clippy, it may be a good idea to reduce the poorly behaved code in Rust to improve code quality.

Tasks

@tempdragon tempdragon changed the title Enable Rustfmt for cg_gcc Enable Rustfmt & Clippy for cg_gcc Feb 28, 2024
@antoyo
Copy link
Contributor

antoyo commented Feb 28, 2024

Yes, good idea, I'll accept a PR doing that. Ideally, we should use the same rustfmt config used by the rust project, if any.

@tempdragon
Copy link
Contributor Author

What about using your rustfmt?(Since you contribute the most code).
It seems that you can simply add your rustfmt.toml there? Or maybe I can upload my but it will just be a mare copy of the default one.

@antoyo
Copy link
Contributor

antoyo commented Feb 28, 2024

I tried once to get the formatting I wanted with rustfmt, but I was unable to because some settings were missing.
Using a different config than rustc will still cause issues to some people because VS Code sometimes ignore the config, so it seems best to use the same config as the Rust project.
It seems we'd only need to add a few lines to .rustfmt.toml to use the same config as the Rust project: https://github.com/rust-lang/rust/blob/master/rustfmt.toml#L2-L4

@tempdragon
Copy link
Contributor Author

I once noticed that in some cases, Rustfmt may behave differently on different machines. A friend of mine used VSCode and our rustfmt seemed to behaved differently, causing our code to be reformatted over and over again. Still not sure about why and how.

@antoyo
Copy link
Contributor

antoyo commented Feb 28, 2024

Apparently, one cause is because formatting a selection of code only sends the code to be formatted to rustfmt, not the filename, so it cannot find the config.

@tempdragon
Copy link
Contributor Author

tempdragon commented Feb 28, 2024

By the way, have you switched back to vim? Is VSCode rustfmt-related behavior still a problem for you?

@antoyo
Copy link
Contributor

antoyo commented Feb 28, 2024

Oh, I never used VS Code. It's some people contributing to cg_gcc via the Rust repo that have this issue.

I've been using neovim (and vim before) for many years.

@antoyo
Copy link
Contributor

antoyo commented Feb 28, 2024

I formatted the code in #458.

@tempdragon
Copy link
Contributor Author

tempdragon commented Feb 29, 2024

By the way, is adding a full rustfmt specification useful?
Customized Conf

@tgross35
Copy link
Contributor

tgross35 commented Mar 1, 2024

You probably want to use the same file as upstream https://github.com/rust-lang/rust/blob/6f435eb0eb2926cdb6640b3382b9e3e21ef05f07/rustfmt.toml, the ignore section isn't needed.

Fyi, if these wind up being large changes it's often a good idea to crosscheck. Two people run the same rustfmt commands then git ls-files -s . | git hash-object --stdin to make sure their hashes line up.

Adding the hash of the formatting commit to .git-blame-ignore-revs gets the noise out of blame views (automatic on GH, need to pass --ignore-revs-file via CLI)

@antoyo
Copy link
Contributor

antoyo commented Mar 16, 2024

Completed by #469.

@antoyo antoyo closed this as completed Mar 16, 2024
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

No branches or pull requests

3 participants