[PATCH v3 0/3] clang-format: modify rules to reduce false-positives
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Jul 2, 2025, 09:23 UTC
- Message-ID
- <20250702-525-make-clang-format-more-robust-v3-0-705344f30580@gmail.com>
- In-Reply-To
- <20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com>
This series is in response to an email thread [1] around the usage of '.clang-format' and its effectiveness.
The goal of the series is to improve the usage of 'clang-format' in the repository. To do this we: 1. Reduce the number of false positives. Majority of which is due to line-wrapping. We remove the 'ColumnLimit' from 'clang-format'. This removes the responsibility of line-wrapping from 'clang-format' and puts it into the hands of the user. 2. Add a rule to 'meson' to run 'git clang-format' by running 'meson compile style'. 3. Make the 'RemoveBracesLLVM' permanent by moving it to '.clang-format'.
With this, running `git clang-format` for the last 25 commits in master, seems to produce much less false positives.
diff --git a/builtin/diff.c b/builtin/diff.c index c6231edce4..246b81caa2 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -30,14 +30,13 @@ #define DIFF_NO_INDEX_IMPLICIT 2
static const char builtin_diff_usage[] = -"git diff [<options>] [<commit>] [--] [<path>...]\n" -" or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\n" -" or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\n" -" or: git diff [<options>] <commit>...<commit> [--] [<path>...]\n" -" or: git diff [<options>] <blob> <blob>\n" -" or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]" -"\n" -COMMON_DIFF_OPTIONS_HELP; + "git diff [<options>] [<commit>] [--] [<path>...]\n" + " or: git diff [<options>] --cached [--merge-base] [<commit>] [--] [<path>...]\n" + " or: git diff [<options>] [--merge-base] <commit> [<commit>...] <commit> [--] [<path>...]\n" + " or: git diff [<options>] <commit>...<commit> [--] [<path>...]\n" + " or: git diff [<options>] <blob> <blob>\n" + " or: git diff [<options>] --no-index [--] <path> <path> [<pathspec>...]" + "\n" COMMON_DIFF_OPTIONS_HELP;
static const char *blob_path(struct object_array_entry *entry)
{
diff --git a/builtin/stash.c b/builtin/stash.c
index 7cd3ad8aa4..90e441a6e5 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1802,7 +1802,8 @@ static int push_stash(int argc, const char **argv, const char *prefix,argc = parse_options(argc, argv, prefix, options, push_assumed ? git_stash_usage : - git_stash_push_usage, flags); + git_stash_push_usage, + flags); force_assume |= patch_mode; }
diff --git a/bundle-uri.c b/bundle-uri.c
index c9d65aa0ce..89f59aafe8 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -123,7 +123,7 @@ void print_bundle_list(FILE *fp, struct bundle_list *list)
for (i = 0; i < BUNDLE_HEURISTIC__COUNT; i++) {
if (heuristics[i].heuristic == list->heuristic) {
fprintf(fp, "\theuristic = %s\n",
- heuristics[list->heuristic].name);
+ heuristics[list->heuristic].name);
break;
}
}
diff --git a/diff-no-index.c b/diff-no-index.c
index 4aeeb98cfa..a3892a9ccc 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -325,7 +325,7 @@ static int fixup_paths(const char **path, struct strbuf *replacement)
return 0;
} -static const char * const diff_no_index_usage[] = {
+static const char *const diff_no_index_usage[] = {
N_("git diff --no-index [<options>] <path> <path> [<pathspec>...]"),
NULL
};
diff --git a/pathspec.h b/pathspec.h
index 5e3a6f1fe7..601b9ca201 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -80,7 +80,7 @@ struct pathspec {
* For git diff --no-index, indicate that we are operating without
* a repository or index.
*/
-#define PATHSPEC_NO_REPOSITORY (1<<7)
+#define PATHSPEC_NO_REPOSITORY (1 << 7) /**
* Given command line arguments and a prefix, convert the input toWhile now I'm tempted to mark the 'check-style' CI job as required. I think we should do that in the future.
[1]: https://lore.kernel.org/git/xmqqmsa3adpw.fsf@gitster.g/
--- Changes in v3: - In meson.build, set 'native: true' and use the variable obtained from 'find_program()' directly in 'run_target()'. - Link to v2: https://lore.kernel.org/r/20250630-525-make-clang-format-more-robust-v2-0-05cbcdbf7817@gmail.com
Changes in v2: - Drop the patch to add 120 column length to editorconfig. This way, we will continue to use the default of 80 columns. Adding a higher column length makes editorconfig combine smaller lines during block formatting. This is not desirable. - Ensure that meson specifically checks for 'git-clang-format' and not just 'clang-format'. - Link to v1: https://lore.kernel.org/r/20250625-525-make-clang-format-more-robust-v1-0-67a49ecc2fd5@gmail.com
--- .clang-format | 27 +++++++++++++++------------ ci/run-style-check.sh | 18 +----------------- meson.build | 12 ++++++++++++ 3 files changed, 28 insertions(+), 29 deletions(-)
Karthik Nayak (3):
clang-format: set 'ColumnLimit' to 0
clang-format: add 'RemoveBracesLLVM' to the main config
meson: add rule to run 'git clang-format'Range-diff versus v2:
1: f8271d7114 = 1: c7a7f6d798 clang-format: set 'ColumnLimit' to 0
2: f49237edda = 2: c68ea68349 clang-format: add 'RemoveBracesLLVM' to the main config
3: 8d2a1647a2 ! 3: b972289ab8 meson: add rule to run 'git clang-format'
@@ meson.build: if headers_to_check.length() != 0 and compiler.get_argument_syntax(
alias_target('check-headers', hdr_check)
endif
-+git_clang_format = find_program('git-clang-format', required: false)
++git_clang_format = find_program('git-clang-format', required: false, native: true)
+if git_clang_format.found()
+ run_target('style',
+ command: [
-+ 'git', 'clang-format',
++ git_clang_format,
+ '--style', 'file',
+ '--diff',
+ '--extensions', 'c,h'base-commit: f0135a9047ca37d4d117dcf21f7e3e89fad85d00 change-id: 20250625-525-make-clang-format-more-robust-968126c991b9
Thanks - Karthik