diff mbox series

[v3,2/3] clang-format: align consecutive macro definitions

Message ID 1586d5376915a4662c92b8c0881596952c4500bb.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 generally align consecutive macro definitions for better readability:

  #define OUTPUT_ANNOTATE_COMPAT      (1U<<0)
  #define OUTPUT_LONG_OBJECT_NAME     (1U<<1)
  #define OUTPUT_RAW_TIMESTAMP        (1U<<2)
  #define OUTPUT_PORCELAIN            (1U<<3)

over

  #define OUTPUT_ANNOTATE_COMPAT (1U<<0)
  #define OUTPUT_LONG_OBJECT_NAME (1U<<1)
  #define OUTPUT_RAW_TIMESTAMP (1U<<2)
  #define OUTPUT_PORCELAIN (1U<<3)

So let's add the rule in clang-format to follow this.

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

Comments

Kyle Lippincott Oct. 14, 2024, 9:12 p.m. UTC | #1
On Fri, Oct 11, 2024 at 6:50 PM Karthik Nayak <karthik.188@gmail.com> wrote:
>
> We generally align consecutive macro definitions for better readability:
>
>   #define OUTPUT_ANNOTATE_COMPAT      (1U<<0)
>   #define OUTPUT_LONG_OBJECT_NAME     (1U<<1)
>   #define OUTPUT_RAW_TIMESTAMP        (1U<<2)
>   #define OUTPUT_PORCELAIN            (1U<<3)

I like this change, thanks. Is there a way of apply clang-format for
*only* one rule/aspect? i.e. can we apply *only* this, and preserve
every other line? At first glance, I don't see a way of doing it. If
there was, I might recommend a whole series just to applying these
changes, but with how out of compliance much of the codebase is today,
that's not going to be feasible; we'd need to format it in a way that
we might not want (the current style), and then fix it, and that seems
counterproductive.

>
> over
>
>   #define OUTPUT_ANNOTATE_COMPAT (1U<<0)
>   #define OUTPUT_LONG_OBJECT_NAME (1U<<1)
>   #define OUTPUT_RAW_TIMESTAMP (1U<<2)
>   #define OUTPUT_PORCELAIN (1U<<3)
>
> So let's add the rule in clang-format to follow this.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  .clang-format | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/.clang-format b/.clang-format
> index 66a2360ae5..9547fe1b77 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -32,6 +32,9 @@ AlignConsecutiveAssignments: false
>  # double b = 3.14;
>  AlignConsecutiveDeclarations: false
>
> +# Align consecutive macro definitions.
> +AlignConsecutiveMacros: true
> +
>  # Align escaped newlines as far left as possible
>  # #define A   \
>  #   int aaaa; \
> --
> 2.47.0
>
karthik nayak Oct. 15, 2024, 7:57 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 generally align consecutive macro definitions for better readability:
>>
>>   #define OUTPUT_ANNOTATE_COMPAT      (1U<<0)
>>   #define OUTPUT_LONG_OBJECT_NAME     (1U<<1)
>>   #define OUTPUT_RAW_TIMESTAMP        (1U<<2)
>>   #define OUTPUT_PORCELAIN            (1U<<3)
>
> I like this change, thanks. Is there a way of apply clang-format for
> *only* one rule/aspect? i.e. can we apply *only* this, and preserve
> every other line? At first glance, I don't see a way of doing it. If
> there was, I might recommend a whole series just to applying these
> changes, but with how out of compliance much of the codebase is today,
> that's not going to be feasible; we'd need to format it in a way that
> we might not want (the current style), and then fix it, and that seems
> counterproductive.
>

I think we can apply a single rule by specifying the rule over the CLI.

Overall I think its best to take things iteratively. Also because
'clang-format' is a bit rough around the edges and we might discover
some other changes needed.
diff mbox series

Patch

diff --git a/.clang-format b/.clang-format
index 66a2360ae5..9547fe1b77 100644
--- a/.clang-format
+++ b/.clang-format
@@ -32,6 +32,9 @@  AlignConsecutiveAssignments: false
 # double b = 3.14;
 AlignConsecutiveDeclarations: false
 
+# Align consecutive macro definitions.
+AlignConsecutiveMacros: true
+
 # Align escaped newlines as far left as possible
 # #define A   \
 #   int aaaa; \