[PATCH GSOC] diff: use conventional comparison order

[PATCH GSOC] diff: use conventional comparison order

[aum2357]

diff: use conventional comparison order

Replace `0 <= addremove_explicit` with `addremove_explicit >= 0`
to follow the common coding style where variables appear on the
left side of comparisons.
---
 builtin/add.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/add.c b/builtin/add.c
index 0ee21692c2..ad0d6047af 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -432,7 +432,7 @@ int cmd_add(int argc,
 	argc--;
 	argv++;
 
-	if (0 <= addremove_explicit)
+	if ( addremove_explicit >= 0 )
 		addremove = addremove_explicit;
 	else if (take_worktree_changes && ADDREMOVE_DEFAULT)
 		addremove = 0; /* "-u" was given but not "-A" */
-- 
2.53.0

Re: [PATCH GSOC] diff: use conventional comparison order

[Tian Yuchen]

Hi aum2357,

On 3/13/26 22:04, aum2357 wrote:
> diff: use conventional comparison order
> 
> Replace `0 <= addremove_explicit` with `addremove_explicit >= 0`
> to follow the common coding style where variables appear on the
> left side of comparisons.
> ---
>   builtin/add.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/add.c b/builtin/add.c
> index 0ee21692c2..ad0d6047af 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -432,7 +432,7 @@ int cmd_add(int argc,
>   	argc--;
>   	argv++;
>   
> -	if (0 <= addremove_explicit)
> +	if ( addremove_explicit >= 0 )
>   		addremove = addremove_explicit;
>   	else if (take_worktree_changes && ADDREMOVE_DEFAULT)
>   		addremove = 0; /* "-u" was given but not "-A" */

I guess this syntax is probably intended to prevent the compiler from 
not reporting an error when someone accidentally types '='. But since 
the code is already written and runs fine, isn't it a bit unnecessary to 
change it?

You can try a command like

> git grep -E ‘\(.*<=.*\)’

to see what happens. The codebase actually includes both, right?



By the way, as far as I know, no country allows numbers to be part of a 
name. In other word, I don't think 'aum2357' is your real name ;)
If you've browsed the mailing list, you'll notice that everyone submits 
patches using their real names. Please adjust yours accordingly.

Regards,

Yuchen

Re: [PATCH GSOC] diff: use conventional comparison order

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:

> I guess this syntax is probably intended to prevent the compiler from 
> not reporting an error when someone accidentally types '='.

You guessed wrong.  See CodingGuidelines.

Re: [PATCH GSOC] diff: use conventional comparison order

[Tian Yuchen]

On 3/14/26 04:18, Junio C Hamano wrote:

> You guessed wrong.  See CodingGuidelines.

It does have nothing to do with the compiler. Thanks for pointing out.

However, the coding guidelines state:

> Both are valid, and we use both.

So, the real key is this sentence:

> Just do not mix styles in the same part of the code and mimic
> existing styles in the neighbourhood.

However, in builtin/add.c before the patch, there isn't even a single 
“>” symbol used for comparison. The “<” symbol is used throughout the 
comparison sections.

I find it quite strange because the author says:

> to follow the common coding style

This patch seems more like it breaks the common coding style.

Regards,

Yuchen

Re: [PATCH GSOC] diff: use conventional comparison order

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:
> However, in builtin/add.c before the patch, there isn't even a single 
> “>” symbol used for comparison. The “<” symbol is used throughout the 
> comparison sections.

Look a bit harder.  I think there is one comparison that uses a
comparison that does not follow "textual order reflects actual
order" convention.

	while (--i >= 0) {

> I find it quite strange because the author says:
>
>> to follow the common coding style
>
> This patch seems more like it breaks the common coding style.

To somebody who does not know both conventions and understand that
both are valid, the only one that is familiar to the person would be
the only common one.