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

Only write files on change. #96

Merged
merged 1 commit into from
Dec 15, 2024

Conversation

djacu
Copy link
Contributor

@djacu djacu commented Dec 15, 2024

When formatting files, genemichaels always writes the formatted file to disk which modifies the file timestamp. This is problematic for tools that detect whether a file has been reformatted or not.

This commit changes the behavior by comparing the source against the formatted output and only writing to disk if they are different.

Fixes #95


I agree that when the request is merged I assign the copyright of the request to the repository owner.

When formatting files, genemichaels always writes the formatted file to
disk which modifies the file timestamp. This is problematic for tools
that detect whether a file has been reformatted or not.

This commit changes the behavior by comparing the source against the
formatted output and only writing to disk if they are different.

See: andrewbaxter#95
djacu added a commit to djacu/nixpkgs that referenced this pull request Dec 15, 2024
When formatting rust files with `genemichaels`, the timestamp of the
file is always modified whether or not the file was changed. This is
problematic for detecting whether a file has been reformatted or not,
such as when using tools such as `pre-commit`, `treefmt`, and
`treefmt-nix`.

See the following:
andrewbaxter/genemichaels#95
andrewbaxter/genemichaels#96
numtide/treefmt-nix#277
@andrewbaxter
Copy link
Owner

Oh fantastic! Yeah, we shouldn't have been always writing. Thanks for this!

@andrewbaxter
Copy link
Owner

Ah, that test was something I had for debug dumping output, I should get rid of that.

@andrewbaxter andrewbaxter merged commit 3c58f09 into andrewbaxter:master Dec 15, 2024
3 of 4 checks passed
@andrewbaxter
Copy link
Owner

Released as 0.5.12.

@djacu djacu deleted the djacu/only-write-on-change branch December 15, 2024 08:04
@djacu
Copy link
Contributor Author

djacu commented Dec 15, 2024

Ah, that test was something I had for debug dumping output, I should get rid of that.

Do you mean this test that has been failing since 0.5.7?
https://github.com/andrewbaxter/genemichaels/actions/runs/12336988263/job/34430124529

I was going to ask about that since I'd like to add the latest to nixpkgs but stopped at 0.5.7 since the tests after that started failing. See NixOS/nixpkgs#363473

@andrewbaxter
Copy link
Owner

Oh yeah, that one! Sorry, didn't realize it was causing problems externally. I'll get rid of it now.

@andrewbaxter
Copy link
Owner

Okay as of 0.5.13 that's been removed and the tests are all passing.

@djacu
Copy link
Contributor Author

djacu commented Dec 15, 2024

Not a problem. I thought it was part of a developing test for the new macros crate.

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.

genemichaels always updates the timestamp of a file
2 participants