Re: [PATCH 1/8] clang-format: indent preprocessor directives after hash

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> We do not have a rule around the indentation of preprocessor directives.
>> This was also discussed on the list [1], noting how there is often
>> inconsistency in the styling. While there was discussion, there was no
>> conclusion around what is the preferred style here. One style being
>> indenting after the hash:
>>
>>     #if FOO
>>     #  if BAR
>>     #    include <foo>
>>     #  endif
>>     #endif
>>
>> The other being before the hash:
>>
>>     #if FOO
>>       #if BAR
>>         #include <foo>
>>       #endif
>>     #endif
>>
>> Let's pick the former and add 'IndentPPDirectives: AfterHash' value to
>> our '.clang-format'. There is no clear reason to pick one over the
>> other, but it would definitely be nicer to be consistent.
>
> When I experimented with reindenting the CPP directives in
> <git-compat-util.h> [*], I think I saw an existing violation in an
> early part of the file.  Outside the borrowed code in compat/, we
> have these:
>
>     $ git grep -e '^[ 	]\{1,\}#' master -- ':!compat/' \*.[ch]
>     blame.c:	#define FINGERPRINT_FILE_THRESHOLD	10
>     block-sha1/sha1.c:  #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))
>     block-sha1/sha1.c:  #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
>     block-sha1/sha1.c:  #define setW(x, val) (W(x) = (val))
>     diff.h:	#define COLOR_MOVED_DEFAULT COLOR_MOVED_ZEBRA
>     diff.h:	#define COLOR_MOVED_MIN_ALNUM_COUNT 20
>     diff.h:	#define COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE (1<<5)
>     diff.h:	#define COLOR_MOVED_WS_ERROR (1<<0)
>     git-compat-util.h: #define GIT_GNUC_PREREQ(maj, min) 0
>     pkt-line.c:	#define hex(a) (hexchar[(a) & 15])
>     pkt-line.c:	#undef hex
>     sha1dc/sha1.c:	#define sha1_load(m, t, temp)  { temp = m[t]; }
>     sha1dc/sha1.c:	#define sha1_load(m, t, temp)  { temp = m[t]; sha1_bswap32(temp); }
>
> Should we clean them up before we start adding these rules to the
> file, especially if we plan to run the rules for stylistic check?
> Otherwise wouldn't we see noises coming from the existing lines that
> may dwarf the new ones, whose addition we want prevent?
>

> If we were to run the check in CI to help contributors, I would
> assume that you are arranging it to only complain about the lines
> touched by the commits they are contributing, not about the existing
> style violations.  This comment is not limited to the CPP directive
> indentation but any other style rules .clang-format defines.
>

> If we are not checking only for lines affected by commits being
> contributed, then perhaps we should exclude compat/ from these
> rules?
>
> Thanks for working on this.
>
> [Reference]
>
>  * https://lore.kernel.org/git/xmqqjzim197j.fsf@gitster.g/