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

feat: adds --test flag in yr fmt #72

Closed
wants to merge 2 commits into from

Conversation

shellcromancer
Copy link

Adds -t/--test flag in yr fmt which returns an error code if formatting made changes on the file.

This feature exists in a few other language formatting tools (jsonnet, terraform, gofmt) which is useful as part of stylistic CI/CD checks

Adds -t/--test flag in yr fmt which returns an error code
if formatting made changes on the file.
Copy link

google-cla bot commented Jan 13, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@shellcromancer
Copy link
Author

Signed the Google CLA, should be ready for a maintainer review, thanks! 👍

let output = formatted.into_inner();

if *test.unwrap() && input != output {
process::exit(3)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not process::exit(2)? Exit code 2 is not used for any purpose yet.

I'm also wondering if it wouldn't be better to return the path of the files that requires changes, as gofmt -l does. This provides more information to the user. If you call yr fm with multiple files, you may want to know which files are changed and which don't.

Copy link
Author

@shellcromancer shellcromancer Mar 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for taking a look! I updated this to use exit code 2, and output a list of files that require changes. Here's an image of runs with and without changes required.

image

@plusvic
Copy link
Member

plusvic commented Sep 9, 2024

In 4b08899 I made multiple changes to the fmt command. Now it always exits with error code 0 if files didn't change or 1 if otherwise.

@plusvic plusvic closed this Sep 9, 2024
@shellcromancer
Copy link
Author

Makes sense, thanks for including the feature in your changes. 👍

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