[PATCH 0/4] clang-format: modify rules to reduce false-positives

[PATCH 0/4] clang-format: modify rules to reduce false-positives

[Karthik Nayak]

This series is in response to an email thread [1] around the usage of
'.clang-format' and its effectiveness.

The goal of the series is to improve the usage of 'clang-format' in the
repository. To do this we:
1. Reduce the number of false positives. Majority of which is due to
   line-wrapping. We remove the 'ColumnLimit' from 'clang-format'. This
   removes the responsibility of line-wrapping from 'clang-format' and puts
   it into the hands of the user.
2. Add a 120 character limit to the '.editorconfig' to ensure that
   editors wrap lines beyond that.
3. Add a rule to 'meson' to run 'git clang-format' by running 'meson
   compile style'.
4. Make the 'RemoveBracesLLVM' permanent by moving it to
   '.clang-format'.

With this, running `git clang-format` for the last 25 commits in master,
seems to produce much less false positives.

  diff --git a/builtin/diff.c b/builtin/diff.c
  index c6231edce4..246b81caa2 100644
  --- a/builtin/diff.c
  +++ b/builtin/diff.c
  @@ -30,14 +30,13 @@
   #define DIFF_NO_INDEX_IMPLICIT 2

   static const char builtin_diff_usage[] =
  -"git diff [<options>] [<commit>] [--] [<path>...]\n"
  -"   or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\n"
  -"   or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\n"
  -"   or: git diff [<options>] <commit>...<commit> [--] [<path>...]\n"
  -"   or: git diff [<options>] <blob> <blob>\n"
  -"   or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]"
  -"\n"
  -COMMON_DIFF_OPTIONS_HELP;
  +	"git diff [<options>] [<commit>] [--] [<path>...]\n"
  +	"   or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\n"
  +	"   or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\n"
  +	"   or: git diff [<options>] <commit>...<commit> [--] [<path>...]\n"
  +	"   or: git diff [<options>] <blob> <blob>\n"
  +	"   or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]"
  +	"\n" COMMON_DIFF_OPTIONS_HELP;

   static const char *blob_path(struct object_array_entry *entry)
   {
  diff --git a/builtin/stash.c b/builtin/stash.c
  index 7cd3ad8aa4..90e441a6e5 100644
  --- a/builtin/stash.c
  +++ b/builtin/stash.c
  @@ -1802,7 +1802,8 @@ static int push_stash(int argc, const char **argv, const char *prefix,

 		  argc = parse_options(argc, argv, prefix, options,
 				       push_assumed ? git_stash_usage :
  -				     git_stash_push_usage, flags);
  +						    git_stash_push_usage,
  +				     flags);
 		  force_assume |= patch_mode;
 	  }

  diff --git a/bundle-uri.c b/bundle-uri.c
  index c9d65aa0ce..89f59aafe8 100644
  --- a/bundle-uri.c
  +++ b/bundle-uri.c
  @@ -123,7 +123,7 @@ void print_bundle_list(FILE *fp, struct bundle_list *list)
 		  for (i = 0; i < BUNDLE_HEURISTIC__COUNT; i++) {
 			  if (heuristics[i].heuristic == list->heuristic) {
 				  fprintf(fp, "\theuristic = %s\n",
  -				       heuristics[list->heuristic].name);
  +					heuristics[list->heuristic].name);
 				  break;
 			  }
 		  }
  diff --git a/diff-no-index.c b/diff-no-index.c
  index 4aeeb98cfa..a3892a9ccc 100644
  --- a/diff-no-index.c
  +++ b/diff-no-index.c
  @@ -325,7 +325,7 @@ static int fixup_paths(const char **path, struct strbuf *replacement)
 	  return 0;
   }

  -static const char * const diff_no_index_usage[] = {
  +static const char *const diff_no_index_usage[] = {
 	  N_("git diff --no-index [<options>] <path> <path> [<pathspec>...]"),
 	  NULL
   };
  diff --git a/pathspec.h b/pathspec.h
  index 5e3a6f1fe7..601b9ca201 100644
  --- a/pathspec.h
  +++ b/pathspec.h
  @@ -80,7 +80,7 @@ struct pathspec {
    * For git diff --no-index, indicate that we are operating without
    * a repository or index.
    */
  -#define PATHSPEC_NO_REPOSITORY (1<<7)
  +#define PATHSPEC_NO_REPOSITORY (1 << 7)

   /**
    * Given command line arguments and a prefix, convert the input to

While now I'm tempted to mark the 'check-style' CI job as required. I
think we should do that in the future.

[1]: https://lore.kernel.org/git/xmqqmsa3adpw.fsf@gitster.g/

---
 .clang-format         | 27 +++++++++++++++------------
 .editorconfig         |  1 +
 ci/run-style-check.sh | 18 +-----------------
 meson.build           | 12 ++++++++++++
 4 files changed, 29 insertions(+), 29 deletions(-)

Karthik Nayak (4):
      editorconfig: set maximum line length to 120 characters
      clang-format: set 'ColumnLimit' to 0
      clang-format: add 'RemoveBracesLLVM' to the main config
      meson: add rule to run 'git clang-format'



base-commit: f0135a9047ca37d4d117dcf21f7e3e89fad85d00
change-id: 20250625-525-make-clang-format-more-robust-968126c991b9

Thanks
- Karthik

[PATCH 1/4] editorconfig: set maximum line length to 120 characters

[Karthik Nayak]

As per 'Documentation/CodingGuidelines', we try to keep to at most 80
characters per line. However, there are often certain cases where we
extend this for the sake of readability.

Add a maximum limit of 120 characters to the '.editorconfig'. This means
that if an individual line exceeds 120 characters, the editor will wrap
that line. This provides a lot wiggle room over the recommended 80
character limit.

Contrary to settings within '.clang-format' which are used for
statically formatting source code, the '.editorconfig' rules are hints
to the editor. These are not enforced by CI and are guidelines for
editors to follow. As such, the 'max_line_length' used here is only
supported in a set of editors [1].

[1]: https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 .editorconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.editorconfig b/.editorconfig
index 2d3929b591..d0f940fd23 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -7,6 +7,7 @@ insert_final_newline = true
 [{*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,Makefile}]
 indent_style = tab
 tab_width = 8
+max_line_length = 120
 
 [*.py]
 indent_style = space

-- 
2.49.0

[PATCH 2/4] clang-format: set 'ColumnLimit' to 0

[Karthik Nayak]

When clang-format was introduced to the Git project in
6134de6ac1 (clang-format: outline the git project's coding style,
2017-08-14), the 'ColumnLimit' was set to 80. This is inline with our
recommendation in 'Documentation/CodingGuidelines', which states:

  We try to keep to at most 80 characters per line.

However while this is recommended limit, this is not the enforced
limit. In some cases in we do overflow this limit to prioritize
readability. Setting the 'ColumnLimit' also means that shorter lines are
concatenated to simply as the result would still be below 80 characters,
which is undesirable.

In the past, we tried to adjust the penalties around line wrapping, once
in 42efde4c29 (clang-format: adjust line break penalties, 2017-09-29)
and another time in 5e9fa0f9fa (clang-format: re-adjust line break
penalties, 2024-10-18). While these settings help tweak the line break
penalties to be more in-line with the requirements of the Git project,
using 'clang-format' still produces a lot of false positives.

So to make 'clang-format' more usable, set the 'ColumnLimit' to 0. This
means that line-wrapping is no-longer a concern of the formatter and
something that the user needs to take care of. The previous commit also
added a more flexible guideline to the '.editorconfig' setting a
'max_line_length' of 120 characters. This should provide some guidance
to users.

In the future, it would be nice to re-instate this limit with adequate
penalties which would follow our guidelines, but currently, it makes
more sense to have a working formatter which we can rely on and which
doesn't create too many false positives.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 .clang-format | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/.clang-format b/.clang-format
index 9547fe1b77..19d6cf4200 100644
--- a/.clang-format
+++ b/.clang-format
@@ -12,7 +12,15 @@ UseTab: Always
 TabWidth: 8
 IndentWidth: 8
 ContinuationIndentWidth: 8
-ColumnLimit: 80
+
+# While we do want to enforce a character limit of 80 characters, we often
+# allow lines to overflow that limit to prioritize readability. Setting a
+# character limit here with penalties has been finicky and creates too many
+# false positives.
+#
+# NEEDSWORK: It would be nice if we can find optimal settings to ensure we
+# can re-enable the limit here.
+ColumnLimit: 0
 
 # C Language specifics
 Language: Cpp
@@ -210,16 +218,5 @@ MaxEmptyLinesToKeep: 1
 # No empty line at the start of a block.
 KeepEmptyLinesAtTheStartOfBlocks: false
 
-# Penalties
-# This decides what order things should be done if a line is too long
-PenaltyBreakAssignment: 5
-PenaltyBreakBeforeFirstCallParameter: 5
-PenaltyBreakComment: 5
-PenaltyBreakFirstLessLess: 0
-PenaltyBreakOpenParenthesis: 300
-PenaltyBreakString: 5
-PenaltyExcessCharacter: 10
-PenaltyReturnTypeOnItsOwnLine: 300
-
 # Don't sort #include's
 SortIncludes: false

-- 
2.49.0

[PATCH 3/4] clang-format: add 'RemoveBracesLLVM' to the main config

[Karthik Nayak]

In 1b8f306612 (ci/style-check: add `RemoveBracesLLVM` in CI job,
2024-07-23) we added 'RemoveBracesLLVM' to the CI job of running the
clang formatter.

This rule checks and warns against using braces on simple
single-statement bodies of statements. Since we haven't had any issues
regarding this rule, we can now move it into the main clang-format
config and remove it from being CI exclusive.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 .clang-format         |  6 ++++++
 ci/run-style-check.sh | 18 +-----------------
 2 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/.clang-format b/.clang-format
index 19d6cf4200..dcfd0aad60 100644
--- a/.clang-format
+++ b/.clang-format
@@ -220,3 +220,9 @@ KeepEmptyLinesAtTheStartOfBlocks: false
 
 # Don't sort #include's
 SortIncludes: false
+
+# Remove optional braces of control statements (if, else, for, and while)
+# according to the LLVM coding style. This avoids braces on simple
+# single-statement bodies of statements but keeps braces if one side of
+# if/else if/.../else cascade has multi-statement body.
+RemoveBracesLLVM: true
diff --git a/ci/run-style-check.sh b/ci/run-style-check.sh
index 6cd4b1d934..0832c19df0 100755
--- a/ci/run-style-check.sh
+++ b/ci/run-style-check.sh
@@ -5,21 +5,5 @@
 
 baseCommit=$1
 
-# Remove optional braces of control statements (if, else, for, and while)
-# according to the LLVM coding style. This avoids braces on simple
-# single-statement bodies of statements but keeps braces if one side of
-# if/else if/.../else cascade has multi-statement body.
-#
-# As this rule comes with a warning [1], we want to experiment with it
-# before adding it in-tree. since the CI job for the style check is allowed
-# to fail, appending the rule here allows us to validate its efficacy.
-# While also ensuring that end-users are not affected directly.
-#
-# [1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm
-{
-	cat .clang-format
-	echo "RemoveBracesLLVM: true"
-} >/tmp/clang-format-rules
-
-git clang-format --style=file:/tmp/clang-format-rules \
+git clang-format --style=file:.clang-format \
 	--diff --extensions c,h "$baseCommit"

-- 
2.49.0

[PATCH 4/4] meson: add rule to run 'git clang-format'

[Karthik Nayak]

The Makefile has a 'style' rule to run 'git clang-format'. While Meson
intrinsically supports a 'clang-format' target, which can be run when
using the ninja backend by running 'ninja clang-format', this runs the
formatting on all existing files.

Our Meson build doesn't yet support a way to run 'git clang-format',
which runs the formatter between the working directory and commit
provided. Add a new 'style' target to Meson to mimic the target in the
Makefile.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 meson.build | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/meson.build b/meson.build
index 7fea4a34d6..578db26df2 100644
--- a/meson.build
+++ b/meson.build
@@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
   alias_target('check-headers', hdr_check)
 endif
 
+clang_format = find_program('clang-format', required: false)
+if clang_format.found()
+  run_target('style',
+    command: [
+      'git', 'clang-format',
+      '--style', 'file',
+      '--diff',
+      '--extensions', 'c,h'
+    ]
+  )
+endif
+
 foreach key, value : {
   'DIFF': diff.full_path(),
   'GIT_SOURCE_DIR': meson.project_source_root(),

-- 
2.49.0

Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> As per 'Documentation/CodingGuidelines', we try to keep to at most 80
> characters per line. However, there are often certain cases where we
> extend this for the sake of readability.
>
> Add a maximum limit of 120 characters to the '.editorconfig'. This means
> that if an individual line exceeds 120 characters, the editor will wrap
> that line. This provides a lot wiggle room over the recommended 80
> character limit.

Ideally "when the line is overly long to be more than 120 columns,
please wrap it to 80 columns or less" is what we want.  If the
result of formatting a single 125 column line leaves us with two
lines, one with 100 columns and another with 25 columns, this would
not be very useful.  As this is meant to give suggestions without
enforcing hard rule, wouldn't it make more sense to set it to 80?

I dunno.

> Contrary to settings within '.clang-format' which are used for
> statically formatting source code, the '.editorconfig' rules are hints
> to the editor. These are not enforced by CI and are guidelines for
> editors to follow. As such, the 'max_line_length' used here is only
> supported in a set of editors [1].
>
> [1]: https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length

Thanks.

The discussion around "rulers" https://github.com/editorconfig/editorconfig/issues/89
was also interesting.

> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  .editorconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/.editorconfig b/.editorconfig
> index 2d3929b591..d0f940fd23 100644
> --- a/.editorconfig
> +++ b/.editorconfig
> @@ -7,6 +7,7 @@ insert_final_newline = true
>  [{*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,Makefile}]
>  indent_style = tab
>  tab_width = 8
> +max_line_length = 120
>  
>  [*.py]
>  indent_style = space

Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters

[Karthik Nayak]

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

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> As per 'Documentation/CodingGuidelines', we try to keep to at most 80
>> characters per line. However, there are often certain cases where we
>> extend this for the sake of readability.
>>
>> Add a maximum limit of 120 characters to the '.editorconfig'. This means
>> that if an individual line exceeds 120 characters, the editor will wrap
>> that line. This provides a lot wiggle room over the recommended 80
>> character limit.
>
> Ideally "when the line is overly long to be more than 120 columns,
> please wrap it to 80 columns or less" is what we want.
>

Yup, this would be nice, but neither '.editorconfig' or '.clang-format'
support a rule like this.

> If the
> result of formatting a single 125 column line leaves us with two
> lines, one with 100 columns and another with 25 columns, this would
> not be very useful.  As this is meant to give suggestions without
> enforcing hard rule, wouldn't it make more sense to set it to 80?
>
> I dunno.
>

So my intent was to instead was to allow the user to be in charge of
line-wrapping, but for no reason should that go beyond 120 columns.

I'm happy to change that to 80 columns, this does mean that supported
editors will start wrapping at 80 columns. Users will have to override
as necessary.

>> Contrary to settings within '.clang-format' which are used for
>> statically formatting source code, the '.editorconfig' rules are hints
>> to the editor. These are not enforced by CI and are guidelines for
>> editors to follow. As such, the 'max_line_length' used here is only
>> supported in a set of editors [1].
>>
>> [1]: https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
>
> Thanks.
>
> The discussion around "rulers" https://github.com/editorconfig/editorconfig/issues/89
> was also interesting.
>

Indeed. That would be a really nice feature and in line with what we
would want.

>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>  .editorconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/.editorconfig b/.editorconfig
>> index 2d3929b591..d0f940fd23 100644
>> --- a/.editorconfig
>> +++ b/.editorconfig
>> @@ -7,6 +7,7 @@ insert_final_newline = true
>>  [{*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,Makefile}]
>>  indent_style = tab
>>  tab_width = 8
>> +max_line_length = 120
>>
>>  [*.py]
>>  indent_style = space

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

Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> So my intent was to instead was to allow the user to be in charge of
> line-wrapping, but for no reason should that go beyond 120 columns.
>
> I'm happy to change that to 80 columns, this does mean that supported
> editors will start wrapping at 80 columns. Users will have to override
> as necessary.

OK.  So stepping back a bit

 1. We advise people to avoid exceeding 80 columns

 2. A line can be easier to read without wrapping strictly at 80
    columns but left as a single line, slightly going above the
    limit..

 3. Even with the second observation above, a line that is way
    longer than 80 columns is not acceptably long.

Now, what is the line between the #2 and #3?  If we set these "hard
limit" to that number, as long as the tool does not pack multiple
shorter lines into one line using that number as a limit, we'd be at
a happy place, I would imagine?

Thanks.

Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters

[Justin Tobler]

On 25/06/25 06:43PM, Karthik Nayak wrote:
> As per 'Documentation/CodingGuidelines', we try to keep to at most 80
> characters per line. However, there are often certain cases where we
> extend this for the sake of readability.
> 
> Add a maximum limit of 120 characters to the '.editorconfig'. This means
> that if an individual line exceeds 120 characters, the editor will wrap
> that line. This provides a lot wiggle room over the recommended 80
> character limit.

I frequently use the format operator in vim to reformat entire blocks of
text and it is commonly configured to use `max_line_length` from an
`.editorconfig` file to know when to wrap lines. Changing the value to
120 would cause my editor to prefer 120 character lines when
reformatting, which I would personally not like.

Being that `max_line_length` is only a suggestion for the editor, I
think we may be better off setting it to 80 characters or leaving it
unset entirely.

-Justin

> Contrary to settings within '.clang-format' which are used for
> statically formatting source code, the '.editorconfig' rules are hints
> to the editor. These are not enforced by CI and are guidelines for
> editors to follow. As such, the 'max_line_length' used here is only
> supported in a set of editors [1].
> 
> [1]: https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
> 
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>

Re: [PATCH 4/4] meson: add rule to run 'git clang-format'

[Justin Tobler]

On 25/06/25 06:43PM, Karthik Nayak wrote:
> The Makefile has a 'style' rule to run 'git clang-format'. While Meson
> intrinsically supports a 'clang-format' target, which can be run when
> using the ninja backend by running 'ninja clang-format', this runs the
> formatting on all existing files.
> 
> Our Meson build doesn't yet support a way to run 'git clang-format',
> which runs the formatter between the working directory and commit
> provided. Add a new 'style' target to Meson to mimic the target in the
> Makefile.
> 
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  meson.build | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 7fea4a34d6..578db26df2 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>    alias_target('check-headers', hdr_check)
>  endif
>  
> +clang_format = find_program('clang-format', required: false)

Should we be checking for `git-clang-format` instead?

> +if clang_format.found()
> +  run_target('style',
> +    command: [
> +      'git', 'clang-format',
> +      '--style', 'file',
> +      '--diff',
> +      '--extensions', 'c,h'
> +    ]
> +  )
> +endif
> +
>  foreach key, value : {
>    'DIFF': diff.full_path(),
>    'GIT_SOURCE_DIR': meson.project_source_root(),
> 
> -- 
> 2.49.0
> 
> 

Re: [PATCH 4/4] meson: add rule to run 'git clang-format'

[Karthik Nayak]

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

Justin Tobler <jltobler@gmail.com> writes:

> On 25/06/25 06:43PM, Karthik Nayak wrote:
>> The Makefile has a 'style' rule to run 'git clang-format'. While Meson
>> intrinsically supports a 'clang-format' target, which can be run when
>> using the ninja backend by running 'ninja clang-format', this runs the
>> formatting on all existing files.
>>
>> Our Meson build doesn't yet support a way to run 'git clang-format',
>> which runs the formatter between the working directory and commit
>> provided. Add a new 'style' target to Meson to mimic the target in the
>> Makefile.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>  meson.build | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index 7fea4a34d6..578db26df2 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>>    alias_target('check-headers', hdr_check)
>>  endif
>>
>> +clang_format = find_program('clang-format', required: false)
>
> Should we be checking for `git-clang-format` instead?
>

Yeah. While `git-clang-format` is packaged with `clang-format`, it does
make more sense to check for the former.

Thanks

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

Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters

[Karthik Nayak]

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

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> So my intent was to instead was to allow the user to be in charge of
>> line-wrapping, but for no reason should that go beyond 120 columns.
>>
>> I'm happy to change that to 80 columns, this does mean that supported
>> editors will start wrapping at 80 columns. Users will have to override
>> as necessary.
>
> OK.  So stepping back a bit
>
>  1. We advise people to avoid exceeding 80 columns
>
>  2. A line can be easier to read without wrapping strictly at 80
>     columns but left as a single line, slightly going above the
>     limit..
>
>  3. Even with the second observation above, a line that is way
>     longer than 80 columns is not acceptably long.
>
> Now, what is the line between the #2 and #3?  If we set these "hard
> limit" to that number, as long as the tool does not pack multiple
> shorter lines into one line using that number as a limit, we'd be at
> a happy place, I would imagine?
>

Spot on. Doing some backhand analysis on our code base:

Total lines: 2136626
Max length: 904
Average length: 36.2
Median length: 32.0
90th percentile: 73.0
95th percentile: 79.0

Lines > 80 chars: 91650 (4.3%)
Lines > 100 chars: 54589 (2.6%)
Lines > 120 chars: 35529 (1.7%)

The `max_line_length` in '.editorconfig' is stated as:

  Forces hard line wrapping after the amount of characters specified.
  unset to turn off this feature (use the editor settings).

So it doesn't force packing shorter lines together, rather, only wraps
lines above the specified limit. This limit can be thought of the limit
beyond which a line is of unacceptable length. We can move this lower to
perhaps 100 or even 80, but that would force line wrapping at 80
characters. Which can be great, but there are situations like you
mentioned where we want to extend.

> Thanks.

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

Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters

[Karthik Nayak]

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

Justin Tobler <jltobler@gmail.com> writes:

> On 25/06/25 06:43PM, Karthik Nayak wrote:
>> As per 'Documentation/CodingGuidelines', we try to keep to at most 80
>> characters per line. However, there are often certain cases where we
>> extend this for the sake of readability.
>>
>> Add a maximum limit of 120 characters to the '.editorconfig'. This means
>> that if an individual line exceeds 120 characters, the editor will wrap
>> that line. This provides a lot wiggle room over the recommended 80
>> character limit.
>

Hello Justin,

> I frequently use the format operator in vim to reformat entire blocks of
> text and it is commonly configured to use `max_line_length` from an
> `.editorconfig` file to know when to wrap lines. Changing the value to
> 120 would cause my editor to prefer 120 character lines when
> reformatting, which I would personally not like.
>

It would only wrap lines longer than 120 columns. Currently editorconfig
doesn't wrap any line length. So we're essentially saying, any line
above 120 is not something we want to accept and hence wrap. This
doesn't mean that shorter lines will be combined together. Wouldn't this
be better than the current situation?

> Being that `max_line_length` is only a suggestion for the editor, I
> think we may be better off setting it to 80 characters or leaving it
> unset entirely.
>
> -Justin
>
>> Contrary to settings within '.clang-format' which are used for
>> statically formatting source code, the '.editorconfig' rules are hints
>> to the editor. These are not enforced by CI and are guidelines for
>> editors to follow. As such, the 'max_line_length' used here is only
>> supported in a set of editors [1].
>>
>> [1]: https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>

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

Re: [PATCH 4/4] meson: add rule to run 'git clang-format'

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

>>> diff --git a/meson.build b/meson.build
>>> index 7fea4a34d6..578db26df2 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>>>    alias_target('check-headers', hdr_check)
>>>  endif
>>>
>>> +clang_format = find_program('clang-format', required: false)
>>
>> Should we be checking for `git-clang-format` instead?
>>
>
> Yeah. While `git-clang-format` is packaged with `clang-format`, it does
> make more sense to check for the former.

Just for my education, what does find_program() look for?  Installed
packages, or a program on your $PATH?  I am guessing that the answer
is the latter, in which case it is not like "it makes more sense to
check for git-clang-format"---rather it is "it would not work at all
if we looked for clang-format", no?

Thanks.

Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters

[Justin Tobler]

On 25/06/27 01:51AM, Karthik Nayak wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > On 25/06/25 06:43PM, Karthik Nayak wrote:
> >> As per 'Documentation/CodingGuidelines', we try to keep to at most 80
> >> characters per line. However, there are often certain cases where we
> >> extend this for the sake of readability.
> >>
> >> Add a maximum limit of 120 characters to the '.editorconfig'. This means
> >> that if an individual line exceeds 120 characters, the editor will wrap
> >> that line. This provides a lot wiggle room over the recommended 80
> >> character limit.
> >
> 
> Hello Justin,
> 
> > I frequently use the format operator in vim to reformat entire blocks of
> > text and it is commonly configured to use `max_line_length` from an
> > `.editorconfig` file to know when to wrap lines. Changing the value to
> > 120 would cause my editor to prefer 120 character lines when
> > reformatting, which I would personally not like.
> >
> 
> It would only wrap lines longer than 120 columns. Currently editorconfig
> doesn't wrap any line length. So we're essentially saying, any line
> above 120 is not something we want to accept and hence wrap. This
> doesn't mean that shorter lines will be combined together. Wouldn't this
> be better than the current situation?

When `max_line_length` is set in a ".editorconfig" file, in my vim
editor it overrides the `textwidth` configuration which was already set
to 80 by default. So changing to 120 would change line wrapping behavior
for me at least. I could disable using the ".editorconfig", but I would
prefer to avoid doing that :)

-Justin

Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters

[Karthik Nayak]

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

Justin Tobler <jltobler@gmail.com> writes:

> On 25/06/27 01:51AM, Karthik Nayak wrote:
>> Justin Tobler <jltobler@gmail.com> writes:
>>
>> > On 25/06/25 06:43PM, Karthik Nayak wrote:
>> >> As per 'Documentation/CodingGuidelines', we try to keep to at most 80
>> >> characters per line. However, there are often certain cases where we
>> >> extend this for the sake of readability.
>> >>
>> >> Add a maximum limit of 120 characters to the '.editorconfig'. This means
>> >> that if an individual line exceeds 120 characters, the editor will wrap
>> >> that line. This provides a lot wiggle room over the recommended 80
>> >> character limit.
>> >
>>
>> Hello Justin,
>>
>> > I frequently use the format operator in vim to reformat entire blocks of
>> > text and it is commonly configured to use `max_line_length` from an
>> > `.editorconfig` file to know when to wrap lines. Changing the value to
>> > 120 would cause my editor to prefer 120 character lines when
>> > reformatting, which I would personally not like.
>> >
>>
>> It would only wrap lines longer than 120 columns. Currently editorconfig
>> doesn't wrap any line length. So we're essentially saying, any line
>> above 120 is not something we want to accept and hence wrap. This
>> doesn't mean that shorter lines will be combined together. Wouldn't this
>> be better than the current situation?
>
> When `max_line_length` is set in a ".editorconfig" file, in my vim
> editor it overrides the `textwidth` configuration which was already set
> to 80 by default. So changing to 120 would change line wrapping behavior
> for me at least. I could disable using the ".editorconfig", but I would
> prefer to avoid doing that :)
>
> -Justin

Thanks for explaining. So it seems like vim in this case _does_ combine
shorter lines to fit to 120 columns when formatting a block, this is not
something we desire. So let me drop this patch.

- Karthik

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

Re: [PATCH 4/4] meson: add rule to run 'git clang-format'

[Karthik Nayak]

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

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>>> diff --git a/meson.build b/meson.build
>>>> index 7fea4a34d6..578db26df2 100644
>>>> --- a/meson.build
>>>> +++ b/meson.build
>>>> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>>>>    alias_target('check-headers', hdr_check)
>>>>  endif
>>>>
>>>> +clang_format = find_program('clang-format', required: false)
>>>
>>> Should we be checking for `git-clang-format` instead?
>>>
>>
>> Yeah. While `git-clang-format` is packaged with `clang-format`, it does
>> make more sense to check for the former.
>
> Just for my education, what does find_program() look for?  Installed
> packages, or a program on your $PATH?  I am guessing that the answer
> is the latter, in which case it is not like "it makes more sense to
> check for git-clang-format"---rather it is "it would not work at all
> if we looked for clang-format", no?
>
> Thanks.

Good question. To quote from the documentation [1]:

  find_program()

  program_name here is a string that can be an executable or script to
  be searched for in PATH or other places inside the project.

So, 'git-clang-format' would work. I've also verified the same on my
end.

[1]: https://mesonbuild.com/Reference-manual_functions.html#find_program

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

[PATCH v2 0/3] clang-format: modify rules to reduce false-positives

[Karthik Nayak]

This series is in response to an email thread [1] around the usage of
'.clang-format' and its effectiveness.

The goal of the series is to improve the usage of 'clang-format' in the
repository. To do this we:
1. Reduce the number of false positives. Majority of which is due to
   line-wrapping. We remove the 'ColumnLimit' from 'clang-format'. This
   removes the responsibility of line-wrapping from 'clang-format' and puts
   it into the hands of the user.
2. Add a rule to 'meson' to run 'git clang-format' by running 'meson
   compile style'.
3. Make the 'RemoveBracesLLVM' permanent by moving it to
   '.clang-format'.

With this, running `git clang-format` for the last 25 commits in master,
seems to produce much less false positives.

  diff --git a/builtin/diff.c b/builtin/diff.c
  index c6231edce4..246b81caa2 100644
  --- a/builtin/diff.c
  +++ b/builtin/diff.c
  @@ -30,14 +30,13 @@
   #define DIFF_NO_INDEX_IMPLICIT 2

   static const char builtin_diff_usage[] =
  -"git diff [<options>] [<commit>] [--] [<path>...]\n"
  -"   or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\n"
  -"   or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\n"
  -"   or: git diff [<options>] <commit>...<commit> [--] [<path>...]\n"
  -"   or: git diff [<options>] <blob> <blob>\n"
  -"   or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]"
  -"\n"
  -COMMON_DIFF_OPTIONS_HELP;
  +	"git diff [<options>] [<commit>] [--] [<path>...]\n"
  +	"   or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\n"
  +	"   or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\n"
  +	"   or: git diff [<options>] <commit>...<commit> [--] [<path>...]\n"
  +	"   or: git diff [<options>] <blob> <blob>\n"
  +	"   or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]"
  +	"\n" COMMON_DIFF_OPTIONS_HELP;

   static const char *blob_path(struct object_array_entry *entry)
   {
  diff --git a/builtin/stash.c b/builtin/stash.c
  index 7cd3ad8aa4..90e441a6e5 100644
  --- a/builtin/stash.c
  +++ b/builtin/stash.c
  @@ -1802,7 +1802,8 @@ static int push_stash(int argc, const char **argv, const char *prefix,

 		  argc = parse_options(argc, argv, prefix, options,
 				       push_assumed ? git_stash_usage :
  -				     git_stash_push_usage, flags);
  +						    git_stash_push_usage,
  +				     flags);
 		  force_assume |= patch_mode;
 	  }

  diff --git a/bundle-uri.c b/bundle-uri.c
  index c9d65aa0ce..89f59aafe8 100644
  --- a/bundle-uri.c
  +++ b/bundle-uri.c
  @@ -123,7 +123,7 @@ void print_bundle_list(FILE *fp, struct bundle_list *list)
 		  for (i = 0; i < BUNDLE_HEURISTIC__COUNT; i++) {
 			  if (heuristics[i].heuristic == list->heuristic) {
 				  fprintf(fp, "\theuristic = %s\n",
  -				       heuristics[list->heuristic].name);
  +					heuristics[list->heuristic].name);
 				  break;
 			  }
 		  }
  diff --git a/diff-no-index.c b/diff-no-index.c
  index 4aeeb98cfa..a3892a9ccc 100644
  --- a/diff-no-index.c
  +++ b/diff-no-index.c
  @@ -325,7 +325,7 @@ static int fixup_paths(const char **path, struct strbuf *replacement)
 	  return 0;
   }

  -static const char * const diff_no_index_usage[] = {
  +static const char *const diff_no_index_usage[] = {
 	  N_("git diff --no-index [<options>] <path> <path> [<pathspec>...]"),
 	  NULL
   };
  diff --git a/pathspec.h b/pathspec.h
  index 5e3a6f1fe7..601b9ca201 100644
  --- a/pathspec.h
  +++ b/pathspec.h
  @@ -80,7 +80,7 @@ struct pathspec {
    * For git diff --no-index, indicate that we are operating without
    * a repository or index.
    */
  -#define PATHSPEC_NO_REPOSITORY (1<<7)
  +#define PATHSPEC_NO_REPOSITORY (1 << 7)

   /**
    * Given command line arguments and a prefix, convert the input to

While now I'm tempted to mark the 'check-style' CI job as required. I
think we should do that in the future.

[1]: https://lore.kernel.org/git/xmqqmsa3adpw.fsf@gitster.g/

---
Changes in v2:
- Drop the patch to add 120 column length to editorconfig. This way, we
  will continue to use the default of 80 columns. Adding a higher column
  length makes editorconfig combine smaller lines during block
  formatting. This is not desirable.
- Ensure that meson specifically checks for 'git-clang-format' and not
  just 'clang-format'.
- Link to v1: https://lore.kernel.org/r/20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com

---
 .clang-format         | 27 +++++++++++++++------------
 ci/run-style-check.sh | 18 +-----------------
 meson.build           | 12 ++++++++++++
 3 files changed, 28 insertions(+), 29 deletions(-)

Karthik Nayak (3):
      clang-format: set 'ColumnLimit' to 0
      clang-format: add 'RemoveBracesLLVM' to the main config
      meson: add rule to run 'git clang-format'

Range-diff versus v1:

1:  a62b4b7fe8 < -:  ---------- editorconfig: set maximum line length to 120 characters
2:  4b4d49273e = 1:  5479d22f80 clang-format: set 'ColumnLimit' to 0
3:  b9abb50adf = 2:  dacc7cc2b3 clang-format: add 'RemoveBracesLLVM' to the main config
4:  03cc722f8f ! 3:  b13832ed57 meson: add rule to run 'git clang-format'
    @@ meson.build: if headers_to_check.length() != 0 and compiler.get_argument_syntax(
        alias_target('check-headers', hdr_check)
      endif
      
    -+clang_format = find_program('clang-format', required: false)
    -+if clang_format.found()
    ++git_clang_format = find_program('git-clang-format', required: false)
    ++if git_clang_format.found()
     +  run_target('style',
     +    command: [
     +      'git', 'clang-format',


base-commit: f0135a9047ca37d4d117dcf21f7e3e89fad85d00
change-id: 20250625-525-make-clang-format-more-robust-968126c991b9

Thanks
- Karthik

[PATCH v2 1/3] clang-format: set 'ColumnLimit' to 0

[Karthik Nayak]

When clang-format was introduced to the Git project in
6134de6ac1 (clang-format: outline the git project's coding style,
2017-08-14), the 'ColumnLimit' was set to 80. This is inline with our
recommendation in 'Documentation/CodingGuidelines', which states:

  We try to keep to at most 80 characters per line.

However while this is recommended limit, this is not the enforced
limit. In some cases in we do overflow this limit to prioritize
readability. Setting the 'ColumnLimit' also means that shorter lines are
concatenated to simply as the result would still be below 80 characters,
which is undesirable.

In the past, we tried to adjust the penalties around line wrapping, once
in 42efde4c29 (clang-format: adjust line break penalties, 2017-09-29)
and another time in 5e9fa0f9fa (clang-format: re-adjust line break
penalties, 2024-10-18). While these settings help tweak the line break
penalties to be more in-line with the requirements of the Git project,
using 'clang-format' still produces a lot of false positives.

So to make 'clang-format' more usable, set the 'ColumnLimit' to 0. This
means that line-wrapping is no-longer a concern of the formatter and
something that the user needs to take care of. The previous commit also
added a more flexible guideline to the '.editorconfig' setting a
'max_line_length' of 120 characters. This should provide some guidance
to users.

In the future, it would be nice to re-instate this limit with adequate
penalties which would follow our guidelines, but currently, it makes
more sense to have a working formatter which we can rely on and which
doesn't create too many false positives.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 .clang-format | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/.clang-format b/.clang-format
index 9547fe1b77..19d6cf4200 100644
--- a/.clang-format
+++ b/.clang-format
@@ -12,7 +12,15 @@ UseTab: Always
 TabWidth: 8
 IndentWidth: 8
 ContinuationIndentWidth: 8
-ColumnLimit: 80
+
+# While we do want to enforce a character limit of 80 characters, we often
+# allow lines to overflow that limit to prioritize readability. Setting a
+# character limit here with penalties has been finicky and creates too many
+# false positives.
+#
+# NEEDSWORK: It would be nice if we can find optimal settings to ensure we
+# can re-enable the limit here.
+ColumnLimit: 0
 
 # C Language specifics
 Language: Cpp
@@ -210,16 +218,5 @@ MaxEmptyLinesToKeep: 1
 # No empty line at the start of a block.
 KeepEmptyLinesAtTheStartOfBlocks: false
 
-# Penalties
-# This decides what order things should be done if a line is too long
-PenaltyBreakAssignment: 5
-PenaltyBreakBeforeFirstCallParameter: 5
-PenaltyBreakComment: 5
-PenaltyBreakFirstLessLess: 0
-PenaltyBreakOpenParenthesis: 300
-PenaltyBreakString: 5
-PenaltyExcessCharacter: 10
-PenaltyReturnTypeOnItsOwnLine: 300
-
 # Don't sort #include's
 SortIncludes: false

-- 
2.49.0

[PATCH v2 2/3] clang-format: add 'RemoveBracesLLVM' to the main config

[Karthik Nayak]

In 1b8f306612 (ci/style-check: add `RemoveBracesLLVM` in CI job,
2024-07-23) we added 'RemoveBracesLLVM' to the CI job of running the
clang formatter.

This rule checks and warns against using braces on simple
single-statement bodies of statements. Since we haven't had any issues
regarding this rule, we can now move it into the main clang-format
config and remove it from being CI exclusive.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 .clang-format         |  6 ++++++
 ci/run-style-check.sh | 18 +-----------------
 2 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/.clang-format b/.clang-format
index 19d6cf4200..dcfd0aad60 100644
--- a/.clang-format
+++ b/.clang-format
@@ -220,3 +220,9 @@ KeepEmptyLinesAtTheStartOfBlocks: false
 
 # Don't sort #include's
 SortIncludes: false
+
+# Remove optional braces of control statements (if, else, for, and while)
+# according to the LLVM coding style. This avoids braces on simple
+# single-statement bodies of statements but keeps braces if one side of
+# if/else if/.../else cascade has multi-statement body.
+RemoveBracesLLVM: true
diff --git a/ci/run-style-check.sh b/ci/run-style-check.sh
index 6cd4b1d934..0832c19df0 100755
--- a/ci/run-style-check.sh
+++ b/ci/run-style-check.sh
@@ -5,21 +5,5 @@
 
 baseCommit=$1
 
-# Remove optional braces of control statements (if, else, for, and while)
-# according to the LLVM coding style. This avoids braces on simple
-# single-statement bodies of statements but keeps braces if one side of
-# if/else if/.../else cascade has multi-statement body.
-#
-# As this rule comes with a warning [1], we want to experiment with it
-# before adding it in-tree. since the CI job for the style check is allowed
-# to fail, appending the rule here allows us to validate its efficacy.
-# While also ensuring that end-users are not affected directly.
-#
-# [1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm
-{
-	cat .clang-format
-	echo "RemoveBracesLLVM: true"
-} >/tmp/clang-format-rules
-
-git clang-format --style=file:/tmp/clang-format-rules \
+git clang-format --style=file:.clang-format \
 	--diff --extensions c,h "$baseCommit"

-- 
2.49.0

[PATCH v2 3/3] meson: add rule to run 'git clang-format'

[Karthik Nayak]

The Makefile has a 'style' rule to run 'git clang-format'. While Meson
intrinsically supports a 'clang-format' target, which can be run when
using the ninja backend by running 'ninja clang-format', this runs the
formatting on all existing files.

Our Meson build doesn't yet support a way to run 'git clang-format',
which runs the formatter between the working directory and commit
provided. Add a new 'style' target to Meson to mimic the target in the
Makefile.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 meson.build | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/meson.build b/meson.build
index 7fea4a34d6..20ce0525a1 100644
--- a/meson.build
+++ b/meson.build
@@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
   alias_target('check-headers', hdr_check)
 endif
 
+git_clang_format = find_program('git-clang-format', required: false)
+if git_clang_format.found()
+  run_target('style',
+    command: [
+      'git', 'clang-format',
+      '--style', 'file',
+      '--diff',
+      '--extensions', 'c,h'
+    ]
+  )
+endif
+
 foreach key, value : {
   'DIFF': diff.full_path(),
   'GIT_SOURCE_DIR': meson.project_source_root(),

-- 
2.49.0

Re: [PATCH 4/4] meson: add rule to run 'git clang-format'

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

>>>>> +clang_format = find_program('clang-format', required: false)
>>>>
>>>> Should we be checking for `git-clang-format` instead?
>>>
>>> Yeah. While `git-clang-format` is packaged with `clang-format`, it does
>>> make more sense to check for the former.
>>
>> Just for my education, what does find_program() look for?  Installed
>> packages, or a program on your $PATH?  I am guessing that the answer
>> is the latter, in which case it is not like "it makes more sense to
>> check for git-clang-format"---rather it is "it would not work at all
>> if we looked for clang-format", no?
>>
>> Thanks.
>
> Good question. To quote from the documentation [1]:
>
>   find_program()
>
>   program_name here is a string that can be an executable or script to
>   be searched for in PATH or other places inside the project.
>
> So, 'git-clang-format' would work. I've also verified the same on my
> end.

I think that much everybody would know by what other uses of
find_program() are looking for by checking "git grep find_program"
output.

I was confused by your "it does make *MORE* sense to check for the
former" (emphasis mine), as if you were saying that both would work
but using 'git-clang-format' would be a more kosher way to express
what we want.  Given that it would not work at all if you used
'clang-format' instead, that statement was misleading.

So the response I was expecting in the message I am responding to
was more like "'clang-format' would *not* work at all, and we must
check 'git-clang-format' instead".

Thanks.

Re: [PATCH v2 0/3] clang-format: modify rules to reduce false-positives

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> - Drop the patch to add 120 column length to editorconfig. This way, we
>   will continue to use the default of 80 columns. Adding a higher column
>   length makes editorconfig combine smaller lines during block
>   formatting. This is not desirable.

Makes sense.

> - Ensure that meson specifically checks for 'git-clang-format' and not
>   just 'clang-format'.

OK.

Looking better.  Thanks.

Re: [PATCH v2 3/3] meson: add rule to run 'git clang-format'

[Patrick Steinhardt]

On Mon, Jun 30, 2025 at 10:38:22AM +0200, Karthik Nayak wrote:
> The Makefile has a 'style' rule to run 'git clang-format'. While Meson
> intrinsically supports a 'clang-format' target, which can be run when
> using the ninja backend by running 'ninja clang-format', this runs the
> formatting on all existing files.
> 
> Our Meson build doesn't yet support a way to run 'git clang-format',
> which runs the formatter between the working directory and commit
> provided. Add a new 'style' target to Meson to mimic the target in the
> Makefile.

Hm. Meson already knows to wire up clang-format automaically if it's
available. But it indeed doesn't know to only format files that have
been changed, so I guess this style makes sense regardless of that.

> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  meson.build | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 7fea4a34d6..20ce0525a1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>    alias_target('check-headers', hdr_check)
>  endif
>  
> +git_clang_format = find_program('git-clang-format', required: false)
> +if git_clang_format.found()
> +  run_target('style',
> +    command: [
> +      'git', 'clang-format',
> +      '--style', 'file',
> +      '--diff',
> +      '--extensions', 'c,h'
> +    ]
> +  )
> +endif

Do we want to call this target `clang-format-changed` though, so that it
is consistent with the implicit `clang-format` target?

Patrick

Re: [PATCH v2 3/3] meson: add rule to run 'git clang-format'

[Toon Claes]

Karthik Nayak <karthik.188@gmail.com> writes:

> The Makefile has a 'style' rule to run 'git clang-format'. While Meson
> intrinsically supports a 'clang-format' target, which can be run when
> using the ninja backend by running 'ninja clang-format', this runs the
> formatting on all existing files.
>
> Our Meson build doesn't yet support a way to run 'git clang-format',
> which runs the formatter between the working directory and commit
> provided. Add a new 'style' target to Meson to mimic the target in the
> Makefile.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  meson.build | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index 7fea4a34d6..20ce0525a1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>    alias_target('check-headers', hdr_check)
>  endif
>  
> +git_clang_format = find_program('git-clang-format', required: false)

I think we should include `native: true` as well.

> +if git_clang_format.found()
> +  run_target('style',
> +    command: [
> +      'git', 'clang-format',

We should be using the `git_clang_format` variable here to ensure we
call the tool we've found.

> +      '--style', 'file',
> +      '--diff',
> +      '--extensions', 'c,h'
> +    ]
> +  )
> +endif
> +
>  foreach key, value : {
>    'DIFF': diff.full_path(),
>    'GIT_SOURCE_DIR': meson.project_source_root(),
>
> -- 
> 2.49.0
>
>

-- 
Cheers,
Toon

Re: [PATCH v2 0/3] clang-format: modify rules to reduce false-positives

[Toon Claes]

Karthik Nayak <karthik.188@gmail.com> writes:

> This series is in response to an email thread [1] around the usage of
> '.clang-format' and its effectiveness.
>
> The goal of the series is to improve the usage of 'clang-format' in the
> repository. To do this we:
> 1. Reduce the number of false positives. Majority of which is due to
>    line-wrapping. We remove the 'ColumnLimit' from 'clang-format'. This
>    removes the responsibility of line-wrapping from 'clang-format' and puts
>    it into the hands of the user.
> 2. Add a rule to 'meson' to run 'git clang-format' by running 'meson
>    compile style'.
> 3. Make the 'RemoveBracesLLVM' permanent by moving it to
>    '.clang-format'.

Thanks for working on this. While the changes are not huge, I appreciate
them.

> With this, running `git clang-format` for the last 25 commits in master,
> seems to produce much less false positives.
>
> [snip]
>
>   diff --git a/pathspec.h b/pathspec.h
>   index 5e3a6f1fe7..601b9ca201 100644
>   --- a/pathspec.h
>   +++ b/pathspec.h
>   @@ -80,7 +80,7 @@ struct pathspec {
>     * For git diff --no-index, indicate that we are operating without
>     * a repository or index.
>     */
>   -#define PATHSPEC_NO_REPOSITORY (1<<7)
>   +#define PATHSPEC_NO_REPOSITORY (1 << 7)

I'm surprised, but I couldn't find a setting to change this...


-- 
Cheers,
Toon

Re: [PATCH v2 3/3] meson: add rule to run 'git clang-format'

[Karthik Nayak]

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

Patrick Steinhardt <ps@pks.im> writes:

> On Mon, Jun 30, 2025 at 10:38:22AM +0200, Karthik Nayak wrote:
>> The Makefile has a 'style' rule to run 'git clang-format'. While Meson
>> intrinsically supports a 'clang-format' target, which can be run when
>> using the ninja backend by running 'ninja clang-format', this runs the
>> formatting on all existing files.
>>
>> Our Meson build doesn't yet support a way to run 'git clang-format',
>> which runs the formatter between the working directory and commit
>> provided. Add a new 'style' target to Meson to mimic the target in the
>> Makefile.
>
> Hm. Meson already knows to wire up clang-format automaically if it's
> available. But it indeed doesn't know to only format files that have
> been changed, so I guess this style makes sense regardless of that.
>

Exactly!

>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>  meson.build | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index 7fea4a34d6..20ce0525a1 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>>    alias_target('check-headers', hdr_check)
>>  endif
>>
>> +git_clang_format = find_program('git-clang-format', required: false)
>> +if git_clang_format.found()
>> +  run_target('style',
>> +    command: [
>> +      'git', 'clang-format',
>> +      '--style', 'file',
>> +      '--diff',
>> +      '--extensions', 'c,h'
>> +    ]
>> +  )
>> +endif
>
> Do we want to call this target `clang-format-changed` though, so that it
> is consistent with the implicit `clang-format` target?
>

I was aiming for consistency with the Makefile, I don't think anyone
uses `meson compile clang-format` anyways. I do like the short form, but
open to adding an alias and setting `make style` for deprecation in the
following releases.

> Patrick

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

Re: [PATCH v2 3/3] meson: add rule to run 'git clang-format'

[Karthik Nayak]

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

Toon Claes <toon@iotcl.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> The Makefile has a 'style' rule to run 'git clang-format'. While Meson
>> intrinsically supports a 'clang-format' target, which can be run when
>> using the ninja backend by running 'ninja clang-format', this runs the
>> formatting on all existing files.
>>
>> Our Meson build doesn't yet support a way to run 'git clang-format',
>> which runs the formatter between the working directory and commit
>> provided. Add a new 'style' target to Meson to mimic the target in the
>> Makefile.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>  meson.build | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index 7fea4a34d6..20ce0525a1 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
>>    alias_target('check-headers', hdr_check)
>>  endif
>>
>> +git_clang_format = find_program('git-clang-format', required: false)
>
> I think we should include `native: true` as well.
>

Does it really matter here? I must admit I don't understand the
repercussions here.

>> +if git_clang_format.found()
>> +  run_target('style',
>> +    command: [
>> +      'git', 'clang-format',
>
> We should be using the `git_clang_format` variable here to ensure we
> call the tool we've found.
>

Good point, let me add that.

>> +      '--style', 'file',
>> +      '--diff',
>> +      '--extensions', 'c,h'
>> +    ]
>> +  )
>> +endif
>> +
>>  foreach key, value : {
>>    'DIFF': diff.full_path(),
>>    'GIT_SOURCE_DIR': meson.project_source_root(),
>>
>> --
>> 2.49.0
>>
>>
>
> --
> Cheers,
> Toon

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

Re: [PATCH v2 0/3] clang-format: modify rules to reduce false-positives

[Karthik Nayak]

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

Toon Claes <toon@iotcl.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> This series is in response to an email thread [1] around the usage of
>> '.clang-format' and its effectiveness.
>>
>> The goal of the series is to improve the usage of 'clang-format' in the
>> repository. To do this we:
>> 1. Reduce the number of false positives. Majority of which is due to
>>    line-wrapping. We remove the 'ColumnLimit' from 'clang-format'. This
>>    removes the responsibility of line-wrapping from 'clang-format' and puts
>>    it into the hands of the user.
>> 2. Add a rule to 'meson' to run 'git clang-format' by running 'meson
>>    compile style'.
>> 3. Make the 'RemoveBracesLLVM' permanent by moving it to
>>    '.clang-format'.
>
> Thanks for working on this. While the changes are not huge, I appreciate
> them.
>
>> With this, running `git clang-format` for the last 25 commits in master,
>> seems to produce much less false positives.
>>
>> [snip]
>>
>>   diff --git a/pathspec.h b/pathspec.h
>>   index 5e3a6f1fe7..601b9ca201 100644
>>   --- a/pathspec.h
>>   +++ b/pathspec.h
>>   @@ -80,7 +80,7 @@ struct pathspec {
>>     * For git diff --no-index, indicate that we are operating without
>>     * a repository or index.
>>     */
>>   -#define PATHSPEC_NO_REPOSITORY (1<<7)
>>   +#define PATHSPEC_NO_REPOSITORY (1 << 7)
>
> I'm surprised, but I couldn't find a setting to change this...
>
>

I think spacing around binary operators is intrinsic and not an option
[1]. But I'm not sure...

[1]: https://github.com/llvm/llvm-project/issues/19412

> --
> Cheers,
> Toon

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

Re: [PATCH v2 3/3] meson: add rule to run 'git clang-format'

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>> +git_clang_format = find_program('git-clang-format', required: false)
>> +if git_clang_format.found()
>> +  run_target('style',
>> +    command: [
>> +      'git', 'clang-format',
>> +      '--style', 'file',
>> +      '--diff',
>> +      '--extensions', 'c,h'
>> +    ]
>> +  )
>> +endif
>
> Do we want to call this target `clang-format-changed` though, so that it
> is consistent with the implicit `clang-format` target?

It think this is trying to be consistent with the 'make style'
target.

Re: [PATCH v2 0/3] clang-format: modify rules to reduce false-positives

[Junio C Hamano]

Toon Claes <toon@iotcl.com> writes:

>>   -#define PATHSPEC_NO_REPOSITORY (1<<7)
>>   +#define PATHSPEC_NO_REPOSITORY (1 << 7)
>
> I'm surprised, but I couldn't find a setting to change this...

We do not mind spaces around binary operators like "<<", and the
change suggested by the tool is an improvement in this case, I
think.  The lack of U suffix to ensure that a bitmask is unsigned
bothers me more, but that is not formatting issue.

Thanks.

Re: [PATCH v2 3/3] meson: add rule to run 'git clang-format'

[Patrick Steinhardt]

On Tue, Jul 01, 2025 at 11:12:16AM -0400, Karthik Nayak wrote:
> Toon Claes <toon@iotcl.com> writes:
> > Karthik Nayak <karthik.188@gmail.com> writes:
> >> diff --git a/meson.build b/meson.build
> >> index 7fea4a34d6..20ce0525a1 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
> >>    alias_target('check-headers', hdr_check)
> >>  endif
> >>
> >> +git_clang_format = find_program('git-clang-format', required: false)
> >
> > I think we should include `native: true` as well.
> >
> 
> Does it really matter here? I must admit I don't understand the
> repercussions here.

It doesn't really, as `native: true` is the default. But we explicitly
say whether we want native or non-native binaries for all the other
calls to `find_program()` to make it more obvious, so I think it would
be a sensible addition here.

Patrick

[PATCH v3 0/3] clang-format: modify rules to reduce false-positives

[Karthik Nayak]

This series is in response to an email thread [1] around the usage of
'.clang-format' and its effectiveness.

The goal of the series is to improve the usage of 'clang-format' in the
repository. To do this we:
1. Reduce the number of false positives. Majority of which is due to
   line-wrapping. We remove the 'ColumnLimit' from 'clang-format'. This
   removes the responsibility of line-wrapping from 'clang-format' and puts
   it into the hands of the user.
2. Add a rule to 'meson' to run 'git clang-format' by running 'meson
   compile style'.
3. Make the 'RemoveBracesLLVM' permanent by moving it to
   '.clang-format'.

With this, running `git clang-format` for the last 25 commits in master,
seems to produce much less false positives.

  diff --git a/builtin/diff.c b/builtin/diff.c
  index c6231edce4..246b81caa2 100644
  --- a/builtin/diff.c
  +++ b/builtin/diff.c
  @@ -30,14 +30,13 @@
   #define DIFF_NO_INDEX_IMPLICIT 2

   static const char builtin_diff_usage[] =
  -"git diff [<options>] [<commit>] [--] [<path>...]\n"
  -"   or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\n"
  -"   or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\n"
  -"   or: git diff [<options>] <commit>...<commit> [--] [<path>...]\n"
  -"   or: git diff [<options>] <blob> <blob>\n"
  -"   or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]"
  -"\n"
  -COMMON_DIFF_OPTIONS_HELP;
  +	"git diff [<options>] [<commit>] [--] [<path>...]\n"
  +	"   or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\n"
  +	"   or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\n"
  +	"   or: git diff [<options>] <commit>...<commit> [--] [<path>...]\n"
  +	"   or: git diff [<options>] <blob> <blob>\n"
  +	"   or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]"
  +	"\n" COMMON_DIFF_OPTIONS_HELP;

   static const char *blob_path(struct object_array_entry *entry)
   {
  diff --git a/builtin/stash.c b/builtin/stash.c
  index 7cd3ad8aa4..90e441a6e5 100644
  --- a/builtin/stash.c
  +++ b/builtin/stash.c
  @@ -1802,7 +1802,8 @@ static int push_stash(int argc, const char **argv, const char *prefix,

 		  argc = parse_options(argc, argv, prefix, options,
 				       push_assumed ? git_stash_usage :
  -				     git_stash_push_usage, flags);
  +						    git_stash_push_usage,
  +				     flags);
 		  force_assume |= patch_mode;
 	  }

  diff --git a/bundle-uri.c b/bundle-uri.c
  index c9d65aa0ce..89f59aafe8 100644
  --- a/bundle-uri.c
  +++ b/bundle-uri.c
  @@ -123,7 +123,7 @@ void print_bundle_list(FILE *fp, struct bundle_list *list)
 		  for (i = 0; i < BUNDLE_HEURISTIC__COUNT; i++) {
 			  if (heuristics[i].heuristic == list->heuristic) {
 				  fprintf(fp, "\theuristic = %s\n",
  -				       heuristics[list->heuristic].name);
  +					heuristics[list->heuristic].name);
 				  break;
 			  }
 		  }
  diff --git a/diff-no-index.c b/diff-no-index.c
  index 4aeeb98cfa..a3892a9ccc 100644
  --- a/diff-no-index.c
  +++ b/diff-no-index.c
  @@ -325,7 +325,7 @@ static int fixup_paths(const char **path, struct strbuf *replacement)
 	  return 0;
   }

  -static const char * const diff_no_index_usage[] = {
  +static const char *const diff_no_index_usage[] = {
 	  N_("git diff --no-index [<options>] <path> <path> [<pathspec>...]"),
 	  NULL
   };
  diff --git a/pathspec.h b/pathspec.h
  index 5e3a6f1fe7..601b9ca201 100644
  --- a/pathspec.h
  +++ b/pathspec.h
  @@ -80,7 +80,7 @@ struct pathspec {
    * For git diff --no-index, indicate that we are operating without
    * a repository or index.
    */
  -#define PATHSPEC_NO_REPOSITORY (1<<7)
  +#define PATHSPEC_NO_REPOSITORY (1 << 7)

   /**
    * Given command line arguments and a prefix, convert the input to

While now I'm tempted to mark the 'check-style' CI job as required. I
think we should do that in the future.

[1]: https://lore.kernel.org/git/xmqqmsa3adpw.fsf@gitster.g/

---
Changes in v3:
- In meson.build, set 'native: true' and use the variable obtained from
  'find_program()' directly in 'run_target()'.
- Link to v2: https://lore.kernel.org/r/20250630-525-make-clang-format-more-robust-v2-0-05cbcdbf7817@gmail.com

Changes in v2:
- Drop the patch to add 120 column length to editorconfig. This way, we
  will continue to use the default of 80 columns. Adding a higher column
  length makes editorconfig combine smaller lines during block
  formatting. This is not desirable.
- Ensure that meson specifically checks for 'git-clang-format' and not
  just 'clang-format'.
- Link to v1: https://lore.kernel.org/r/20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com

---
 .clang-format         | 27 +++++++++++++++------------
 ci/run-style-check.sh | 18 +-----------------
 meson.build           | 12 ++++++++++++
 3 files changed, 28 insertions(+), 29 deletions(-)

Karthik Nayak (3):
      clang-format: set 'ColumnLimit' to 0
      clang-format: add 'RemoveBracesLLVM' to the main config
      meson: add rule to run 'git clang-format'

Range-diff versus v2:

1:  f8271d7114 = 1:  c7a7f6d798 clang-format: set 'ColumnLimit' to 0
2:  f49237edda = 2:  c68ea68349 clang-format: add 'RemoveBracesLLVM' to the main config
3:  8d2a1647a2 ! 3:  b972289ab8 meson: add rule to run 'git clang-format'
    @@ meson.build: if headers_to_check.length() != 0 and compiler.get_argument_syntax(
        alias_target('check-headers', hdr_check)
      endif
      
    -+git_clang_format = find_program('git-clang-format', required: false)
    ++git_clang_format = find_program('git-clang-format', required: false, native: true)
     +if git_clang_format.found()
     +  run_target('style',
     +    command: [
    -+      'git', 'clang-format',
    ++      git_clang_format,
     +      '--style', 'file',
     +      '--diff',
     +      '--extensions', 'c,h'


base-commit: f0135a9047ca37d4d117dcf21f7e3e89fad85d00
change-id: 20250625-525-make-clang-format-more-robust-968126c991b9

Thanks
- Karthik

[PATCH v3 1/3] clang-format: set 'ColumnLimit' to 0

[Karthik Nayak]

When clang-format was introduced to the Git project in
6134de6ac1 (clang-format: outline the git project's coding style,
2017-08-14), the 'ColumnLimit' was set to 80. This is inline with our
recommendation in 'Documentation/CodingGuidelines', which states:

  We try to keep to at most 80 characters per line.

However while this is recommended limit, this is not the enforced
limit. In some cases in we do overflow this limit to prioritize
readability. Setting the 'ColumnLimit' also means that shorter lines are
concatenated to simply as the result would still be below 80 characters,
which is undesirable.

In the past, we tried to adjust the penalties around line wrapping, once
in 42efde4c29 (clang-format: adjust line break penalties, 2017-09-29)
and another time in 5e9fa0f9fa (clang-format: re-adjust line break
penalties, 2024-10-18). While these settings help tweak the line break
penalties to be more in-line with the requirements of the Git project,
using 'clang-format' still produces a lot of false positives.

So to make 'clang-format' more usable, set the 'ColumnLimit' to 0. This
means that line-wrapping is no-longer a concern of the formatter and
something that the user needs to take care of. The previous commit also
added a more flexible guideline to the '.editorconfig' setting a
'max_line_length' of 120 characters. This should provide some guidance
to users.

In the future, it would be nice to re-instate this limit with adequate
penalties which would follow our guidelines, but currently, it makes
more sense to have a working formatter which we can rely on and which
doesn't create too many false positives.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 .clang-format | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/.clang-format b/.clang-format
index 9547fe1b77..19d6cf4200 100644
--- a/.clang-format
+++ b/.clang-format
@@ -12,7 +12,15 @@ UseTab: Always
 TabWidth: 8
 IndentWidth: 8
 ContinuationIndentWidth: 8
-ColumnLimit: 80
+
+# While we do want to enforce a character limit of 80 characters, we often
+# allow lines to overflow that limit to prioritize readability. Setting a
+# character limit here with penalties has been finicky and creates too many
+# false positives.
+#
+# NEEDSWORK: It would be nice if we can find optimal settings to ensure we
+# can re-enable the limit here.
+ColumnLimit: 0
 
 # C Language specifics
 Language: Cpp
@@ -210,16 +218,5 @@ MaxEmptyLinesToKeep: 1
 # No empty line at the start of a block.
 KeepEmptyLinesAtTheStartOfBlocks: false
 
-# Penalties
-# This decides what order things should be done if a line is too long
-PenaltyBreakAssignment: 5
-PenaltyBreakBeforeFirstCallParameter: 5
-PenaltyBreakComment: 5
-PenaltyBreakFirstLessLess: 0
-PenaltyBreakOpenParenthesis: 300
-PenaltyBreakString: 5
-PenaltyExcessCharacter: 10
-PenaltyReturnTypeOnItsOwnLine: 300
-
 # Don't sort #include's
 SortIncludes: false

-- 
2.49.0

[PATCH v3 2/3] clang-format: add 'RemoveBracesLLVM' to the main config

[Karthik Nayak]

In 1b8f306612 (ci/style-check: add `RemoveBracesLLVM` in CI job,
2024-07-23) we added 'RemoveBracesLLVM' to the CI job of running the
clang formatter.

This rule checks and warns against using braces on simple
single-statement bodies of statements. Since we haven't had any issues
regarding this rule, we can now move it into the main clang-format
config and remove it from being CI exclusive.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 .clang-format         |  6 ++++++
 ci/run-style-check.sh | 18 +-----------------
 2 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/.clang-format b/.clang-format
index 19d6cf4200..dcfd0aad60 100644
--- a/.clang-format
+++ b/.clang-format
@@ -220,3 +220,9 @@ KeepEmptyLinesAtTheStartOfBlocks: false
 
 # Don't sort #include's
 SortIncludes: false
+
+# Remove optional braces of control statements (if, else, for, and while)
+# according to the LLVM coding style. This avoids braces on simple
+# single-statement bodies of statements but keeps braces if one side of
+# if/else if/.../else cascade has multi-statement body.
+RemoveBracesLLVM: true
diff --git a/ci/run-style-check.sh b/ci/run-style-check.sh
index 6cd4b1d934..0832c19df0 100755
--- a/ci/run-style-check.sh
+++ b/ci/run-style-check.sh
@@ -5,21 +5,5 @@
 
 baseCommit=$1
 
-# Remove optional braces of control statements (if, else, for, and while)
-# according to the LLVM coding style. This avoids braces on simple
-# single-statement bodies of statements but keeps braces if one side of
-# if/else if/.../else cascade has multi-statement body.
-#
-# As this rule comes with a warning [1], we want to experiment with it
-# before adding it in-tree. since the CI job for the style check is allowed
-# to fail, appending the rule here allows us to validate its efficacy.
-# While also ensuring that end-users are not affected directly.
-#
-# [1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm
-{
-	cat .clang-format
-	echo "RemoveBracesLLVM: true"
-} >/tmp/clang-format-rules
-
-git clang-format --style=file:/tmp/clang-format-rules \
+git clang-format --style=file:.clang-format \
 	--diff --extensions c,h "$baseCommit"

-- 
2.49.0

[PATCH v3 3/3] meson: add rule to run 'git clang-format'

[Karthik Nayak]

The Makefile has a 'style' rule to run 'git clang-format'. While Meson
intrinsically supports a 'clang-format' target, which can be run when
using the ninja backend by running 'ninja clang-format', this runs the
formatting on all existing files.

Our Meson build doesn't yet support a way to run 'git clang-format',
which runs the formatter between the working directory and commit
provided. Add a new 'style' target to Meson to mimic the target in the
Makefile.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 meson.build | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/meson.build b/meson.build
index 7fea4a34d6..48dff818e0 100644
--- a/meson.build
+++ b/meson.build
@@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'
   alias_target('check-headers', hdr_check)
 endif
 
+git_clang_format = find_program('git-clang-format', required: false, native: true)
+if git_clang_format.found()
+  run_target('style',
+    command: [
+      git_clang_format,
+      '--style', 'file',
+      '--diff',
+      '--extensions', 'c,h'
+    ]
+  )
+endif
+
 foreach key, value : {
   'DIFF': diff.full_path(),
   'GIT_SOURCE_DIR': meson.project_source_root(),

-- 
2.49.0