[PATCH] symbolic-ref: trivial style fix

[PATCH] symbolic-ref: trivial style fix

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/symbolic-ref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index f481959..71286b4 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -47,7 +47,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, options,
 			     git_symbolic_ref_usage, 0);
-	if (msg &&!*msg)
+	if (msg && !*msg)
 		die("Refusing to perform update with empty message");
 
 	if (delete) {
-- 
1.8.4-fc

Re: [PATCH] symbolic-ref: trivial style fix

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---

Let's do something like this instead.

 - We usually refrain from making such a tree-wide change in order
   to avoid unnecessary conflicts with other "real work" patches,
   but the end result does not have a potentially cumbersome
   tree-wide impact.

 - We also tend to frown upon an "I fixed it here only because I
   happened to notice this one, there may be others but it is up to
   the readers to see if there are other instances" half-assed code
   churn.

The point of the proposed log message is to tell readers that this
is a tree-wide clean-up that is worth applying.

-- >8 --
From: Felipe Contreras <felipe.contreras@gmail.com>
Subject: C: have space around && and || operators

Correct all hits from

    git grep -e '\(&&\|||\)[^ ]' -e '[^	 ]\(&&\|||\)' -- '*.c'

i.e. && or || operators that are followed by anything but a SP,
or that follow something other than a SP or a HT, so that these
operators have a SP around it when necessary.

We usually refrain from making this kind of a tree-wide change in
order to avoid unnecessary conflicts with other "real work" patches,
but the end result does not have a potentially cumbersome tree-wide
impact.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/read-tree.c    | 2 +-
 builtin/rev-list.c     | 2 +-
 builtin/symbolic-ref.c | 2 +-
 compat/regex/regcomp.c | 2 +-
 xdiff/xemit.c          | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 0f5d7fe..0d7ef84 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -178,7 +178,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 
 	if (1 < opts.index_only + opts.update)
 		die("-u and -i at the same time makes no sense");
-	if ((opts.update||opts.index_only) && !opts.merge)
+	if ((opts.update || opts.index_only) && !opts.merge)
 		die("%s is meaningless without -m, --reset, or --prefix",
 		    opts.update ? "-u" : "-i");
 	if ((opts.dir && !opts.update))
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 4fc1616..0745e2d 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -322,7 +322,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 		revs.commit_format = CMIT_FMT_RAW;
 
 	if ((!revs.commits &&
-	     (!(revs.tag_objects||revs.tree_objects||revs.blob_objects) &&
+	     (!(revs.tag_objects || revs.tree_objects || revs.blob_objects) &&
 	      !revs.pending.nr)) ||
 	    revs.diff)
 		usage(rev_list_usage);
diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index f481959..71286b4 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -47,7 +47,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, options,
 			     git_symbolic_ref_usage, 0);
-	if (msg &&!*msg)
+	if (msg && !*msg)
 		die("Refusing to perform update with empty message");
 
 	if (delete) {
diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c
index b2c5d46..06f3088 100644
--- a/compat/regex/regcomp.c
+++ b/compat/regex/regcomp.c
@@ -339,7 +339,7 @@ re_compile_fastmap_iter (regex_t *bufp, const re_dfastate_t *init_state,
 	      p = buf;
 	      *p++ = dfa->nodes[node].opr.c;
 	      while (++node < dfa->nodes_len
-		     &&	dfa->nodes[node].type == CHARACTER
+		     && dfa->nodes[node].type == CHARACTER
 		     && dfa->nodes[node].mb_partial)
 		*p++ = dfa->nodes[node].opr.c;
 	      memset (&state, '\0', sizeof (state));
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 4d86458..4266ada 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -108,7 +108,7 @@ static long def_ff(const char *rec, long len, char *buf, long sz, void *priv)
 {
 	if (len > 0 &&
 			(isalpha((unsigned char)*rec) || /* identifier? */
-			 *rec == '_' ||	/* also identifier? */
+			 *rec == '_' || /* also identifier? */
 			 *rec == '$')) { /* identifiers from VMS and other esoterico */
 		if (len > sz)
 			len = sz;

Re: [PATCH] symbolic-ref: trivial style fix

[Jonathan Nieder]

Junio C Hamano wrote:

> From: Felipe Contreras <felipe.contreras@gmail.com>
> Subject: C: have space around && and || operators
[...]
>  builtin/read-tree.c    | 2 +-
>  builtin/rev-list.c     | 2 +-
>  builtin/symbolic-ref.c | 2 +-
>  compat/regex/regcomp.c | 2 +-
>  xdiff/xemit.c          | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)

For what it's worth,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

[...]
> --- a/compat/regex/regcomp.c
> +++ b/compat/regex/regcomp.c
> @@ -339,7 +339,7 @@ re_compile_fastmap_iter (regex_t *bufp, const re_dfastate_t *init_state,
>  	      p = buf;
>  	      *p++ = dfa->nodes[node].opr.c;
>  	      while (++node < dfa->nodes_len
> -		     &&	dfa->nodes[node].type == CHARACTER
> +		     && dfa->nodes[node].type == CHARACTER

It took a little staring to see what changed here.  The preimage has
a tab, probably from an autoformatter gone wild.  I don't think fixing
it should interfere with importing new versions of compat/regex, so
the change seems fine.

Thanks,
Jonathan

Re: [PATCH] symbolic-ref: trivial style fix

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

>> -		     &&	dfa->nodes[node].type == CHARACTER
>> +		     && dfa->nodes[node].type == CHARACTER
>
> It took a little staring to see what changed here.  The preimage has
> a tab, probably from an autoformatter gone wild.  I don't think fixing
> it should interfere with importing new versions of compat/regex, so
> the change seems fine.

xdiff/xemit.c had the breakage of the same nature. Perhaps the
commit log message should mention these two.

Re: [PATCH] symbolic-ref: trivial style fix

[Felipe Contreras]

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> 
> Let's do something like this instead.
> 
>  - We usually refrain from making such a tree-wide change in order
>    to avoid unnecessary conflicts with other "real work" patches,
>    but the end result does not have a potentially cumbersome
>    tree-wide impact.

You might want to change that phrasing to "we usually give low priority". Sure,
this is low priority, but this is something has to be done sooner or later, if
we don't the code-style will be forever inconsistent.

If there is a conflict it would be sensible to delay the change for another
release perhaps, it's perfectly sensible to ask the submitter to set another
version, and hopefully there would be no conflict.

Usually resolving the conflict is trivial, but there's a lot of options.

Refraining from making a code-style fix is not ideal. We want those. It's only
a matter of how to implement them.

>  - We also tend to frown upon an "I fixed it here only because I
>    happened to notice this one, there may be others but it is up to
>    the readers to see if there are other instances" half-assed code
>    churn.

If you have more general patch, sure, but would you really reject a patch that
fixes one thing, because it doesn't fix all similar things at the same time?

> The point of the proposed log message is to tell readers that this
> is a tree-wide clean-up that is worth applying.
> 
> -- >8 --
> From: Felipe Contreras <felipe.contreras@gmail.com>
> Subject: C: have space around && and || operators

Saying this patch is from me is not really accurate, it's based on a patch by
me, or it was reported by me, but it's not really from me. I don't have a
problem taking the credit though, as probably half the change is finding out
there's a problem, just saying.

-- 
Felipe Contreras

Re: [PATCH] symbolic-ref: trivial style fix

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Saying this patch is from me is not really accurate, it's based on a patch by
> me, or it was reported by me, but it's not really from me.

OK, will reword.

Thanks.