Add formatting targets to the makefile

[pablogsal]

As requested by @lysnikolaou , add one target that allows formatting pegen/pegen.c (requires
clang-format and clang-tidy) available on the system and another target
that formats everything (Python and C files).

Feel free to suggest tweaks to the C file or if you want additional files to be formatted.

[gvanrossum]

How do I get clang-today and clang-format?

[gvanrossum]

The difference in naming style for this target from ‘black’ somehow bothers me.

[pablogsal]

Should I rename black to format-python and format-pegen to format-c?

[gvanrossum]

Sounds good.

[pablogsal]

How do I get clang-tidy and clang-format?

Not a heavy MacOS user but after a bit of research seems that:

• Clang format can easily be installed by brew install clang-format
• It seems that clang-tidy is not on brew so the easiest way is to install LLVM and symlink the tools (yeah, I know, is a bit overkill):

brew install llvm
ln -s "$(brew --prefix llvm)/bin/clang-format" "/usr/local/bin/clang-format"
ln -s "$(brew --prefix llvm)/bin/clang-tidy" "/usr/local/bin/clang-tidy"

To be honest, the original reason I didn't initially commit this to the makefile is that I find these tools a bit heavyweight and I am not an expert with them so I will understand if we end deciding to use only clang-format (seems that is the easiest to get) or to not add any new target at all.

[gvanrossum]

How do I get clang-today and clang-format?

Not a heavy MacOS user but after a bit of research seems that:

* Clang format can easily be installed by `brew install clang-format`

* It seems that `clang-tidy` is not on brew so the easiest way is to install LLVM and symlink the tools (yeah, I know, is a bit overkill):

brew install llvm
ln -s "$(brew --prefix llvm)/bin/clang-format" "/usr/local/bin/clang-format"
ln -s "$(brew --prefix llvm)/bin/clang-tidy" "/usr/local/bin/clang-tidy"

To be honest, the original reason I didn't initially commit this to the makefile is that I find these tools a bit heavyweight and I am not an expert with them so I will understand if we end deciding to use only clang-format (seems that is the easiest to get) or to not add any new target at all.

Maybe we should only use clang-format then? (Honestly I don't know which one does what -- why do we need both?) And it would be nice if instructions for installing it were in a comment in the Makefile at least (so someone for whom it doesn't work might find them upon inspecting the source of the Makefile).

[pablogsal]

why do we need both

It seems that only clang-tidy can add the braces around statements, I was unable to make clang-format do that :(

And it would be nice if instructions for installing it were in a comment in the Makefile at least (so someone for whom it doesn't work might find them upon inspecting the source of the Makefile).

Agreed, will update the PR soon.

[gvanrossum]

It seems that only clang-tidy can add the braces around statements, I was unable to make clang-format do that :(

I'm not adamant about braces everywhere, despite what PEP 7 says. Maybe you can have a separate rule that runs clang-tidy but which isn't depended on by format.

Separately, maybe we can run make format from Travis-CI and somehow fail if any file had to be formatted? Just like we run make mypy. (Though I think checking that nothing was formatted is a bit trickier.)

[lysnikolaou]

@pablogsal Thanks for doing this!

Maybe you can have a separate rule that runs clang-tidy but which isn't depended on by format.

I agree, that this is the way to go, since clang-tidy is not that easy to install.

[gvanrossum]

Were you going to finish this?

[pablogsal]

Were you going to finish this?

I have rebased and addressed the feedback.

[gvanrossum]

Can you add comments explaining how to install clang-format and clang-tidy? I now know it's brew install clang-format for the former, so a single line would suffice there, but the other is more complicated, right?

[gvanrossum]

(Oh, add "On Mac" to those comments...)

[gvanrossum]

Great!