diff mbox series

[v3,3/3] clang-format: don't align expressions after linebreaks

Message ID 36a53299c1ab1b55a09b7e1d499832e6715ebaba.1728697428.git.karthik.188@gmail.com (mailing list archive)
State New
Headers show
Series clang-format: fix rules to make the CI job cleaner | expand

Commit Message

karthik nayak Oct. 12, 2024, 1:49 a.m. UTC
We enforce alignment of expressions after linebreaks. Which means for
code such as

    return a || b;

it will expect:

   return a ||
          b;

we instead want 'b' to be indent with tabs, which is already done by the
'ContinuationIndentWidth' variable. So let's explicitly set
'AlignOperands' to false.

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

Comments

Kyle Lippincott Oct. 14, 2024, 9:23 p.m. UTC | #1
On Fri, Oct 11, 2024 at 6:50 PM Karthik Nayak <karthik.188@gmail.com> wrote:
>
> We enforce alignment of expressions after linebreaks. Which means for
> code such as
>
>     return a || b;
>
> it will expect:
>
>    return a ||
>           b;
>
> we instead want 'b' to be indent with tabs, which is already done by the
> 'ContinuationIndentWidth' variable.

Why do we want `b` to be indented by 8 columns instead of aligned? I
think this is harder to read:

int some_int_variable = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
        bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb;

Of course, this is even better, if it fits in 80 cols:

int some_int_variable =
        aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb;

> So let's explicitly set
> 'AlignOperands' to false.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  .clang-format | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/.clang-format b/.clang-format
> index 9547fe1b77..b48e7813e4 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -42,10 +42,9 @@ AlignConsecutiveMacros: true
>  #   int cccccccc;
>  AlignEscapedNewlines: Left
>
> -# Align operands of binary and ternary expressions
> -# int aaa = bbbbbbbbbbb +
> -#           cccccc;
> -AlignOperands: true
> +# Don't enforce alignment after linebreaks and instead
> +# rely on the ContinuationIndentWidth value.
> +AlignOperands: false
>
>  # Don't align trailing comments
>  # int a; // Comment a
> --
> 2.47.0
>
karthik nayak Oct. 15, 2024, 11:17 a.m. UTC | #2
Kyle Lippincott <spectral@google.com> writes:

> On Fri, Oct 11, 2024 at 6:50 PM Karthik Nayak <karthik.188@gmail.com> wrote:
>>
>> We enforce alignment of expressions after linebreaks. Which means for
>> code such as
>>
>>     return a || b;
>>
>> it will expect:
>>
>>    return a ||
>>           b;
>>
>> we instead want 'b' to be indent with tabs, which is already done by the
>> 'ContinuationIndentWidth' variable.
>
> Why do we want `b` to be indented by 8 columns instead of aligned? I
> think this is harder to read:
>
> int some_int_variable = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
>         bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb;
>

The reason I added this is because by default editors will follow the
'.editorconfig' and follow the 'tab_width = 8' rule when there is a line
break.

This means more often than not, most patches don't follow this rule. We
don't enforce clang-format at this point so it makes more sense to align
the rule to what everyone is doing. The goal being that once we have a
good set of base rules with less false positives, we can start
enforcing.

> Of course, this is even better, if it fits in 80 cols:
>
> int some_int_variable =
>         aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb;
>

I'm sure we can tweak this with penalties ;) But I'd say this is
something we can tune later.
diff mbox series

Patch

diff --git a/.clang-format b/.clang-format
index 9547fe1b77..b48e7813e4 100644
--- a/.clang-format
+++ b/.clang-format
@@ -42,10 +42,9 @@  AlignConsecutiveMacros: true
 #   int cccccccc;
 AlignEscapedNewlines: Left
 
-# Align operands of binary and ternary expressions
-# int aaa = bbbbbbbbbbb +
-#           cccccc;
-AlignOperands: true
+# Don't enforce alignment after linebreaks and instead
+# rely on the ContinuationIndentWidth value.
+AlignOperands: false
 
 # Don't align trailing comments
 # int a; // Comment a