{"thread":{"id":"63693","subject":"[PATCH 1/4] editorconfig: set maximum line length to 120 characters","startedAt":"2025-06-25T16:43:47Z","lastAt":"2025-07-02T09:23:30Z","messageCount":36,"participants":["Karthik Nayak","Junio C Hamano","Justin Tobler","Patrick Steinhardt","Toon Claes"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"520716","messageId":"20250625-525-make-clang-format-more-robust-v1-1-67a49ecc2fd5@gmail.com","threadId":"63693","inReplyTo":"20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com","subject":"[PATCH 1/4] editorconfig: set maximum line length to 120 characters","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-25T16:43:26Z","receivedAt":"2025-06-25T16:43:47Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"As per 'Documentation/CodingGuidelines', we try to keep to at most 80\ncharacters per line. However, there are often certain cases where we\nextend this for the sake of readability.\n\nAdd a maximum limit of 120 characters to the '.editorconfig'. This means\nthat if an individual line exceeds 120 characters, the editor will wrap\nthat line. This provides a lot wiggle room over the recommended 80\ncharacter limit.\n\nContrary to settings within '.clang-format' which are used for\nstatically formatting source code, the '.editorconfig' rules are hints\nto the editor. These are not enforced by CI and are guidelines for\neditors to follow. As such, the 'max_line_length' used here is only\nsupported in a set of editors [1].\n\n[1]: https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n .editorconfig | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/.editorconfig b/.editorconfig\nindex 2d3929b591..d0f940fd23 100644\n--- a/.editorconfig\n+++ b/.editorconfig\n@@ -7,6 +7,7 @@ insert_final_newline = true\n [{*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,Makefile}]\n indent_style = tab\n tab_width = 8\n+max_line_length = 120\n \n [*.py]\n indent_style = space\n\n-- \n2.49.0\n\n"},{"id":"520717","messageId":"20250625-525-make-clang-format-more-robust-v1-2-67a49ecc2fd5@gmail.com","threadId":"63693","inReplyTo":"20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com","subject":"[PATCH 2/4] clang-format: set 'ColumnLimit' to 0","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-25T16:43:27Z","receivedAt":"2025-06-25T16:43:48Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"When clang-format was introduced to the Git project in\n6134de6ac1 (clang-format: outline the git project's coding style,\n2017-08-14), the 'ColumnLimit' was set to 80. This is inline with our\nrecommendation in 'Documentation/CodingGuidelines', which states:\n\n  We try to keep to at most 80 characters per line.\n\nHowever while this is recommended limit, this is not the enforced\nlimit. In some cases in we do overflow this limit to prioritize\nreadability. Setting the 'ColumnLimit' also means that shorter lines are\nconcatenated to simply as the result would still be below 80 characters,\nwhich is undesirable.\n\nIn the past, we tried to adjust the penalties around line wrapping, once\nin 42efde4c29 (clang-format: adjust line break penalties, 2017-09-29)\nand another time in 5e9fa0f9fa (clang-format: re-adjust line break\npenalties, 2024-10-18). While these settings help tweak the line break\npenalties to be more in-line with the requirements of the Git project,\nusing 'clang-format' still produces a lot of false positives.\n\nSo to make 'clang-format' more usable, set the 'ColumnLimit' to 0. This\nmeans that line-wrapping is no-longer a concern of the formatter and\nsomething that the user needs to take care of. The previous commit also\nadded a more flexible guideline to the '.editorconfig' setting a\n'max_line_length' of 120 characters. This should provide some guidance\nto users.\n\nIn the future, it would be nice to re-instate this limit with adequate\npenalties which would follow our guidelines, but currently, it makes\nmore sense to have a working formatter which we can rely on and which\ndoesn't create too many false positives.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n .clang-format | 21 +++++++++------------\n 1 file changed, 9 insertions(+), 12 deletions(-)\n\ndiff --git a/.clang-format b/.clang-format\nindex 9547fe1b77..19d6cf4200 100644\n--- a/.clang-format\n+++ b/.clang-format\n@@ -12,7 +12,15 @@ UseTab: Always\n TabWidth: 8\n IndentWidth: 8\n ContinuationIndentWidth: 8\n-ColumnLimit: 80\n+\n+# While we do want to enforce a character limit of 80 characters, we often\n+# allow lines to overflow that limit to prioritize readability. Setting a\n+# character limit here with penalties has been finicky and creates too many\n+# false positives.\n+#\n+# NEEDSWORK: It would be nice if we can find optimal settings to ensure we\n+# can re-enable the limit here.\n+ColumnLimit: 0\n \n # C Language specifics\n Language: Cpp\n@@ -210,16 +218,5 @@ MaxEmptyLinesToKeep: 1\n # No empty line at the start of a block.\n KeepEmptyLinesAtTheStartOfBlocks: false\n \n-# Penalties\n-# This decides what order things should be done if a line is too long\n-PenaltyBreakAssignment: 5\n-PenaltyBreakBeforeFirstCallParameter: 5\n-PenaltyBreakComment: 5\n-PenaltyBreakFirstLessLess: 0\n-PenaltyBreakOpenParenthesis: 300\n-PenaltyBreakString: 5\n-PenaltyExcessCharacter: 10\n-PenaltyReturnTypeOnItsOwnLine: 300\n-\n # Don't sort #include's\n SortIncludes: false\n\n-- \n2.49.0\n\n"},{"id":"520718","messageId":"20250625-525-make-clang-format-more-robust-v1-3-67a49ecc2fd5@gmail.com","threadId":"63693","inReplyTo":"20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com","subject":"[PATCH 3/4] clang-format: add 'RemoveBracesLLVM' to the main config","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-25T16:43:28Z","receivedAt":"2025-06-25T16:43:49Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In 1b8f306612 (ci/style-check: add `RemoveBracesLLVM` in CI job,\n2024-07-23) we added 'RemoveBracesLLVM' to the CI job of running the\nclang formatter.\n\nThis rule checks and warns against using braces on simple\nsingle-statement bodies of statements. Since we haven't had any issues\nregarding this rule, we can now move it into the main clang-format\nconfig and remove it from being CI exclusive.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n .clang-format         |  6 ++++++\n ci/run-style-check.sh | 18 +-----------------\n 2 files changed, 7 insertions(+), 17 deletions(-)\n\ndiff --git a/.clang-format b/.clang-format\nindex 19d6cf4200..dcfd0aad60 100644\n--- a/.clang-format\n+++ b/.clang-format\n@@ -220,3 +220,9 @@ KeepEmptyLinesAtTheStartOfBlocks: false\n \n # Don't sort #include's\n SortIncludes: false\n+\n+# Remove optional braces of control statements (if, else, for, and while)\n+# according to the LLVM coding style. This avoids braces on simple\n+# single-statement bodies of statements but keeps braces if one side of\n+# if/else if/.../else cascade has multi-statement body.\n+RemoveBracesLLVM: true\ndiff --git a/ci/run-style-check.sh b/ci/run-style-check.sh\nindex 6cd4b1d934..0832c19df0 100755\n--- a/ci/run-style-check.sh\n+++ b/ci/run-style-check.sh\n@@ -5,21 +5,5 @@\n \n baseCommit=$1\n \n-# Remove optional braces of control statements (if, else, for, and while)\n-# according to the LLVM coding style. This avoids braces on simple\n-# single-statement bodies of statements but keeps braces if one side of\n-# if/else if/.../else cascade has multi-statement body.\n-#\n-# As this rule comes with a warning [1], we want to experiment with it\n-# before adding it in-tree. since the CI job for the style check is allowed\n-# to fail, appending the rule here allows us to validate its efficacy.\n-# While also ensuring that end-users are not affected directly.\n-#\n-# [1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm\n-{\n-\tcat .clang-format\n-\techo \"RemoveBracesLLVM: true\"\n-} >/tmp/clang-format-rules\n-\n-git clang-format --style=file:/tmp/clang-format-rules \\\n+git clang-format --style=file:.clang-format \\\n \t--diff --extensions c,h \"$baseCommit\"\n\n-- \n2.49.0\n\n"},{"id":"520719","messageId":"20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com","threadId":"63693","inReplyTo":null,"subject":"[PATCH 0/4] clang-format: modify rules to reduce false-positives","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-25T16:43:25Z","receivedAt":"2025-06-25T16:43:49Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"This series is in response to an email thread [1] around the usage of\n'.clang-format' and its effectiveness.\n\nThe goal of the series is to improve the usage of 'clang-format' in the\nrepository. To do this we:\n1. Reduce the number of false positives. Majority of which is due to\n   line-wrapping. We remove the 'ColumnLimit' from 'clang-format'. This\n   removes the responsibility of line-wrapping from 'clang-format' and puts\n   it into the hands of the user.\n2. Add a 120 character limit to the '.editorconfig' to ensure that\n   editors wrap lines beyond that.\n3. Add a rule to 'meson' to run 'git clang-format' by running 'meson\n   compile style'.\n4. Make the 'RemoveBracesLLVM' permanent by moving it to\n   '.clang-format'.\n\nWith this, running `git clang-format` for the last 25 commits in master,\nseems to produce much less false positives.\n\n  diff --git a/builtin/diff.c b/builtin/diff.c\n  index c6231edce4..246b81caa2 100644\n  --- a/builtin/diff.c\n  +++ b/builtin/diff.c\n  @@ -30,14 +30,13 @@\n   #define DIFF_NO_INDEX_IMPLICIT 2\n\n   static const char builtin_diff_usage[] =\n  -\"git diff [<options>] [<commit>] [--] [<path>...]\\n\"\n  -\"   or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\\n\"\n  -\"   or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\\n\"\n  -\"   or: git diff [<options>] <commit>...<commit> [--] [<path>...]\\n\"\n  -\"   or: git diff [<options>] <blob> <blob>\\n\"\n  -\"   or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]\"\n  -\"\\n\"\n  -COMMON_DIFF_OPTIONS_HELP;\n  +\t\"git diff [<options>] [<commit>] [--] [<path>...]\\n\"\n  +\t\"   or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\\n\"\n  +\t\"   or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\\n\"\n  +\t\"   or: git diff [<options>] <commit>...<commit> [--] [<path>...]\\n\"\n  +\t\"   or: git diff [<options>] <blob> <blob>\\n\"\n  +\t\"   or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]\"\n  +\t\"\\n\" COMMON_DIFF_OPTIONS_HELP;\n\n   static const char *blob_path(struct object_array_entry *entry)\n   {\n  diff --git a/builtin/stash.c b/builtin/stash.c\n  index 7cd3ad8aa4..90e441a6e5 100644\n  --- a/builtin/stash.c\n  +++ b/builtin/stash.c\n  @@ -1802,7 +1802,8 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n\n \t\t  argc = parse_options(argc, argv, prefix, options,\n \t\t\t\t       push_assumed ? git_stash_usage :\n  -\t\t\t\t     git_stash_push_usage, flags);\n  +\t\t\t\t\t\t    git_stash_push_usage,\n  +\t\t\t\t     flags);\n \t\t  force_assume |= patch_mode;\n \t  }\n\n  diff --git a/bundle-uri.c b/bundle-uri.c\n  index c9d65aa0ce..89f59aafe8 100644\n  --- a/bundle-uri.c\n  +++ b/bundle-uri.c\n  @@ -123,7 +123,7 @@ void print_bundle_list(FILE *fp, struct bundle_list *list)\n \t\t  for (i = 0; i < BUNDLE_HEURISTIC__COUNT; i++) {\n \t\t\t  if (heuristics[i].heuristic == list->heuristic) {\n \t\t\t\t  fprintf(fp, \"\\theuristic = %s\\n\",\n  -\t\t\t\t       heuristics[list->heuristic].name);\n  +\t\t\t\t\theuristics[list->heuristic].name);\n \t\t\t\t  break;\n \t\t\t  }\n \t\t  }\n  diff --git a/diff-no-index.c b/diff-no-index.c\n  index 4aeeb98cfa..a3892a9ccc 100644\n  --- a/diff-no-index.c\n  +++ b/diff-no-index.c\n  @@ -325,7 +325,7 @@ static int fixup_paths(const char **path, struct strbuf *replacement)\n \t  return 0;\n   }\n\n  -static const char * const diff_no_index_usage[] = {\n  +static const char *const diff_no_index_usage[] = {\n \t  N_(\"git diff --no-index [<options>] <path> <path> [<pathspec>...]\"),\n \t  NULL\n   };\n  diff --git a/pathspec.h b/pathspec.h\n  index 5e3a6f1fe7..601b9ca201 100644\n  --- a/pathspec.h\n  +++ b/pathspec.h\n  @@ -80,7 +80,7 @@ struct pathspec {\n    * For git diff --no-index, indicate that we are operating without\n    * a repository or index.\n    */\n  -#define PATHSPEC_NO_REPOSITORY (1<<7)\n  +#define PATHSPEC_NO_REPOSITORY (1 << 7)\n\n   /**\n    * Given command line arguments and a prefix, convert the input to\n\nWhile now I'm tempted to mark the 'check-style' CI job as required. I\nthink we should do that in the future.\n\n[1]: https://lore.kernel.org/git/xmqqmsa3adpw.fsf@gitster.g/\n\n---\n .clang-format         | 27 +++++++++++++++------------\n .editorconfig         |  1 +\n ci/run-style-check.sh | 18 +-----------------\n meson.build           | 12 ++++++++++++\n 4 files changed, 29 insertions(+), 29 deletions(-)\n\nKarthik Nayak (4):\n      editorconfig: set maximum line length to 120 characters\n      clang-format: set 'ColumnLimit' to 0\n      clang-format: add 'RemoveBracesLLVM' to the main config\n      meson: add rule to run 'git clang-format'\n\n\n\nbase-commit: f0135a9047ca37d4d117dcf21f7e3e89fad85d00\nchange-id: 20250625-525-make-clang-format-more-robust-968126c991b9\n\nThanks\n- Karthik\n\n"},{"id":"520720","messageId":"20250625-525-make-clang-format-more-robust-v1-4-67a49ecc2fd5@gmail.com","threadId":"63693","inReplyTo":"20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com","subject":"[PATCH 4/4] meson: add rule to run 'git clang-format'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-25T16:43:29Z","receivedAt":"2025-06-25T16:43:50Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The Makefile has a 'style' rule to run 'git clang-format'. While Meson\nintrinsically supports a 'clang-format' target, which can be run when\nusing the ninja backend by running 'ninja clang-format', this runs the\nformatting on all existing files.\n\nOur Meson build doesn't yet support a way to run 'git clang-format',\nwhich runs the formatter between the working directory and commit\nprovided. Add a new 'style' target to Meson to mimic the target in the\nMakefile.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n meson.build | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/meson.build b/meson.build\nindex 7fea4a34d6..578db26df2 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'\n   alias_target('check-headers', hdr_check)\n endif\n \n+clang_format = find_program('clang-format', required: false)\n+if clang_format.found()\n+  run_target('style',\n+    command: [\n+      'git', 'clang-format',\n+      '--style', 'file',\n+      '--diff',\n+      '--extensions', 'c,h'\n+    ]\n+  )\n+endif\n+\n foreach key, value : {\n   'DIFF': diff.full_path(),\n   'GIT_SOURCE_DIR': meson.project_source_root(),\n\n-- \n2.49.0\n\n"},{"id":"520727","messageId":"xmqqo6ubu0ic.fsf@gitster.g","threadId":"63693","inReplyTo":"20250625-525-make-clang-format-more-robust-v1-1-67a49ecc2fd5@gmail.com","subject":"Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-25T18:41:47Z","receivedAt":"2025-06-25T18:41:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> As per 'Documentation/CodingGuidelines', we try to keep to at most 80\n> characters per line. However, there are often certain cases where we\n> extend this for the sake of readability.\n>\n> Add a maximum limit of 120 characters to the '.editorconfig'. This means\n> that if an individual line exceeds 120 characters, the editor will wrap\n> that line. This provides a lot wiggle room over the recommended 80\n> character limit.\n\nIdeally \"when the line is overly long to be more than 120 columns,\nplease wrap it to 80 columns or less\" is what we want.  If the\nresult of formatting a single 125 column line leaves us with two\nlines, one with 100 columns and another with 25 columns, this would\nnot be very useful.  As this is meant to give suggestions without\nenforcing hard rule, wouldn't it make more sense to set it to 80?\n\nI dunno.\n\n> Contrary to settings within '.clang-format' which are used for\n> statically formatting source code, the '.editorconfig' rules are hints\n> to the editor. These are not enforced by CI and are guidelines for\n> editors to follow. As such, the 'max_line_length' used here is only\n> supported in a set of editors [1].\n>\n> [1]: https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length\n\nThanks.\n\nThe discussion around \"rulers\" https://github.com/editorconfig/editorconfig/issues/89\nwas also interesting.\n\n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n>  .editorconfig | 1 +\n>  1 file changed, 1 insertion(+)\n>\n> diff --git a/.editorconfig b/.editorconfig\n> index 2d3929b591..d0f940fd23 100644\n> --- a/.editorconfig\n> +++ b/.editorconfig\n> @@ -7,6 +7,7 @@ insert_final_newline = true\n>  [{*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,Makefile}]\n>  indent_style = tab\n>  tab_width = 8\n> +max_line_length = 120\n>  \n>  [*.py]\n>  indent_style = space\n"},{"id":"520744","messageId":"CAOLa=ZRu-fBsNpoSzb8z2xUfM7WUUcdR1XRd8zr1U+P61QRccA@mail.gmail.com","threadId":"63693","inReplyTo":"xmqqo6ubu0ic.fsf@gitster.g","subject":"Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-26T08:27:59Z","receivedAt":"2025-06-26T08:28:01Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> As per 'Documentation/CodingGuidelines', we try to keep to at most 80\n>> characters per line. However, there are often certain cases where we\n>> extend this for the sake of readability.\n>>\n>> Add a maximum limit of 120 characters to the '.editorconfig'. This means\n>> that if an individual line exceeds 120 characters, the editor will wrap\n>> that line. This provides a lot wiggle room over the recommended 80\n>> character limit.\n>\n> Ideally \"when the line is overly long to be more than 120 columns,\n> please wrap it to 80 columns or less\" is what we want.\n>\n\nYup, this would be nice, but neither '.editorconfig' or '.clang-format'\nsupport a rule like this.\n\n> If the\n> result of formatting a single 125 column line leaves us with two\n> lines, one with 100 columns and another with 25 columns, this would\n> not be very useful.  As this is meant to give suggestions without\n> enforcing hard rule, wouldn't it make more sense to set it to 80?\n>\n> I dunno.\n>\n\nSo my intent was to instead was to allow the user to be in charge of\nline-wrapping, but for no reason should that go beyond 120 columns.\n\nI'm happy to change that to 80 columns, this does mean that supported\neditors will start wrapping at 80 columns. Users will have to override\nas necessary.\n\n>> Contrary to settings within '.clang-format' which are used for\n>> statically formatting source code, the '.editorconfig' rules are hints\n>> to the editor. These are not enforced by CI and are guidelines for\n>> editors to follow. As such, the 'max_line_length' used here is only\n>> supported in a set of editors [1].\n>>\n>> [1]: https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length\n>\n> Thanks.\n>\n> The discussion around \"rulers\" https://github.com/editorconfig/editorconfig/issues/89\n> was also interesting.\n>\n\nIndeed. That would be a really nice feature and in line with what we\nwould want.\n\n>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n>> ---\n>>  .editorconfig | 1 +\n>>  1 file changed, 1 insertion(+)\n>>\n>> diff --git a/.editorconfig b/.editorconfig\n>> index 2d3929b591..d0f940fd23 100644\n>> --- a/.editorconfig\n>> +++ b/.editorconfig\n>> @@ -7,6 +7,7 @@ insert_final_newline = true\n>>  [{*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,Makefile}]\n>>  indent_style = tab\n>>  tab_width = 8\n>> +max_line_length = 120\n>>\n>>  [*.py]\n>>  indent_style = space\n"},{"id":"520761","messageId":"xmqqbjqar35m.fsf@gitster.g","threadId":"63693","inReplyTo":"CAOLa=ZRu-fBsNpoSzb8z2xUfM7WUUcdR1XRd8zr1U+P61QRccA@mail.gmail.com","subject":"Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-26T14:25:09Z","receivedAt":"2025-06-26T14:25:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> So my intent was to instead was to allow the user to be in charge of\n> line-wrapping, but for no reason should that go beyond 120 columns.\n>\n> I'm happy to change that to 80 columns, this does mean that supported\n> editors will start wrapping at 80 columns. Users will have to override\n> as necessary.\n\nOK.  So stepping back a bit\n\n 1. We advise people to avoid exceeding 80 columns\n\n 2. A line can be easier to read without wrapping strictly at 80\n    columns but left as a single line, slightly going above the\n    limit..\n\n 3. Even with the second observation above, a line that is way\n    longer than 80 columns is not acceptably long.\n\nNow, what is the line between the #2 and #3?  If we set these \"hard\nlimit\" to that number, as long as the tool does not pack multiple\nshorter lines into one line using that number as a limit, we'd be at\na happy place, I would imagine?\n\nThanks.\n"},{"id":"520771","messageId":"qxewasf6rxt6mnbwvfhxarcbvfsn6cnc2jskpddsb5fhxfhpwq@5d727iud7wfp","threadId":"63693","inReplyTo":"20250625-525-make-clang-format-more-robust-v1-1-67a49ecc2fd5@gmail.com","subject":"Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-06-26T16:22:44Z","receivedAt":"2025-06-26T16:28:08Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/06/25 06:43PM, Karthik Nayak wrote:\n> As per 'Documentation/CodingGuidelines', we try to keep to at most 80\n> characters per line. However, there are often certain cases where we\n> extend this for the sake of readability.\n> \n> Add a maximum limit of 120 characters to the '.editorconfig'. This means\n> that if an individual line exceeds 120 characters, the editor will wrap\n> that line. This provides a lot wiggle room over the recommended 80\n> character limit.\n\nI frequently use the format operator in vim to reformat entire blocks of\ntext and it is commonly configured to use `max_line_length` from an\n`.editorconfig` file to know when to wrap lines. Changing the value to\n120 would cause my editor to prefer 120 character lines when\nreformatting, which I would personally not like.\n\nBeing that `max_line_length` is only a suggestion for the editor, I\nthink we may be better off setting it to 80 characters or leaving it\nunset entirely.\n\n-Justin\n\n> Contrary to settings within '.clang-format' which are used for\n> statically formatting source code, the '.editorconfig' rules are hints\n> to the editor. These are not enforced by CI and are guidelines for\n> editors to follow. As such, the 'max_line_length' used here is only\n> supported in a set of editors [1].\n> \n> [1]: https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length\n> \n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n"},{"id":"520773","messageId":"iutm4xxkhchcpfygtqo2s4nit42sclzen63465ljtovzgdsebr@okpj5jtw62r6","threadId":"63693","inReplyTo":"20250625-525-make-clang-format-more-robust-v1-4-67a49ecc2fd5@gmail.com","subject":"Re: [PATCH 4/4] meson: add rule to run 'git clang-format'","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-06-26T16:35:37Z","receivedAt":"2025-06-26T16:41:01Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/06/25 06:43PM, Karthik Nayak wrote:\n> The Makefile has a 'style' rule to run 'git clang-format'. While Meson\n> intrinsically supports a 'clang-format' target, which can be run when\n> using the ninja backend by running 'ninja clang-format', this runs the\n> formatting on all existing files.\n> \n> Our Meson build doesn't yet support a way to run 'git clang-format',\n> which runs the formatter between the working directory and commit\n> provided. Add a new 'style' target to Meson to mimic the target in the\n> Makefile.\n> \n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n>  meson.build | 12 ++++++++++++\n>  1 file changed, 12 insertions(+)\n> \n> diff --git a/meson.build b/meson.build\n> index 7fea4a34d6..578db26df2 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'\n>    alias_target('check-headers', hdr_check)\n>  endif\n>  \n> +clang_format = find_program('clang-format', required: false)\n\nShould we be checking for `git-clang-format` instead?\n\n> +if clang_format.found()\n> +  run_target('style',\n> +    command: [\n> +      'git', 'clang-format',\n> +      '--style', 'file',\n> +      '--diff',\n> +      '--extensions', 'c,h'\n> +    ]\n> +  )\n> +endif\n> +\n>  foreach key, value : {\n>    'DIFF': diff.full_path(),\n>    'GIT_SOURCE_DIR': meson.project_source_root(),\n> \n> -- \n> 2.49.0\n> \n> \n"},{"id":"520798","messageId":"CAOLa=ZSJH-Wji+-oU+ku5aojYzEuC96tUu+24UTuTxCxt9LYNQ@mail.gmail.com","threadId":"63693","inReplyTo":"iutm4xxkhchcpfygtqo2s4nit42sclzen63465ljtovzgdsebr@okpj5jtw62r6","subject":"Re: [PATCH 4/4] meson: add rule to run 'git clang-format'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-27T08:12:11Z","receivedAt":"2025-06-27T08:12:15Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> On 25/06/25 06:43PM, Karthik Nayak wrote:\n>> The Makefile has a 'style' rule to run 'git clang-format'. While Meson\n>> intrinsically supports a 'clang-format' target, which can be run when\n>> using the ninja backend by running 'ninja clang-format', this runs the\n>> formatting on all existing files.\n>>\n>> Our Meson build doesn't yet support a way to run 'git clang-format',\n>> which runs the formatter between the working directory and commit\n>> provided. Add a new 'style' target to Meson to mimic the target in the\n>> Makefile.\n>>\n>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n>> ---\n>>  meson.build | 12 ++++++++++++\n>>  1 file changed, 12 insertions(+)\n>>\n>> diff --git a/meson.build b/meson.build\n>> index 7fea4a34d6..578db26df2 100644\n>> --- a/meson.build\n>> +++ b/meson.build\n>> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'\n>>    alias_target('check-headers', hdr_check)\n>>  endif\n>>\n>> +clang_format = find_program('clang-format', required: false)\n>\n> Should we be checking for `git-clang-format` instead?\n>\n\nYeah. While `git-clang-format` is packaged with `clang-format`, it does\nmake more sense to check for the former.\n\nThanks\n"},{"id":"520800","messageId":"CAOLa=ZRrBS9Jzf_p_cLOhB+meSTmQ8=MF67A_tihLZ-qpj42Aw@mail.gmail.com","threadId":"63693","inReplyTo":"xmqqbjqar35m.fsf@gitster.g","subject":"Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-27T08:38:22Z","receivedAt":"2025-06-27T08:38:24Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> So my intent was to instead was to allow the user to be in charge of\n>> line-wrapping, but for no reason should that go beyond 120 columns.\n>>\n>> I'm happy to change that to 80 columns, this does mean that supported\n>> editors will start wrapping at 80 columns. Users will have to override\n>> as necessary.\n>\n> OK.  So stepping back a bit\n>\n>  1. We advise people to avoid exceeding 80 columns\n>\n>  2. A line can be easier to read without wrapping strictly at 80\n>     columns but left as a single line, slightly going above the\n>     limit..\n>\n>  3. Even with the second observation above, a line that is way\n>     longer than 80 columns is not acceptably long.\n>\n> Now, what is the line between the #2 and #3?  If we set these \"hard\n> limit\" to that number, as long as the tool does not pack multiple\n> shorter lines into one line using that number as a limit, we'd be at\n> a happy place, I would imagine?\n>\n\nSpot on. Doing some backhand analysis on our code base:\n\nTotal lines: 2136626\nMax length: 904\nAverage length: 36.2\nMedian length: 32.0\n90th percentile: 73.0\n95th percentile: 79.0\n\nLines > 80 chars: 91650 (4.3%)\nLines > 100 chars: 54589 (2.6%)\nLines > 120 chars: 35529 (1.7%)\n\nThe `max_line_length` in '.editorconfig' is stated as:\n\n  Forces hard line wrapping after the amount of characters specified.\n  unset to turn off this feature (use the editor settings).\n\nSo it doesn't force packing shorter lines together, rather, only wraps\nlines above the specified limit. This limit can be thought of the limit\nbeyond which a line is of unacceptable length. We can move this lower to\nperhaps 100 or even 80, but that would force line wrapping at 80\ncharacters. Which can be great, but there are situations like you\nmentioned where we want to extend.\n\n> Thanks.\n"},{"id":"520803","messageId":"CAOLa=ZR_ZhjxYgN4pZsjOuC52PZjGMCzLJVZbwGQb=Q9W-0HSQ@mail.gmail.com","threadId":"63693","inReplyTo":"qxewasf6rxt6mnbwvfhxarcbvfsn6cnc2jskpddsb5fhxfhpwq@5d727iud7wfp","subject":"Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-27T08:51:16Z","receivedAt":"2025-06-27T08:51:17Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> On 25/06/25 06:43PM, Karthik Nayak wrote:\n>> As per 'Documentation/CodingGuidelines', we try to keep to at most 80\n>> characters per line. However, there are often certain cases where we\n>> extend this for the sake of readability.\n>>\n>> Add a maximum limit of 120 characters to the '.editorconfig'. This means\n>> that if an individual line exceeds 120 characters, the editor will wrap\n>> that line. This provides a lot wiggle room over the recommended 80\n>> character limit.\n>\n\nHello Justin,\n\n> I frequently use the format operator in vim to reformat entire blocks of\n> text and it is commonly configured to use `max_line_length` from an\n> `.editorconfig` file to know when to wrap lines. Changing the value to\n> 120 would cause my editor to prefer 120 character lines when\n> reformatting, which I would personally not like.\n>\n\nIt would only wrap lines longer than 120 columns. Currently editorconfig\ndoesn't wrap any line length. So we're essentially saying, any line\nabove 120 is not something we want to accept and hence wrap. This\ndoesn't mean that shorter lines will be combined together. Wouldn't this\nbe better than the current situation?\n\n> Being that `max_line_length` is only a suggestion for the editor, I\n> think we may be better off setting it to 80 characters or leaving it\n> unset entirely.\n>\n> -Justin\n>\n>> Contrary to settings within '.clang-format' which are used for\n>> statically formatting source code, the '.editorconfig' rules are hints\n>> to the editor. These are not enforced by CI and are guidelines for\n>> editors to follow. As such, the 'max_line_length' used here is only\n>> supported in a set of editors [1].\n>>\n>> [1]: https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length\n>>\n>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n"},{"id":"520809","messageId":"xmqqtt418dnw.fsf@gitster.g","threadId":"63693","inReplyTo":"CAOLa=ZSJH-Wji+-oU+ku5aojYzEuC96tUu+24UTuTxCxt9LYNQ@mail.gmail.com","subject":"Re: [PATCH 4/4] meson: add rule to run 'git clang-format'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-27T14:25:23Z","receivedAt":"2025-06-27T14:25:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n>>> diff --git a/meson.build b/meson.build\n>>> index 7fea4a34d6..578db26df2 100644\n>>> --- a/meson.build\n>>> +++ b/meson.build\n>>> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'\n>>>    alias_target('check-headers', hdr_check)\n>>>  endif\n>>>\n>>> +clang_format = find_program('clang-format', required: false)\n>>\n>> Should we be checking for `git-clang-format` instead?\n>>\n>\n> Yeah. While `git-clang-format` is packaged with `clang-format`, it does\n> make more sense to check for the former.\n\nJust for my education, what does find_program() look for?  Installed\npackages, or a program on your $PATH?  I am guessing that the answer\nis the latter, in which case it is not like \"it makes more sense to\ncheck for git-clang-format\"---rather it is \"it would not work at all\nif we looked for clang-format\", no?\n\nThanks.\n"},{"id":"520811","messageId":"4xcovwt5f2garsngqtzzcrn7755in4xkq6zxxk5smeo4d2vfp4@l7cqb2lizf65","threadId":"63693","inReplyTo":"CAOLa=ZR_ZhjxYgN4pZsjOuC52PZjGMCzLJVZbwGQb=Q9W-0HSQ@mail.gmail.com","subject":"Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-06-27T15:12:43Z","receivedAt":"2025-06-27T15:18:10Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/06/27 01:51AM, Karthik Nayak wrote:\n> Justin Tobler <jltobler@gmail.com> writes:\n> \n> > On 25/06/25 06:43PM, Karthik Nayak wrote:\n> >> As per 'Documentation/CodingGuidelines', we try to keep to at most 80\n> >> characters per line. However, there are often certain cases where we\n> >> extend this for the sake of readability.\n> >>\n> >> Add a maximum limit of 120 characters to the '.editorconfig'. This means\n> >> that if an individual line exceeds 120 characters, the editor will wrap\n> >> that line. This provides a lot wiggle room over the recommended 80\n> >> character limit.\n> >\n> \n> Hello Justin,\n> \n> > I frequently use the format operator in vim to reformat entire blocks of\n> > text and it is commonly configured to use `max_line_length` from an\n> > `.editorconfig` file to know when to wrap lines. Changing the value to\n> > 120 would cause my editor to prefer 120 character lines when\n> > reformatting, which I would personally not like.\n> >\n> \n> It would only wrap lines longer than 120 columns. Currently editorconfig\n> doesn't wrap any line length. So we're essentially saying, any line\n> above 120 is not something we want to accept and hence wrap. This\n> doesn't mean that shorter lines will be combined together. Wouldn't this\n> be better than the current situation?\n\nWhen `max_line_length` is set in a \".editorconfig\" file, in my vim\neditor it overrides the `textwidth` configuration which was already set\nto 80 by default. So changing to 120 would change line wrapping behavior\nfor me at least. I could disable using the \".editorconfig\", but I would\nprefer to avoid doing that :)\n\n-Justin\n"},{"id":"520901","messageId":"CAOLa=ZTVgEFSES0iv2kLftsDJ27cULo1F1XuJLHURWimuOm5-g@mail.gmail.com","threadId":"63693","inReplyTo":"4xcovwt5f2garsngqtzzcrn7755in4xkq6zxxk5smeo4d2vfp4@l7cqb2lizf65","subject":"Re: [PATCH 1/4] editorconfig: set maximum line length to 120 characters","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-30T08:29:56Z","receivedAt":"2025-06-30T08:29:57Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> On 25/06/27 01:51AM, Karthik Nayak wrote:\n>> Justin Tobler <jltobler@gmail.com> writes:\n>>\n>> > On 25/06/25 06:43PM, Karthik Nayak wrote:\n>> >> As per 'Documentation/CodingGuidelines', we try to keep to at most 80\n>> >> characters per line. However, there are often certain cases where we\n>> >> extend this for the sake of readability.\n>> >>\n>> >> Add a maximum limit of 120 characters to the '.editorconfig'. This means\n>> >> that if an individual line exceeds 120 characters, the editor will wrap\n>> >> that line. This provides a lot wiggle room over the recommended 80\n>> >> character limit.\n>> >\n>>\n>> Hello Justin,\n>>\n>> > I frequently use the format operator in vim to reformat entire blocks of\n>> > text and it is commonly configured to use `max_line_length` from an\n>> > `.editorconfig` file to know when to wrap lines. Changing the value to\n>> > 120 would cause my editor to prefer 120 character lines when\n>> > reformatting, which I would personally not like.\n>> >\n>>\n>> It would only wrap lines longer than 120 columns. Currently editorconfig\n>> doesn't wrap any line length. So we're essentially saying, any line\n>> above 120 is not something we want to accept and hence wrap. This\n>> doesn't mean that shorter lines will be combined together. Wouldn't this\n>> be better than the current situation?\n>\n> When `max_line_length` is set in a \".editorconfig\" file, in my vim\n> editor it overrides the `textwidth` configuration which was already set\n> to 80 by default. So changing to 120 would change line wrapping behavior\n> for me at least. I could disable using the \".editorconfig\", but I would\n> prefer to avoid doing that :)\n>\n> -Justin\n\nThanks for explaining. So it seems like vim in this case _does_ combine\nshorter lines to fit to 120 columns when formatting a block, this is not\nsomething we desire. So let me drop this patch.\n\n- Karthik\n"},{"id":"520902","messageId":"CAOLa=ZQD71Z4GSRNCuV=YL1zwGOXa-sGzpU9=8yfVcg6vJA6SQ@mail.gmail.com","threadId":"63693","inReplyTo":"xmqqtt418dnw.fsf@gitster.g","subject":"Re: [PATCH 4/4] meson: add rule to run 'git clang-format'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-30T08:34:48Z","receivedAt":"2025-06-30T08:34:50Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>>>> diff --git a/meson.build b/meson.build\n>>>> index 7fea4a34d6..578db26df2 100644\n>>>> --- a/meson.build\n>>>> +++ b/meson.build\n>>>> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'\n>>>>    alias_target('check-headers', hdr_check)\n>>>>  endif\n>>>>\n>>>> +clang_format = find_program('clang-format', required: false)\n>>>\n>>> Should we be checking for `git-clang-format` instead?\n>>>\n>>\n>> Yeah. While `git-clang-format` is packaged with `clang-format`, it does\n>> make more sense to check for the former.\n>\n> Just for my education, what does find_program() look for?  Installed\n> packages, or a program on your $PATH?  I am guessing that the answer\n> is the latter, in which case it is not like \"it makes more sense to\n> check for git-clang-format\"---rather it is \"it would not work at all\n> if we looked for clang-format\", no?\n>\n> Thanks.\n\nGood question. To quote from the documentation [1]:\n\n  find_program()\n\n  program_name here is a string that can be an executable or script to\n  be searched for in PATH or other places inside the project.\n\nSo, 'git-clang-format' would work. I've also verified the same on my\nend.\n\n[1]: https://mesonbuild.com/Reference-manual_functions.html#find_program\n"},{"id":"520903","messageId":"20250630-525-make-clang-format-more-robust-v2-0-05cbcdbf7817@gmail.com","threadId":"63693","inReplyTo":"20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com","subject":"[PATCH v2 0/3] clang-format: modify rules to reduce false-positives","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-30T08:38:19Z","receivedAt":"2025-06-30T08:38:37Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"This series is in response to an email thread [1] around the usage of\n'.clang-format' and its effectiveness.\n\nThe goal of the series is to improve the usage of 'clang-format' in the\nrepository. To do this we:\n1. Reduce the number of false positives. Majority of which is due to\n   line-wrapping. We remove the 'ColumnLimit' from 'clang-format'. This\n   removes the responsibility of line-wrapping from 'clang-format' and puts\n   it into the hands of the user.\n2. Add a rule to 'meson' to run 'git clang-format' by running 'meson\n   compile style'.\n3. Make the 'RemoveBracesLLVM' permanent by moving it to\n   '.clang-format'.\n\nWith this, running `git clang-format` for the last 25 commits in master,\nseems to produce much less false positives.\n\n  diff --git a/builtin/diff.c b/builtin/diff.c\n  index c6231edce4..246b81caa2 100644\n  --- a/builtin/diff.c\n  +++ b/builtin/diff.c\n  @@ -30,14 +30,13 @@\n   #define DIFF_NO_INDEX_IMPLICIT 2\n\n   static const char builtin_diff_usage[] =\n  -\"git diff [<options>] [<commit>] [--] [<path>...]\\n\"\n  -\"   or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\\n\"\n  -\"   or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\\n\"\n  -\"   or: git diff [<options>] <commit>...<commit> [--] [<path>...]\\n\"\n  -\"   or: git diff [<options>] <blob> <blob>\\n\"\n  -\"   or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]\"\n  -\"\\n\"\n  -COMMON_DIFF_OPTIONS_HELP;\n  +\t\"git diff [<options>] [<commit>] [--] [<path>...]\\n\"\n  +\t\"   or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\\n\"\n  +\t\"   or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\\n\"\n  +\t\"   or: git diff [<options>] <commit>...<commit> [--] [<path>...]\\n\"\n  +\t\"   or: git diff [<options>] <blob> <blob>\\n\"\n  +\t\"   or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]\"\n  +\t\"\\n\" COMMON_DIFF_OPTIONS_HELP;\n\n   static const char *blob_path(struct object_array_entry *entry)\n   {\n  diff --git a/builtin/stash.c b/builtin/stash.c\n  index 7cd3ad8aa4..90e441a6e5 100644\n  --- a/builtin/stash.c\n  +++ b/builtin/stash.c\n  @@ -1802,7 +1802,8 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n\n \t\t  argc = parse_options(argc, argv, prefix, options,\n \t\t\t\t       push_assumed ? git_stash_usage :\n  -\t\t\t\t     git_stash_push_usage, flags);\n  +\t\t\t\t\t\t    git_stash_push_usage,\n  +\t\t\t\t     flags);\n \t\t  force_assume |= patch_mode;\n \t  }\n\n  diff --git a/bundle-uri.c b/bundle-uri.c\n  index c9d65aa0ce..89f59aafe8 100644\n  --- a/bundle-uri.c\n  +++ b/bundle-uri.c\n  @@ -123,7 +123,7 @@ void print_bundle_list(FILE *fp, struct bundle_list *list)\n \t\t  for (i = 0; i < BUNDLE_HEURISTIC__COUNT; i++) {\n \t\t\t  if (heuristics[i].heuristic == list->heuristic) {\n \t\t\t\t  fprintf(fp, \"\\theuristic = %s\\n\",\n  -\t\t\t\t       heuristics[list->heuristic].name);\n  +\t\t\t\t\theuristics[list->heuristic].name);\n \t\t\t\t  break;\n \t\t\t  }\n \t\t  }\n  diff --git a/diff-no-index.c b/diff-no-index.c\n  index 4aeeb98cfa..a3892a9ccc 100644\n  --- a/diff-no-index.c\n  +++ b/diff-no-index.c\n  @@ -325,7 +325,7 @@ static int fixup_paths(const char **path, struct strbuf *replacement)\n \t  return 0;\n   }\n\n  -static const char * const diff_no_index_usage[] = {\n  +static const char *const diff_no_index_usage[] = {\n \t  N_(\"git diff --no-index [<options>] <path> <path> [<pathspec>...]\"),\n \t  NULL\n   };\n  diff --git a/pathspec.h b/pathspec.h\n  index 5e3a6f1fe7..601b9ca201 100644\n  --- a/pathspec.h\n  +++ b/pathspec.h\n  @@ -80,7 +80,7 @@ struct pathspec {\n    * For git diff --no-index, indicate that we are operating without\n    * a repository or index.\n    */\n  -#define PATHSPEC_NO_REPOSITORY (1<<7)\n  +#define PATHSPEC_NO_REPOSITORY (1 << 7)\n\n   /**\n    * Given command line arguments and a prefix, convert the input to\n\nWhile now I'm tempted to mark the 'check-style' CI job as required. I\nthink we should do that in the future.\n\n[1]: https://lore.kernel.org/git/xmqqmsa3adpw.fsf@gitster.g/\n\n---\nChanges in v2:\n- Drop the patch to add 120 column length to editorconfig. This way, we\n  will continue to use the default of 80 columns. Adding a higher column\n  length makes editorconfig combine smaller lines during block\n  formatting. This is not desirable.\n- Ensure that meson specifically checks for 'git-clang-format' and not\n  just 'clang-format'.\n- Link to v1: https://lore.kernel.org/r/20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com\n\n---\n .clang-format         | 27 +++++++++++++++------------\n ci/run-style-check.sh | 18 +-----------------\n meson.build           | 12 ++++++++++++\n 3 files changed, 28 insertions(+), 29 deletions(-)\n\nKarthik Nayak (3):\n      clang-format: set 'ColumnLimit' to 0\n      clang-format: add 'RemoveBracesLLVM' to the main config\n      meson: add rule to run 'git clang-format'\n\nRange-diff versus v1:\n\n1:  a62b4b7fe8 < -:  ---------- editorconfig: set maximum line length to 120 characters\n2:  4b4d49273e = 1:  5479d22f80 clang-format: set 'ColumnLimit' to 0\n3:  b9abb50adf = 2:  dacc7cc2b3 clang-format: add 'RemoveBracesLLVM' to the main config\n4:  03cc722f8f ! 3:  b13832ed57 meson: add rule to run 'git clang-format'\n    @@ meson.build: if headers_to_check.length() != 0 and compiler.get_argument_syntax(\n        alias_target('check-headers', hdr_check)\n      endif\n      \n    -+clang_format = find_program('clang-format', required: false)\n    -+if clang_format.found()\n    ++git_clang_format = find_program('git-clang-format', required: false)\n    ++if git_clang_format.found()\n     +  run_target('style',\n     +    command: [\n     +      'git', 'clang-format',\n\n\nbase-commit: f0135a9047ca37d4d117dcf21f7e3e89fad85d00\nchange-id: 20250625-525-make-clang-format-more-robust-968126c991b9\n\nThanks\n- Karthik\n\n"},{"id":"520904","messageId":"20250630-525-make-clang-format-more-robust-v2-1-05cbcdbf7817@gmail.com","threadId":"63693","inReplyTo":"20250630-525-make-clang-format-more-robust-v2-0-05cbcdbf7817@gmail.com","subject":"[PATCH v2 1/3] clang-format: set 'ColumnLimit' to 0","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-30T08:38:20Z","receivedAt":"2025-06-30T08:38:38Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"When clang-format was introduced to the Git project in\n6134de6ac1 (clang-format: outline the git project's coding style,\n2017-08-14), the 'ColumnLimit' was set to 80. This is inline with our\nrecommendation in 'Documentation/CodingGuidelines', which states:\n\n  We try to keep to at most 80 characters per line.\n\nHowever while this is recommended limit, this is not the enforced\nlimit. In some cases in we do overflow this limit to prioritize\nreadability. Setting the 'ColumnLimit' also means that shorter lines are\nconcatenated to simply as the result would still be below 80 characters,\nwhich is undesirable.\n\nIn the past, we tried to adjust the penalties around line wrapping, once\nin 42efde4c29 (clang-format: adjust line break penalties, 2017-09-29)\nand another time in 5e9fa0f9fa (clang-format: re-adjust line break\npenalties, 2024-10-18). While these settings help tweak the line break\npenalties to be more in-line with the requirements of the Git project,\nusing 'clang-format' still produces a lot of false positives.\n\nSo to make 'clang-format' more usable, set the 'ColumnLimit' to 0. This\nmeans that line-wrapping is no-longer a concern of the formatter and\nsomething that the user needs to take care of. The previous commit also\nadded a more flexible guideline to the '.editorconfig' setting a\n'max_line_length' of 120 characters. This should provide some guidance\nto users.\n\nIn the future, it would be nice to re-instate this limit with adequate\npenalties which would follow our guidelines, but currently, it makes\nmore sense to have a working formatter which we can rely on and which\ndoesn't create too many false positives.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n .clang-format | 21 +++++++++------------\n 1 file changed, 9 insertions(+), 12 deletions(-)\n\ndiff --git a/.clang-format b/.clang-format\nindex 9547fe1b77..19d6cf4200 100644\n--- a/.clang-format\n+++ b/.clang-format\n@@ -12,7 +12,15 @@ UseTab: Always\n TabWidth: 8\n IndentWidth: 8\n ContinuationIndentWidth: 8\n-ColumnLimit: 80\n+\n+# While we do want to enforce a character limit of 80 characters, we often\n+# allow lines to overflow that limit to prioritize readability. Setting a\n+# character limit here with penalties has been finicky and creates too many\n+# false positives.\n+#\n+# NEEDSWORK: It would be nice if we can find optimal settings to ensure we\n+# can re-enable the limit here.\n+ColumnLimit: 0\n \n # C Language specifics\n Language: Cpp\n@@ -210,16 +218,5 @@ MaxEmptyLinesToKeep: 1\n # No empty line at the start of a block.\n KeepEmptyLinesAtTheStartOfBlocks: false\n \n-# Penalties\n-# This decides what order things should be done if a line is too long\n-PenaltyBreakAssignment: 5\n-PenaltyBreakBeforeFirstCallParameter: 5\n-PenaltyBreakComment: 5\n-PenaltyBreakFirstLessLess: 0\n-PenaltyBreakOpenParenthesis: 300\n-PenaltyBreakString: 5\n-PenaltyExcessCharacter: 10\n-PenaltyReturnTypeOnItsOwnLine: 300\n-\n # Don't sort #include's\n SortIncludes: false\n\n-- \n2.49.0\n\n"},{"id":"520906","messageId":"20250630-525-make-clang-format-more-robust-v2-2-05cbcdbf7817@gmail.com","threadId":"63693","inReplyTo":"20250630-525-make-clang-format-more-robust-v2-0-05cbcdbf7817@gmail.com","subject":"[PATCH v2 2/3] clang-format: add 'RemoveBracesLLVM' to the main config","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-30T08:38:21Z","receivedAt":"2025-06-30T08:38:39Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In 1b8f306612 (ci/style-check: add `RemoveBracesLLVM` in CI job,\n2024-07-23) we added 'RemoveBracesLLVM' to the CI job of running the\nclang formatter.\n\nThis rule checks and warns against using braces on simple\nsingle-statement bodies of statements. Since we haven't had any issues\nregarding this rule, we can now move it into the main clang-format\nconfig and remove it from being CI exclusive.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n .clang-format         |  6 ++++++\n ci/run-style-check.sh | 18 +-----------------\n 2 files changed, 7 insertions(+), 17 deletions(-)\n\ndiff --git a/.clang-format b/.clang-format\nindex 19d6cf4200..dcfd0aad60 100644\n--- a/.clang-format\n+++ b/.clang-format\n@@ -220,3 +220,9 @@ KeepEmptyLinesAtTheStartOfBlocks: false\n \n # Don't sort #include's\n SortIncludes: false\n+\n+# Remove optional braces of control statements (if, else, for, and while)\n+# according to the LLVM coding style. This avoids braces on simple\n+# single-statement bodies of statements but keeps braces if one side of\n+# if/else if/.../else cascade has multi-statement body.\n+RemoveBracesLLVM: true\ndiff --git a/ci/run-style-check.sh b/ci/run-style-check.sh\nindex 6cd4b1d934..0832c19df0 100755\n--- a/ci/run-style-check.sh\n+++ b/ci/run-style-check.sh\n@@ -5,21 +5,5 @@\n \n baseCommit=$1\n \n-# Remove optional braces of control statements (if, else, for, and while)\n-# according to the LLVM coding style. This avoids braces on simple\n-# single-statement bodies of statements but keeps braces if one side of\n-# if/else if/.../else cascade has multi-statement body.\n-#\n-# As this rule comes with a warning [1], we want to experiment with it\n-# before adding it in-tree. since the CI job for the style check is allowed\n-# to fail, appending the rule here allows us to validate its efficacy.\n-# While also ensuring that end-users are not affected directly.\n-#\n-# [1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm\n-{\n-\tcat .clang-format\n-\techo \"RemoveBracesLLVM: true\"\n-} >/tmp/clang-format-rules\n-\n-git clang-format --style=file:/tmp/clang-format-rules \\\n+git clang-format --style=file:.clang-format \\\n \t--diff --extensions c,h \"$baseCommit\"\n\n-- \n2.49.0\n\n"},{"id":"520905","messageId":"20250630-525-make-clang-format-more-robust-v2-3-05cbcdbf7817@gmail.com","threadId":"63693","inReplyTo":"20250630-525-make-clang-format-more-robust-v2-0-05cbcdbf7817@gmail.com","subject":"[PATCH v2 3/3] meson: add rule to run 'git clang-format'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-06-30T08:38:22Z","receivedAt":"2025-06-30T08:38:40Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The Makefile has a 'style' rule to run 'git clang-format'. While Meson\nintrinsically supports a 'clang-format' target, which can be run when\nusing the ninja backend by running 'ninja clang-format', this runs the\nformatting on all existing files.\n\nOur Meson build doesn't yet support a way to run 'git clang-format',\nwhich runs the formatter between the working directory and commit\nprovided. Add a new 'style' target to Meson to mimic the target in the\nMakefile.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n meson.build | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/meson.build b/meson.build\nindex 7fea4a34d6..20ce0525a1 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'\n   alias_target('check-headers', hdr_check)\n endif\n \n+git_clang_format = find_program('git-clang-format', required: false)\n+if git_clang_format.found()\n+  run_target('style',\n+    command: [\n+      'git', 'clang-format',\n+      '--style', 'file',\n+      '--diff',\n+      '--extensions', 'c,h'\n+    ]\n+  )\n+endif\n+\n foreach key, value : {\n   'DIFF': diff.full_path(),\n   'GIT_SOURCE_DIR': meson.project_source_root(),\n\n-- \n2.49.0\n\n"},{"id":"520926","messageId":"xmqq4ivx5k1l.fsf@gitster.g","threadId":"63693","inReplyTo":"CAOLa=ZQD71Z4GSRNCuV=YL1zwGOXa-sGzpU9=8yfVcg6vJA6SQ@mail.gmail.com","subject":"Re: [PATCH 4/4] meson: add rule to run 'git clang-format'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-30T15:24:54Z","receivedAt":"2025-06-30T15:24:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n>>>>> +clang_format = find_program('clang-format', required: false)\n>>>>\n>>>> Should we be checking for `git-clang-format` instead?\n>>>\n>>> Yeah. While `git-clang-format` is packaged with `clang-format`, it does\n>>> make more sense to check for the former.\n>>\n>> Just for my education, what does find_program() look for?  Installed\n>> packages, or a program on your $PATH?  I am guessing that the answer\n>> is the latter, in which case it is not like \"it makes more sense to\n>> check for git-clang-format\"---rather it is \"it would not work at all\n>> if we looked for clang-format\", no?\n>>\n>> Thanks.\n>\n> Good question. To quote from the documentation [1]:\n>\n>   find_program()\n>\n>   program_name here is a string that can be an executable or script to\n>   be searched for in PATH or other places inside the project.\n>\n> So, 'git-clang-format' would work. I've also verified the same on my\n> end.\n\nI think that much everybody would know by what other uses of\nfind_program() are looking for by checking \"git grep find_program\"\noutput.\n\nI was confused by your \"it does make *MORE* sense to check for the\nformer\" (emphasis mine), as if you were saying that both would work\nbut using 'git-clang-format' would be a more kosher way to express\nwhat we want.  Given that it would not work at all if you used\n'clang-format' instead, that statement was misleading.\n\nSo the response I was expecting in the message I am responding to\nwas more like \"'clang-format' would *not* work at all, and we must\ncheck 'git-clang-format' instead\".\n\nThanks.\n"},{"id":"520927","messageId":"xmqqzfdp45c9.fsf@gitster.g","threadId":"63693","inReplyTo":"20250630-525-make-clang-format-more-robust-v2-0-05cbcdbf7817@gmail.com","subject":"Re: [PATCH v2 0/3] clang-format: modify rules to reduce false-positives","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-30T15:27:50Z","receivedAt":"2025-06-30T15:27:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> - Drop the patch to add 120 column length to editorconfig. This way, we\n>   will continue to use the default of 80 columns. Adding a higher column\n>   length makes editorconfig combine smaller lines during block\n>   formatting. This is not desirable.\n\nMakes sense.\n\n> - Ensure that meson specifically checks for 'git-clang-format' and not\n>   just 'clang-format'.\n\nOK.\n\nLooking better.  Thanks.\n"},{"id":"521004","messageId":"aGO00T4c2Q-EYXZx@pks.im","threadId":"63693","inReplyTo":"20250630-525-make-clang-format-more-robust-v2-3-05cbcdbf7817@gmail.com","subject":"Re: [PATCH v2 3/3] meson: add rule to run 'git clang-format'","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-07-01T10:13:37Z","receivedAt":"2025-07-01T10:13:44Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Jun 30, 2025 at 10:38:22AM +0200, Karthik Nayak wrote:\n> The Makefile has a 'style' rule to run 'git clang-format'. While Meson\n> intrinsically supports a 'clang-format' target, which can be run when\n> using the ninja backend by running 'ninja clang-format', this runs the\n> formatting on all existing files.\n> \n> Our Meson build doesn't yet support a way to run 'git clang-format',\n> which runs the formatter between the working directory and commit\n> provided. Add a new 'style' target to Meson to mimic the target in the\n> Makefile.\n\nHm. Meson already knows to wire up clang-format automaically if it's\navailable. But it indeed doesn't know to only format files that have\nbeen changed, so I guess this style makes sense regardless of that.\n\n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n>  meson.build | 12 ++++++++++++\n>  1 file changed, 12 insertions(+)\n> \n> diff --git a/meson.build b/meson.build\n> index 7fea4a34d6..20ce0525a1 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'\n>    alias_target('check-headers', hdr_check)\n>  endif\n>  \n> +git_clang_format = find_program('git-clang-format', required: false)\n> +if git_clang_format.found()\n> +  run_target('style',\n> +    command: [\n> +      'git', 'clang-format',\n> +      '--style', 'file',\n> +      '--diff',\n> +      '--extensions', 'c,h'\n> +    ]\n> +  )\n> +endif\n\nDo we want to call this target `clang-format-changed` though, so that it\nis consistent with the implicit `clang-format` target?\n\nPatrick\n"},{"id":"521046","messageId":"87bjq4qbxi.fsf@iotcl.com","threadId":"63693","inReplyTo":"20250630-525-make-clang-format-more-robust-v2-3-05cbcdbf7817@gmail.com","subject":"Re: [PATCH v2 3/3] meson: add rule to run 'git clang-format'","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-07-01T13:26:49Z","receivedAt":"2025-07-01T13:27:07Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> The Makefile has a 'style' rule to run 'git clang-format'. While Meson\n> intrinsically supports a 'clang-format' target, which can be run when\n> using the ninja backend by running 'ninja clang-format', this runs the\n> formatting on all existing files.\n>\n> Our Meson build doesn't yet support a way to run 'git clang-format',\n> which runs the formatter between the working directory and commit\n> provided. Add a new 'style' target to Meson to mimic the target in the\n> Makefile.\n>\n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n>  meson.build | 12 ++++++++++++\n>  1 file changed, 12 insertions(+)\n>\n> diff --git a/meson.build b/meson.build\n> index 7fea4a34d6..20ce0525a1 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'\n>    alias_target('check-headers', hdr_check)\n>  endif\n>  \n> +git_clang_format = find_program('git-clang-format', required: false)\n\nI think we should include `native: true` as well.\n\n> +if git_clang_format.found()\n> +  run_target('style',\n> +    command: [\n> +      'git', 'clang-format',\n\nWe should be using the `git_clang_format` variable here to ensure we\ncall the tool we've found.\n\n> +      '--style', 'file',\n> +      '--diff',\n> +      '--extensions', 'c,h'\n> +    ]\n> +  )\n> +endif\n> +\n>  foreach key, value : {\n>    'DIFF': diff.full_path(),\n>    'GIT_SOURCE_DIR': meson.project_source_root(),\n>\n> -- \n> 2.49.0\n>\n>\n\n-- \nCheers,\nToon\n"},{"id":"521047","messageId":"878ql8qbhp.fsf@iotcl.com","threadId":"63693","inReplyTo":"20250630-525-make-clang-format-more-robust-v2-0-05cbcdbf7817@gmail.com","subject":"Re: [PATCH v2 0/3] clang-format: modify rules to reduce false-positives","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-07-01T13:36:18Z","receivedAt":"2025-07-01T13:36:31Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> This series is in response to an email thread [1] around the usage of\n> '.clang-format' and its effectiveness.\n>\n> The goal of the series is to improve the usage of 'clang-format' in the\n> repository. To do this we:\n> 1. Reduce the number of false positives. Majority of which is due to\n>    line-wrapping. We remove the 'ColumnLimit' from 'clang-format'. This\n>    removes the responsibility of line-wrapping from 'clang-format' and puts\n>    it into the hands of the user.\n> 2. Add a rule to 'meson' to run 'git clang-format' by running 'meson\n>    compile style'.\n> 3. Make the 'RemoveBracesLLVM' permanent by moving it to\n>    '.clang-format'.\n\nThanks for working on this. While the changes are not huge, I appreciate\nthem.\n\n> With this, running `git clang-format` for the last 25 commits in master,\n> seems to produce much less false positives.\n>\n> [snip]\n>\n>   diff --git a/pathspec.h b/pathspec.h\n>   index 5e3a6f1fe7..601b9ca201 100644\n>   --- a/pathspec.h\n>   +++ b/pathspec.h\n>   @@ -80,7 +80,7 @@ struct pathspec {\n>     * For git diff --no-index, indicate that we are operating without\n>     * a repository or index.\n>     */\n>   -#define PATHSPEC_NO_REPOSITORY (1<<7)\n>   +#define PATHSPEC_NO_REPOSITORY (1 << 7)\n\nI'm surprised, but I couldn't find a setting to change this...\n\n\n-- \nCheers,\nToon\n"},{"id":"521057","messageId":"CAOLa=ZSaAVY2ChSWyU20fMniFHAz-aVtCDR_EizxJfErdy-u+Q@mail.gmail.com","threadId":"63693","inReplyTo":"aGO00T4c2Q-EYXZx@pks.im","subject":"Re: [PATCH v2 3/3] meson: add rule to run 'git clang-format'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-07-01T15:08:10Z","receivedAt":"2025-07-01T15:08:13Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Mon, Jun 30, 2025 at 10:38:22AM +0200, Karthik Nayak wrote:\n>> The Makefile has a 'style' rule to run 'git clang-format'. While Meson\n>> intrinsically supports a 'clang-format' target, which can be run when\n>> using the ninja backend by running 'ninja clang-format', this runs the\n>> formatting on all existing files.\n>>\n>> Our Meson build doesn't yet support a way to run 'git clang-format',\n>> which runs the formatter between the working directory and commit\n>> provided. Add a new 'style' target to Meson to mimic the target in the\n>> Makefile.\n>\n> Hm. Meson already knows to wire up clang-format automaically if it's\n> available. But it indeed doesn't know to only format files that have\n> been changed, so I guess this style makes sense regardless of that.\n>\n\nExactly!\n\n>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n>> ---\n>>  meson.build | 12 ++++++++++++\n>>  1 file changed, 12 insertions(+)\n>>\n>> diff --git a/meson.build b/meson.build\n>> index 7fea4a34d6..20ce0525a1 100644\n>> --- a/meson.build\n>> +++ b/meson.build\n>> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'\n>>    alias_target('check-headers', hdr_check)\n>>  endif\n>>\n>> +git_clang_format = find_program('git-clang-format', required: false)\n>> +if git_clang_format.found()\n>> +  run_target('style',\n>> +    command: [\n>> +      'git', 'clang-format',\n>> +      '--style', 'file',\n>> +      '--diff',\n>> +      '--extensions', 'c,h'\n>> +    ]\n>> +  )\n>> +endif\n>\n> Do we want to call this target `clang-format-changed` though, so that it\n> is consistent with the implicit `clang-format` target?\n>\n\nI was aiming for consistency with the Makefile, I don't think anyone\nuses `meson compile clang-format` anyways. I do like the short form, but\nopen to adding an alias and setting `make style` for deprecation in the\nfollowing releases.\n\n> Patrick\n"},{"id":"521058","messageId":"CAOLa=ZT6-Ea9iaNgYymmT6s5mjWk4i4RxZjcr+7G6HUbg4GdHw@mail.gmail.com","threadId":"63693","inReplyTo":"87bjq4qbxi.fsf@iotcl.com","subject":"Re: [PATCH v2 3/3] meson: add rule to run 'git clang-format'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-07-01T15:12:16Z","receivedAt":"2025-07-01T15:12:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Toon Claes <toon@iotcl.com> writes:\n\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> The Makefile has a 'style' rule to run 'git clang-format'. While Meson\n>> intrinsically supports a 'clang-format' target, which can be run when\n>> using the ninja backend by running 'ninja clang-format', this runs the\n>> formatting on all existing files.\n>>\n>> Our Meson build doesn't yet support a way to run 'git clang-format',\n>> which runs the formatter between the working directory and commit\n>> provided. Add a new 'style' target to Meson to mimic the target in the\n>> Makefile.\n>>\n>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n>> ---\n>>  meson.build | 12 ++++++++++++\n>>  1 file changed, 12 insertions(+)\n>>\n>> diff --git a/meson.build b/meson.build\n>> index 7fea4a34d6..20ce0525a1 100644\n>> --- a/meson.build\n>> +++ b/meson.build\n>> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'\n>>    alias_target('check-headers', hdr_check)\n>>  endif\n>>\n>> +git_clang_format = find_program('git-clang-format', required: false)\n>\n> I think we should include `native: true` as well.\n>\n\nDoes it really matter here? I must admit I don't understand the\nrepercussions here.\n\n>> +if git_clang_format.found()\n>> +  run_target('style',\n>> +    command: [\n>> +      'git', 'clang-format',\n>\n> We should be using the `git_clang_format` variable here to ensure we\n> call the tool we've found.\n>\n\nGood point, let me add that.\n\n>> +      '--style', 'file',\n>> +      '--diff',\n>> +      '--extensions', 'c,h'\n>> +    ]\n>> +  )\n>> +endif\n>> +\n>>  foreach key, value : {\n>>    'DIFF': diff.full_path(),\n>>    'GIT_SOURCE_DIR': meson.project_source_root(),\n>>\n>> --\n>> 2.49.0\n>>\n>>\n>\n> --\n> Cheers,\n> Toon\n"},{"id":"521066","messageId":"CAOLa=ZQr=JfKES6vMfROLsvO0egjwNun6oGkwJsN9_TkQkt3rg@mail.gmail.com","threadId":"63693","inReplyTo":"878ql8qbhp.fsf@iotcl.com","subject":"Re: [PATCH v2 0/3] clang-format: modify rules to reduce false-positives","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-07-01T15:19:26Z","receivedAt":"2025-07-01T15:19:28Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Toon Claes <toon@iotcl.com> writes:\n\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> This series is in response to an email thread [1] around the usage of\n>> '.clang-format' and its effectiveness.\n>>\n>> The goal of the series is to improve the usage of 'clang-format' in the\n>> repository. To do this we:\n>> 1. Reduce the number of false positives. Majority of which is due to\n>>    line-wrapping. We remove the 'ColumnLimit' from 'clang-format'. This\n>>    removes the responsibility of line-wrapping from 'clang-format' and puts\n>>    it into the hands of the user.\n>> 2. Add a rule to 'meson' to run 'git clang-format' by running 'meson\n>>    compile style'.\n>> 3. Make the 'RemoveBracesLLVM' permanent by moving it to\n>>    '.clang-format'.\n>\n> Thanks for working on this. While the changes are not huge, I appreciate\n> them.\n>\n>> With this, running `git clang-format` for the last 25 commits in master,\n>> seems to produce much less false positives.\n>>\n>> [snip]\n>>\n>>   diff --git a/pathspec.h b/pathspec.h\n>>   index 5e3a6f1fe7..601b9ca201 100644\n>>   --- a/pathspec.h\n>>   +++ b/pathspec.h\n>>   @@ -80,7 +80,7 @@ struct pathspec {\n>>     * For git diff --no-index, indicate that we are operating without\n>>     * a repository or index.\n>>     */\n>>   -#define PATHSPEC_NO_REPOSITORY (1<<7)\n>>   +#define PATHSPEC_NO_REPOSITORY (1 << 7)\n>\n> I'm surprised, but I couldn't find a setting to change this...\n>\n>\n\nI think spacing around binary operators is intrinsic and not an option\n[1]. But I'm not sure...\n\n[1]: https://github.com/llvm/llvm-project/issues/19412\n\n> --\n> Cheers,\n> Toon\n"},{"id":"521075","messageId":"xmqq5xgblx0v.fsf@gitster.g","threadId":"63693","inReplyTo":"aGO00T4c2Q-EYXZx@pks.im","subject":"Re: [PATCH v2 3/3] meson: add rule to run 'git clang-format'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-01T16:02:24Z","receivedAt":"2025-07-01T16:02:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n>> +git_clang_format = find_program('git-clang-format', required: false)\n>> +if git_clang_format.found()\n>> +  run_target('style',\n>> +    command: [\n>> +      'git', 'clang-format',\n>> +      '--style', 'file',\n>> +      '--diff',\n>> +      '--extensions', 'c,h'\n>> +    ]\n>> +  )\n>> +endif\n>\n> Do we want to call this target `clang-format-changed` though, so that it\n> is consistent with the implicit `clang-format` target?\n\nIt think this is trying to be consistent with the 'make style'\ntarget.\n"},{"id":"521106","messageId":"xmqqcyajk55k.fsf@gitster.g","threadId":"63693","inReplyTo":"878ql8qbhp.fsf@iotcl.com","subject":"Re: [PATCH v2 0/3] clang-format: modify rules to reduce false-positives","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-01T20:49:43Z","receivedAt":"2025-07-01T20:49:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes <toon@iotcl.com> writes:\n\n>>   -#define PATHSPEC_NO_REPOSITORY (1<<7)\n>>   +#define PATHSPEC_NO_REPOSITORY (1 << 7)\n>\n> I'm surprised, but I couldn't find a setting to change this...\n\nWe do not mind spaces around binary operators like \"<<\", and the\nchange suggested by the tool is an improvement in this case, I\nthink.  The lack of U suffix to ensure that a bitmask is unsigned\nbothers me more, but that is not formatting issue.\n\nThanks.\n\n"},{"id":"521146","messageId":"aGSfedFVoMnhwZbJ@pks.im","threadId":"63693","inReplyTo":"CAOLa=ZT6-Ea9iaNgYymmT6s5mjWk4i4RxZjcr+7G6HUbg4GdHw@mail.gmail.com","subject":"Re: [PATCH v2 3/3] meson: add rule to run 'git clang-format'","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-07-02T02:54:49Z","receivedAt":"2025-07-02T02:54:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Jul 01, 2025 at 11:12:16AM -0400, Karthik Nayak wrote:\n> Toon Claes <toon@iotcl.com> writes:\n> > Karthik Nayak <karthik.188@gmail.com> writes:\n> >> diff --git a/meson.build b/meson.build\n> >> index 7fea4a34d6..20ce0525a1 100644\n> >> --- a/meson.build\n> >> +++ b/meson.build\n> >> @@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'\n> >>    alias_target('check-headers', hdr_check)\n> >>  endif\n> >>\n> >> +git_clang_format = find_program('git-clang-format', required: false)\n> >\n> > I think we should include `native: true` as well.\n> >\n> \n> Does it really matter here? I must admit I don't understand the\n> repercussions here.\n\nIt doesn't really, as `native: true` is the default. But we explicitly\nsay whether we want native or non-native binaries for all the other\ncalls to `find_program()` to make it more obvious, so I think it would\nbe a sensible addition here.\n\nPatrick\n"},{"id":"521154","messageId":"20250702-525-make-clang-format-more-robust-v3-0-705344f30580@gmail.com","threadId":"63693","inReplyTo":"20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com","subject":"[PATCH v3 0/3] clang-format: modify rules to reduce false-positives","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-07-02T09:23:17Z","receivedAt":"2025-07-02T09:23:27Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"This series is in response to an email thread [1] around the usage of\n'.clang-format' and its effectiveness.\n\nThe goal of the series is to improve the usage of 'clang-format' in the\nrepository. To do this we:\n1. Reduce the number of false positives. Majority of which is due to\n   line-wrapping. We remove the 'ColumnLimit' from 'clang-format'. This\n   removes the responsibility of line-wrapping from 'clang-format' and puts\n   it into the hands of the user.\n2. Add a rule to 'meson' to run 'git clang-format' by running 'meson\n   compile style'.\n3. Make the 'RemoveBracesLLVM' permanent by moving it to\n   '.clang-format'.\n\nWith this, running `git clang-format` for the last 25 commits in master,\nseems to produce much less false positives.\n\n  diff --git a/builtin/diff.c b/builtin/diff.c\n  index c6231edce4..246b81caa2 100644\n  --- a/builtin/diff.c\n  +++ b/builtin/diff.c\n  @@ -30,14 +30,13 @@\n   #define DIFF_NO_INDEX_IMPLICIT 2\n\n   static const char builtin_diff_usage[] =\n  -\"git diff [<options>] [<commit>] [--] [<path>...]\\n\"\n  -\"   or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\\n\"\n  -\"   or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\\n\"\n  -\"   or: git diff [<options>] <commit>...<commit> [--] [<path>...]\\n\"\n  -\"   or: git diff [<options>] <blob> <blob>\\n\"\n  -\"   or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]\"\n  -\"\\n\"\n  -COMMON_DIFF_OPTIONS_HELP;\n  +\t\"git diff [<options>] [<commit>] [--] [<path>...]\\n\"\n  +\t\"   or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\\n\"\n  +\t\"   or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\\n\"\n  +\t\"   or: git diff [<options>] <commit>...<commit> [--] [<path>...]\\n\"\n  +\t\"   or: git diff [<options>] <blob> <blob>\\n\"\n  +\t\"   or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]\"\n  +\t\"\\n\" COMMON_DIFF_OPTIONS_HELP;\n\n   static const char *blob_path(struct object_array_entry *entry)\n   {\n  diff --git a/builtin/stash.c b/builtin/stash.c\n  index 7cd3ad8aa4..90e441a6e5 100644\n  --- a/builtin/stash.c\n  +++ b/builtin/stash.c\n  @@ -1802,7 +1802,8 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n\n \t\t  argc = parse_options(argc, argv, prefix, options,\n \t\t\t\t       push_assumed ? git_stash_usage :\n  -\t\t\t\t     git_stash_push_usage, flags);\n  +\t\t\t\t\t\t    git_stash_push_usage,\n  +\t\t\t\t     flags);\n \t\t  force_assume |= patch_mode;\n \t  }\n\n  diff --git a/bundle-uri.c b/bundle-uri.c\n  index c9d65aa0ce..89f59aafe8 100644\n  --- a/bundle-uri.c\n  +++ b/bundle-uri.c\n  @@ -123,7 +123,7 @@ void print_bundle_list(FILE *fp, struct bundle_list *list)\n \t\t  for (i = 0; i < BUNDLE_HEURISTIC__COUNT; i++) {\n \t\t\t  if (heuristics[i].heuristic == list->heuristic) {\n \t\t\t\t  fprintf(fp, \"\\theuristic = %s\\n\",\n  -\t\t\t\t       heuristics[list->heuristic].name);\n  +\t\t\t\t\theuristics[list->heuristic].name);\n \t\t\t\t  break;\n \t\t\t  }\n \t\t  }\n  diff --git a/diff-no-index.c b/diff-no-index.c\n  index 4aeeb98cfa..a3892a9ccc 100644\n  --- a/diff-no-index.c\n  +++ b/diff-no-index.c\n  @@ -325,7 +325,7 @@ static int fixup_paths(const char **path, struct strbuf *replacement)\n \t  return 0;\n   }\n\n  -static const char * const diff_no_index_usage[] = {\n  +static const char *const diff_no_index_usage[] = {\n \t  N_(\"git diff --no-index [<options>] <path> <path> [<pathspec>...]\"),\n \t  NULL\n   };\n  diff --git a/pathspec.h b/pathspec.h\n  index 5e3a6f1fe7..601b9ca201 100644\n  --- a/pathspec.h\n  +++ b/pathspec.h\n  @@ -80,7 +80,7 @@ struct pathspec {\n    * For git diff --no-index, indicate that we are operating without\n    * a repository or index.\n    */\n  -#define PATHSPEC_NO_REPOSITORY (1<<7)\n  +#define PATHSPEC_NO_REPOSITORY (1 << 7)\n\n   /**\n    * Given command line arguments and a prefix, convert the input to\n\nWhile now I'm tempted to mark the 'check-style' CI job as required. I\nthink we should do that in the future.\n\n[1]: https://lore.kernel.org/git/xmqqmsa3adpw.fsf@gitster.g/\n\n---\nChanges in v3:\n- In meson.build, set 'native: true' and use the variable obtained from\n  'find_program()' directly in 'run_target()'.\n- Link to v2: https://lore.kernel.org/r/20250630-525-make-clang-format-more-robust-v2-0-05cbcdbf7817@gmail.com\n\nChanges in v2:\n- Drop the patch to add 120 column length to editorconfig. This way, we\n  will continue to use the default of 80 columns. Adding a higher column\n  length makes editorconfig combine smaller lines during block\n  formatting. This is not desirable.\n- Ensure that meson specifically checks for 'git-clang-format' and not\n  just 'clang-format'.\n- Link to v1: https://lore.kernel.org/r/20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com\n\n---\n .clang-format         | 27 +++++++++++++++------------\n ci/run-style-check.sh | 18 +-----------------\n meson.build           | 12 ++++++++++++\n 3 files changed, 28 insertions(+), 29 deletions(-)\n\nKarthik Nayak (3):\n      clang-format: set 'ColumnLimit' to 0\n      clang-format: add 'RemoveBracesLLVM' to the main config\n      meson: add rule to run 'git clang-format'\n\nRange-diff versus v2:\n\n1:  f8271d7114 = 1:  c7a7f6d798 clang-format: set 'ColumnLimit' to 0\n2:  f49237edda = 2:  c68ea68349 clang-format: add 'RemoveBracesLLVM' to the main config\n3:  8d2a1647a2 ! 3:  b972289ab8 meson: add rule to run 'git clang-format'\n    @@ meson.build: if headers_to_check.length() != 0 and compiler.get_argument_syntax(\n        alias_target('check-headers', hdr_check)\n      endif\n      \n    -+git_clang_format = find_program('git-clang-format', required: false)\n    ++git_clang_format = find_program('git-clang-format', required: false, native: true)\n     +if git_clang_format.found()\n     +  run_target('style',\n     +    command: [\n    -+      'git', 'clang-format',\n    ++      git_clang_format,\n     +      '--style', 'file',\n     +      '--diff',\n     +      '--extensions', 'c,h'\n\n\nbase-commit: f0135a9047ca37d4d117dcf21f7e3e89fad85d00\nchange-id: 20250625-525-make-clang-format-more-robust-968126c991b9\n\nThanks\n- Karthik\n\n"},{"id":"521155","messageId":"20250702-525-make-clang-format-more-robust-v3-1-705344f30580@gmail.com","threadId":"63693","inReplyTo":"20250702-525-make-clang-format-more-robust-v3-0-705344f30580@gmail.com","subject":"[PATCH v3 1/3] clang-format: set 'ColumnLimit' to 0","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-07-02T09:23:18Z","receivedAt":"2025-07-02T09:23:28Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"When clang-format was introduced to the Git project in\n6134de6ac1 (clang-format: outline the git project's coding style,\n2017-08-14), the 'ColumnLimit' was set to 80. This is inline with our\nrecommendation in 'Documentation/CodingGuidelines', which states:\n\n  We try to keep to at most 80 characters per line.\n\nHowever while this is recommended limit, this is not the enforced\nlimit. In some cases in we do overflow this limit to prioritize\nreadability. Setting the 'ColumnLimit' also means that shorter lines are\nconcatenated to simply as the result would still be below 80 characters,\nwhich is undesirable.\n\nIn the past, we tried to adjust the penalties around line wrapping, once\nin 42efde4c29 (clang-format: adjust line break penalties, 2017-09-29)\nand another time in 5e9fa0f9fa (clang-format: re-adjust line break\npenalties, 2024-10-18). While these settings help tweak the line break\npenalties to be more in-line with the requirements of the Git project,\nusing 'clang-format' still produces a lot of false positives.\n\nSo to make 'clang-format' more usable, set the 'ColumnLimit' to 0. This\nmeans that line-wrapping is no-longer a concern of the formatter and\nsomething that the user needs to take care of. The previous commit also\nadded a more flexible guideline to the '.editorconfig' setting a\n'max_line_length' of 120 characters. This should provide some guidance\nto users.\n\nIn the future, it would be nice to re-instate this limit with adequate\npenalties which would follow our guidelines, but currently, it makes\nmore sense to have a working formatter which we can rely on and which\ndoesn't create too many false positives.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n .clang-format | 21 +++++++++------------\n 1 file changed, 9 insertions(+), 12 deletions(-)\n\ndiff --git a/.clang-format b/.clang-format\nindex 9547fe1b77..19d6cf4200 100644\n--- a/.clang-format\n+++ b/.clang-format\n@@ -12,7 +12,15 @@ UseTab: Always\n TabWidth: 8\n IndentWidth: 8\n ContinuationIndentWidth: 8\n-ColumnLimit: 80\n+\n+# While we do want to enforce a character limit of 80 characters, we often\n+# allow lines to overflow that limit to prioritize readability. Setting a\n+# character limit here with penalties has been finicky and creates too many\n+# false positives.\n+#\n+# NEEDSWORK: It would be nice if we can find optimal settings to ensure we\n+# can re-enable the limit here.\n+ColumnLimit: 0\n \n # C Language specifics\n Language: Cpp\n@@ -210,16 +218,5 @@ MaxEmptyLinesToKeep: 1\n # No empty line at the start of a block.\n KeepEmptyLinesAtTheStartOfBlocks: false\n \n-# Penalties\n-# This decides what order things should be done if a line is too long\n-PenaltyBreakAssignment: 5\n-PenaltyBreakBeforeFirstCallParameter: 5\n-PenaltyBreakComment: 5\n-PenaltyBreakFirstLessLess: 0\n-PenaltyBreakOpenParenthesis: 300\n-PenaltyBreakString: 5\n-PenaltyExcessCharacter: 10\n-PenaltyReturnTypeOnItsOwnLine: 300\n-\n # Don't sort #include's\n SortIncludes: false\n\n-- \n2.49.0\n\n"},{"id":"521156","messageId":"20250702-525-make-clang-format-more-robust-v3-2-705344f30580@gmail.com","threadId":"63693","inReplyTo":"20250702-525-make-clang-format-more-robust-v3-0-705344f30580@gmail.com","subject":"[PATCH v3 2/3] clang-format: add 'RemoveBracesLLVM' to the main config","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-07-02T09:23:19Z","receivedAt":"2025-07-02T09:23:29Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In 1b8f306612 (ci/style-check: add `RemoveBracesLLVM` in CI job,\n2024-07-23) we added 'RemoveBracesLLVM' to the CI job of running the\nclang formatter.\n\nThis rule checks and warns against using braces on simple\nsingle-statement bodies of statements. Since we haven't had any issues\nregarding this rule, we can now move it into the main clang-format\nconfig and remove it from being CI exclusive.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n .clang-format         |  6 ++++++\n ci/run-style-check.sh | 18 +-----------------\n 2 files changed, 7 insertions(+), 17 deletions(-)\n\ndiff --git a/.clang-format b/.clang-format\nindex 19d6cf4200..dcfd0aad60 100644\n--- a/.clang-format\n+++ b/.clang-format\n@@ -220,3 +220,9 @@ KeepEmptyLinesAtTheStartOfBlocks: false\n \n # Don't sort #include's\n SortIncludes: false\n+\n+# Remove optional braces of control statements (if, else, for, and while)\n+# according to the LLVM coding style. This avoids braces on simple\n+# single-statement bodies of statements but keeps braces if one side of\n+# if/else if/.../else cascade has multi-statement body.\n+RemoveBracesLLVM: true\ndiff --git a/ci/run-style-check.sh b/ci/run-style-check.sh\nindex 6cd4b1d934..0832c19df0 100755\n--- a/ci/run-style-check.sh\n+++ b/ci/run-style-check.sh\n@@ -5,21 +5,5 @@\n \n baseCommit=$1\n \n-# Remove optional braces of control statements (if, else, for, and while)\n-# according to the LLVM coding style. This avoids braces on simple\n-# single-statement bodies of statements but keeps braces if one side of\n-# if/else if/.../else cascade has multi-statement body.\n-#\n-# As this rule comes with a warning [1], we want to experiment with it\n-# before adding it in-tree. since the CI job for the style check is allowed\n-# to fail, appending the rule here allows us to validate its efficacy.\n-# While also ensuring that end-users are not affected directly.\n-#\n-# [1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm\n-{\n-\tcat .clang-format\n-\techo \"RemoveBracesLLVM: true\"\n-} >/tmp/clang-format-rules\n-\n-git clang-format --style=file:/tmp/clang-format-rules \\\n+git clang-format --style=file:.clang-format \\\n \t--diff --extensions c,h \"$baseCommit\"\n\n-- \n2.49.0\n\n"},{"id":"521157","messageId":"20250702-525-make-clang-format-more-robust-v3-3-705344f30580@gmail.com","threadId":"63693","inReplyTo":"20250702-525-make-clang-format-more-robust-v3-0-705344f30580@gmail.com","subject":"[PATCH v3 3/3] meson: add rule to run 'git clang-format'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-07-02T09:23:20Z","receivedAt":"2025-07-02T09:23:30Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The Makefile has a 'style' rule to run 'git clang-format'. While Meson\nintrinsically supports a 'clang-format' target, which can be run when\nusing the ninja backend by running 'ninja clang-format', this runs the\nformatting on all existing files.\n\nOur Meson build doesn't yet support a way to run 'git clang-format',\nwhich runs the formatter between the working directory and commit\nprovided. Add a new 'style' target to Meson to mimic the target in the\nMakefile.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n meson.build | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/meson.build b/meson.build\nindex 7fea4a34d6..48dff818e0 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -2144,6 +2144,18 @@ if headers_to_check.length() != 0 and compiler.get_argument_syntax() == 'gcc'\n   alias_target('check-headers', hdr_check)\n endif\n \n+git_clang_format = find_program('git-clang-format', required: false, native: true)\n+if git_clang_format.found()\n+  run_target('style',\n+    command: [\n+      git_clang_format,\n+      '--style', 'file',\n+      '--diff',\n+      '--extensions', 'c,h'\n+    ]\n+  )\n+endif\n+\n foreach key, value : {\n   'DIFF': diff.full_path(),\n   'GIT_SOURCE_DIR': meson.project_source_root(),\n\n-- \n2.49.0\n\n"}]}