Re: [PATCH v6 6/6] ci/style-check: add `RemoveBracesLLVM` in CI job

[Junio C Hamano <gitster@pobox.com>]

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

> For 'clang-format', setting 'RemoveBracesLLVM' to 'true', adds a check
> to ensure we avoid curly braces for single-statement bodies in
> conditional blocks.
>
> However, the option does come with two warnings [1]:
>
>     This option will be renamed and expanded to support other styles.
>
> and
>
>     Setting this option to true could lead to incorrect code formatting
>     due to clang-format’s lack of complete semantic information. As
>     such, extra care should be taken to review code changes made by
>     this option.
>
> The latter seems to be of concern. While we want to experiment with the
> rule, adding it to the in-tree '.clang-format' could affect end-users.
> Let's only add it to the CI jobs for now. With time, we can evaluate
> its efficacy and decide if we want to add it to '.clang-format' or
> retract it entirely. We do so, by adding the existing rules in
> '.clang-format' and this rule to a temp file outside the working tree,
> which is then used by 'git clang-format'. This ensures we don't murk
> with files in-tree.
>
> [1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removebracesllvm
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  ci/run-style-check.sh | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)

Have we done enough experiment by now and don't we want to move it
to the set of rules in our tree?

How often do people use "git clang-format" before they submit their
series these days, by the way?

> diff --git a/ci/run-style-check.sh b/ci/run-style-check.sh
> index 76dd37d22b..6cd4b1d934 100755
> --- a/ci/run-style-check.sh
> +++ b/ci/run-style-check.sh
> @@ -5,4 +5,21 @@
>  
>  baseCommit=$1
>  
> -git clang-format --style file --diff --extensions c,h "$baseCommit"
> +# 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 \
> +	--diff --extensions c,h "$baseCommit"