Re: [PATCH 1/3] Documentation: clarify indentation style for C preprocessor directives

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2429 bytes --]

On Wed, Jul 24, 2024 at 10:06:03PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> >> There has recently been some discussion around how C preprocessor
> >> directives shall be indented [1]. This discussion was settled towards
> >> indenting after the hash by two spaces [2]. Document it as such.
> >
> > It was settled to have space after and not before the hash, but I do
> > not recall ever agreeing to two spaces.  I prefer to increment by 1
> > for each level instead to keep the whole thing less distracting
> > while carrying meaningful information.
> 
> Using the indentation consistently is a good thing I do not object
> to.
> 
> Among our existing header and source files (excluding compat/ that
> is full of borrowed sources), scanning output from
> 
>     $ git grep -e '#  *' '*.[ch]' ':!compat/'
> 
> tells me that
> 
>  - builtin/gc.c (3 lines)
>  - git-compat-util.h (52 lines)
>  - trace.c (2 lines)
>  - wildmatch.c (6 lines)
> 
> use one space indent after '#' per level, while
> 
>  - hash.h (10 lines, inconsistently 2 uses 1 per level)
>  - thread-utils.c (8 lines)
> 
> use two space indent after '#' per level.
> 
> In compat/ directory, only compat/nedmalloc uses 2-space indent.
> There in the hierarchy are so many files we borrowed from GNU, whose
> coding style sticks to one-space indent.

Thanks for checking. I don't really mind whether we use one or two
spaces, as long as we are being consistent. I actually took the 2-space
indent from the comment for "IndentPPDirectives" in ".clang-format",
where it looks as if clang-format was using two spaces. So I took this
as being the official style we settled on.

One thing I noticed though is that clang-format is inconsistent with
either of those styles because it uses a tab character to indent nested
preprocessor directives. So the comment we have in that file is quite
misleading. We can fix this with the below change though. I'll add that
to this series.

Patrick

diff --git a/.clang-format b/.clang-format
index 16fd12253e..0e3606a8bb 100644
--- a/.clang-format
+++ b/.clang-format
@@ -105,6 +105,7 @@ IndentCaseLabels: false
 # #  include <foo>
 # #endif
 IndentPPDirectives: AfterHash
+PPIndentWidth: 1
 
 # Don't indent a function definition or declaration if it is wrapped after the
 # type

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]