Avoid writing outfile unnecessarily for better compatibility with other tooling like pre-commit hooks

cheshirekow / cmake_format

Source code formatter for cmake listfiles.

GNU General Public License v3.0

958 stars 105 forks source link

Avoid writing outfile unnecessarily for better compatibility with other tooling like pre-commit hooks #118

Closed mtkennerly closed 5 years ago

mtkennerly commented 5 years ago

I use pre-commit hooks in my projects to run clang-format -i and some other commands automatically, and I'd like to incorporate cmake-format -i as well. However, cmake-format always overwrites the outfile, even if it's identical to the original, and that write is interpreted by the hook as a sign that there was a problem that had to be fixed, which means the hook fails and blocks the commit. This PR ensures that an in-place write is only performed if there was a change:

Move the outfile opening code to the end of main(), once we know if there have been changes.
Remove the on-disk temp file, since process_file() and such are now working with a dummy StringIO.

geieredgar commented 5 years ago

Hello, I've run into the same issues as you while trying to use cmake-format -i as pre-commit hook under Windows.

As far as I unterstand, pre-commit detects a modification of files by running a git diff command before and after running the hook and comparing the output of both commands. Always overwriting the file should therefore not be a problem.

For me, the hook failed because cmake-format replaced the windows-style line endings with unix-style line endings, which caused the after-hook git-diff command to print a warning message. To fix this, I tried to set the line-ending option of cmake-format to auto (either through a command line option or through the configuration file), but both failed because of what I believe is a bug inside cmake-format I tried to fix with this PR: #119

cheshirekow commented 5 years ago

@geiredgar, as a general rule, I suggest that you pick a line ending convention and enforce it. Rather than using --line-ending auto it sounds like you want to use --line-ending windows. auto is kind of a dumb hack... but I will look into why it isn't working.

cheshirekow commented 5 years ago

@mtkennerly the output is already buffered so there is no need to use StringIO to buffer it a second time. This is done in 0.5.4.