Junio C Hamano <gitster@xxxxxxxxx> writes:
> Karthik Nayak <karthik.188@xxxxxxxxx> 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?
>
Making syntax changes just so the rule works was something I did
consider, but I avoided it mostly because the CI only applies to the
changes made and not pre-existing files.
This also allows us to apply the boy scout rule and cleanup as we go.
> 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.
>
Yes exactly, the usage of 'git-clang-format' allows us to only check for
lines changed.
> 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/
Attachment:
signature.asc
Description: PGP signature
