patchdiff: use conventional comparison order
5 messages between Mar 13, 2026 and Mar 14, 2026, from aum2357, Tian Yuchen, Junio C Hamano.
Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.
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(-)
Show changes to builtin/add.c +1 −1
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
Hi aum2357,
On 3/13/26 22:04, aum2357 wrote:
Show 22 quoted lines
> 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
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
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
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) {Show 5 quoted lines
> 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.
