From 9e45fc6ce54171bec645917c05b79e3455266a61 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Wed, 2 Jul 2025 11:23:18 +0200 Subject: [PATCH 1/3] clang-format: set 'ColumnLimit' to 0 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 Signed-off-by: Junio C Hamano --- .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 From 73d8380e56087ac0a4285596e74444e26d01fb89 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Wed, 2 Jul 2025 11:23:19 +0200 Subject: [PATCH 2/3] clang-format: add 'RemoveBracesLLVM' to the main config 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 Signed-off-by: Junio C Hamano --- .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" From 3f7e447aaf40724e54fe81c99bee104829e3d23d Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Wed, 2 Jul 2025 11:23:20 +0200 Subject: [PATCH 3/3] meson: add rule to run 'git clang-format' 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 Signed-off-by: Junio C Hamano --- meson.build | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/meson.build b/meson.build index 596f5ac711..7f2d7b5f8c 100644 --- a/meson.build +++ b/meson.build @@ -2132,6 +2132,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(),