diff mbox series

[6/8] clang-format: formalize some of the spacing rules

Message ID 20240708092317.267915-7-karthik.188@gmail.com (mailing list archive)
State Superseded
Headers show
Series clang-format: add more rules and enable on CI | expand

Commit Message

karthik nayak July 8, 2024, 9:23 a.m. UTC
There are some spacing rules that we follow in the project and it makes
sen to formalize them:
* Ensure there is no space inserted after the logical not '!' operator.
* Ensure there is no space before the case statement's color.
* Ensure there is no space before the first bracket '[' of an array.
* Ensure there is no space in empty blocks.

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

Comments

Junio C Hamano July 8, 2024, 4:54 p.m. UTC | #1
Karthik Nayak <karthik.188@gmail.com> writes:

> There are some spacing rules that we follow in the project and it makes
> sen to formalize them:
> * Ensure there is no space inserted after the logical not '!' operator.

Shouldn't the rule be more like "no space between any single operand
prefix or postfix operator and its operand"?  "foo++", "--foo", "~0"
are the examples that come to my mind.

> * Ensure there is no space before the case statement's color.

"color" -> "colon".

> * Ensure there is no space before the first bracket '[' of an array.
> * Ensure there is no space in empty blocks.

Hmph, I actually thought we preferred to be more explicit, using

	if (foo)
		; /* nothing */

instead of any of

	if (foo) {}
	if (foo) { }
	if (foo) { ; }
	if (foo) { ; /* nothing */ }

to write an empty statement.

> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  .clang-format | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/.clang-format b/.clang-format
> index 1a5f0c9046..05036f610b 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -126,11 +126,18 @@ RemoveBracesLLVM: true
>  # x = (int32)y;    not    x = (int32) y;
>  SpaceAfterCStyleCast: false
>  
> +# No space is inserted after the logical not operator
> +SpaceAfterLogicalNot: false
> +
>  # Insert spaces before and after assignment operators
>  # int a = 5;    not    int a=5;
>  # a += 42;             a+=42;
>  SpaceBeforeAssignmentOperators: true
>  
> +# Spaces will be removed before case colon.
> +# case 1: break;    not     case 1 : break;
> +SpaceBeforeCaseColon: false
> +
>  # Put a space before opening parentheses only after control statement keywords.
>  # void f() {
>  #   if (true) {
> @@ -139,6 +146,13 @@ SpaceBeforeAssignmentOperators: true
>  # }
>  SpaceBeforeParens: ControlStatements
>  
> +# No space before first '[' in arrays
> +# int a[5][5];     not      int a [5][5];
> +SpaceBeforeSquareBrackets: false
> +
> +# No space will be inserted into {}
> +# while (true) {}    not    while (true) { }
> +SpaceInEmptyBlock: false
>  
>  # The number of spaces before trailing line comments (// - comments).
>  # This does not affect trailing block comments (/* - comments).
Justin Tobler July 8, 2024, 4:56 p.m. UTC | #2
On 24/07/08 11:23AM, Karthik Nayak wrote:
> There are some spacing rules that we follow in the project and it makes
> sen to formalize them:
> * Ensure there is no space inserted after the logical not '!' operator.
> * Ensure there is no space before the case statement's color.

s/color/colon

> * Ensure there is no space before the first bracket '[' of an array.
> * Ensure there is no space in empty blocks.
karthik nayak July 8, 2024, 5:26 p.m. UTC | #3
Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> There are some spacing rules that we follow in the project and it makes
>> sen to formalize them:
>> * Ensure there is no space inserted after the logical not '!' operator.
>
> Shouldn't the rule be more like "no space between any single operand
> prefix or postfix operator and its operand"?  "foo++", "--foo", "~0"
> are the examples that come to my mind.
>

The rule here is SpaceAfterLogicalNot [1], is specific to logical not
operator. Unfortunately I couldn't find a general rule for unary
operators. That would be very useful indeed.

>> * Ensure there is no space before the case statement's color.
>
> "color" -> "colon".
>

Will fix, thanks!

>> * Ensure there is no space before the first bracket '[' of an array.
>> * Ensure there is no space in empty blocks.
>
> Hmph, I actually thought we preferred to be more explicit, using
>
> 	if (foo)
> 		; /* nothing */
>
> instead of any of
>
> 	if (foo) {}
> 	if (foo) { }
> 	if (foo) { ; }
> 	if (foo) { ; /* nothing */ }
>
> to write an empty statement.
>

Yup, that is correct. This rule doesn't state that we need to use 'if (foo) {}'
over the more explicit format. It only states that if we do create an
empty block without statements, the rule is to have no spaces.

We only have a few instances of this in our code.

[1]: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#spaceafterlogicalnot

[snip]
karthik nayak July 8, 2024, 5:27 p.m. UTC | #4
Justin Tobler <jltobler@gmail.com> writes:

> On 24/07/08 11:23AM, Karthik Nayak wrote:
>> There are some spacing rules that we follow in the project and it makes
>> sen to formalize them:
>> * Ensure there is no space inserted after the logical not '!' operator.
>> * Ensure there is no space before the case statement's color.
>
> s/color/colon
>

Thanks, will fix in the next version!

>> * Ensure there is no space before the first bracket '[' of an array.
>> * Ensure there is no space in empty blocks.
Eric Sunshine July 8, 2024, 8:53 p.m. UTC | #5
On Mon, Jul 8, 2024 at 5:24 AM Karthik Nayak <karthik.188@gmail.com> wrote:
> There are some spacing rules that we follow in the project and it makes
> sen to formalize them:

Since nobody else pointed it out: s/sen/sense/
karthik nayak July 10, 2024, 1:36 p.m. UTC | #6
Eric Sunshine <sunshine@sunshineco.com> writes:

> On Mon, Jul 8, 2024 at 5:24 AM Karthik Nayak <karthik.188@gmail.com> wrote:
>> There are some spacing rules that we follow in the project and it makes
>> sen to formalize them:
>
> Since nobody else pointed it out: s/sen/sense/

Will fix in v2.
diff mbox series

Patch

diff --git a/.clang-format b/.clang-format
index 1a5f0c9046..05036f610b 100644
--- a/.clang-format
+++ b/.clang-format
@@ -126,11 +126,18 @@  RemoveBracesLLVM: true
 # x = (int32)y;    not    x = (int32) y;
 SpaceAfterCStyleCast: false
 
+# No space is inserted after the logical not operator
+SpaceAfterLogicalNot: false
+
 # Insert spaces before and after assignment operators
 # int a = 5;    not    int a=5;
 # a += 42;             a+=42;
 SpaceBeforeAssignmentOperators: true
 
+# Spaces will be removed before case colon.
+# case 1: break;    not     case 1 : break;
+SpaceBeforeCaseColon: false
+
 # Put a space before opening parentheses only after control statement keywords.
 # void f() {
 #   if (true) {
@@ -139,6 +146,13 @@  SpaceBeforeAssignmentOperators: true
 # }
 SpaceBeforeParens: ControlStatements
 
+# No space before first '[' in arrays
+# int a[5][5];     not      int a [5][5];
+SpaceBeforeSquareBrackets: false
+
+# No space will be inserted into {}
+# while (true) {}    not    while (true) { }
+SpaceInEmptyBlock: false
 
 # The number of spaces before trailing line comments (// - comments).
 # This does not affect trailing block comments (/* - comments).