Extend formatter script to run on all staged files, merge add/strip action

[tcm-marcel]

This is especially useful when using the new pre-commit hook that prevents me from committing.

• add -f option to formatter that applies the selected action to all staged files (git)
• adapt pre-commit hook to only check stages files

(since -a, -s and -c were already taken, I just picked any other letter that somehow looks like "staged files" when looking from far away)

Testing: I run Ubuntu, can someone test this on a Mac?

[tcm-marcel]

There was a typo in the commit message and in the title - of course it is meant to run on all staged files, not the other way round.

[coveralls]

Coverage increased (+0.01%) to 75.392% when pulling a6c6384 on tcm-marcel:pr/formatter into bbcf739 on cmu-db:master.

[coveralls]

Coverage decreased (-0.004%) to 75.377% when pulling a6c6384 on tcm-marcel:pr/formatter into bbcf739 on cmu-db:master.

[coveralls]

Coverage remained the same at 75.427% when pulling 770c42d on tcm-marcel:pr/formatter into 03dc205 on cmu-db:master.

[schedutron]

Perhaps a lame question, but how exactly do I test this? I'm on a Mac.

[tcm-marcel]

@schedutron

1. Install the pre-commit hook
2. Edit some source or header files in peloton to obviously violate the code style (e.g. lines too long)
3. Add some (not all) of them to the staging area (git add) and try to commit. The pre-commit hook should prevent this.
4. run ./script/formatting/formatter.py -c -f - the staged files should be formatted now and ready to commit, the unstaged ones should remain unchanged

Thanks!

[schedutron]

I installed the hook and edited few source files (made some of the lines longer that 80 chars), staged them and committed some of them, but the hook didn't stop me. Rather, I got this commit output:

[pr/formatter 8c9d66ad4] Check formatter
 1 file changed, 1 insertion(+), 3 deletions(-)

EDIT: Turns out there was a problem with my local version of clang-format (I had 3.7, while the validator and formatter scripts use 3.6). I checked with having 3.7 in the scripts and the hook works great as expected!

And, is there a way to only run the hook if there are staged files? I tried to commit without staging and the hook still ran.

[tcm-marcel]

Tanks for testing!

And, is there a way to only run the hook if there are staged files? I tried to commit without staging and the hook still ran.

The commit hook only checks staged files. But there is no way to temporary disable it, if that's what you mean.

[schedutron]

@tcm-marcel Maybe the formatter can be edited to only run when there are nonzero files passed to the --files parameter?

[tcm-marcel]

@schedutron was right, I changed the hook to only run if actually .cpp/.h files are staged. If committing a non-source file, the validator would run with empty parameters otherwise.

Referring to last team meeting I also merged the add/strip actions to a new update action, that updates the header or adds a new one if necessary.

[coveralls]

Coverage increased (+0.5%) to 75.947% when pulling 812988e on tcm-marcel:pr/formatter into 03dc205 on cmu-db:master.

[pmenon]

I took a quick look at this. Looks good!

Also, since you're in this part anyways, can you add a pre-check in source_validator.py that looks for clang-format. It doesn't have to be 3.6, but it should print a nice error message, rather than a stack trace it currently does.

[pmenon]

You can remove this echo since all scanned files are printed in the source_validator.py script.

[tcm-marcel]

oops, the echo is left from debugging and the exit $? should of course stay there

[schedutron]

@pmenon So you're suggesting that @tcm-marcel should add a try-except block to check for the clang-format command, right?

[coveralls]

Coverage increased (+0.01%) to 75.93% when pulling b3011b0 on tcm-marcel:pr/formatter into b9f1e43 on cmu-db:master.

[coveralls]

Coverage decreased (-0.02%) to 75.841% when pulling 0dad892 on tcm-marcel:pr/formatter into c4971f3 on cmu-db:master.