Re: [RFC] clang-format: outline the git project's coding style

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 13, 2017, 17:33 UTC
Message-ID: <xmqqy3qnwexy.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <26bebb5b-857f-1501-5139-a5513d9875ec@ramsayjones.plus.com>

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

Show 31 quoted lines

> As a start, how about something like this:
>
> -- >8 --
> $ git diff
> diff --git a/Makefile b/Makefile
> index 461c845d3..7555def45 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2440,6 +2440,18 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
>  .PHONY: sparse $(SP_OBJ)
>  sparse: $(SP_OBJ)
>  
> +ST_C = $(patsubst %.o,%.stc,$(C_OBJ))
> +ST_H = $(patsubst %.h,%.sth,$(LIB_H))
> +
> +$(ST_C): %.stc: %.c FORCE
> +       checkpatch.pl --no-tree --show-types --ignore=NEW_TYPEDEFS,INLINE -f $<
> +
> +$(ST_H): %.sth: %.h FORCE
> +       checkpatch.pl --no-tree --show-types --ignore=NEW_TYPEDEFS,INLINE -f $<
> +
> +.PHONY: style $(ST_C) $(ST_H)
> +style: $(ST_C) $(ST_H)
> +
>  check: common-cmds.h
>         @if sparse; \
>         then \
> $ 
> -- >8 --
> ...
> Hmm, on reflection, it may be a bit too crude! :-D

As you already saw in the output from this, I think this is a good illustration that shows why we want an incremental tool that works on the changes, not on full file contents. Contributors who want their changes accepted and want to help the review process by avoiding trivial coding style violations in their patches should not have to find _their_ piece from an output about the whole file contents, most of which is likely to have been inherited from the original. They are not working on Git to produce unnecessary code churn whose only purpose is to make existing and otherwise dormant code conform to the style tool's liking. That's not their focus.

IOW I was expecting something that works on the output from "git diff HEAD" or "git format-patch --stdout @{u}.."

Unless you were somehow envisioning that having a baseline line this, and then take another full dump after their patch and comparing the two, would make a good foundation for that incremental checker, that is. I am not sure if that would be a workable approach myself, though.

Thanks.