On 13/08/17 18:33, Junio C Hamano wrote:
> Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> writes:
>> 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}.."
Yes, I had already tried the following, which maybe more workable,
but it is only lightly tested. (we may want to create our own version
of checkpatch.pl, which is written specifically for the kernel ...)
$ git diff
diff --git a/Makefile b/Makefile
index 461c845d3..a25028e68 100644
--- a/Makefile
+++ b/Makefile
@@ -2440,6 +2440,15 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
.PHONY: sparse $(SP_OBJ)
sparse: $(SP_OBJ)
+STYLE_REVS = HEAD
+STYLE_IGNORES = NEW_TYPEDEFS,INLINE
+
+.PHONY: style
+style:
+ @git diff $(STYLE_REVS) -- '*.[ch]' | \
+ checkpatch.pl -q --no-tree --show-types \
+ --ignore=$(STYLE_IGNORES) --patch - 2>/dev/null || true
+
check: common-cmds.h
@if sparse; \
then \
$ make style # I don't have any changes to *.[ch] files!
$ make STYLE_REVS=HEAD~3 style
WARNING:LONG_LINE: line over 80 characters
#181: FILE: git.c:320:
+ if (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY) &&
WARNING:LONG_LINE: line over 80 characters
#221: FILE: revision.c:2317:
+ if (revs->def && !revs->pending.nr && !revs->rev_input_given && !got_rev_arg) {
total: 0 errors, 2 warnings, 206 lines checked
$
I suspect this closer to what you had in mind. ;-)
(although the --ignore list may need adding to).
ATB,
Ramsay Jones
