Re: [PATCH v2 2/2] Makefile: add style build rule

[Jeff King <peff@peff.net>]

On Mon, Aug 14, 2017 at 03:28:50PM -0700, Stefan Beller wrote:

> >> +.PHONY: style
> >> +style:
> >> +     git clang-format --style file --diff --extensions c,h
> >
> > Did we get "git clang-format" subcommand, or is "s/git //" implied
> > somewhere?
> 
> You need to have clang-format installed (debian/Ubuntu package, or
> however it is named in your distribution), which provides this command
> for us.

Sadly it is called git-clang-format-3.8, git-clang-format-5.0, etc, in
the Debian packages. I think we need a CLANG_FORMAT variable that can be
overridden.

I am surprised that there's no base "git-clang-format" symlink for the
default version. There is for "clang-format" and "clang" themselves. So
arguably this is a bug in the Debian packaging.

I suspect the "-p" version is going to be the one people invoke the most
often. Should it take the coveted "make style" slot, and the diff get
pushed off to another target?

I was also confused at first that the "-p" version requires you to stage
the changes first. I don't know if we can make that less confusing via a
"make style". Or if it's just something people would get used to. But
sadly it makes the command not-quite orthogonal to "make test" in the
workflow. You can't "make style && make test && git add -p".  You have
to add first, then check style, then you'd want to test that result to
make sure it didn't change the meaning of the code.

-Peff