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

2013-10-16 Thread Junio C Hamano
Felipe Contreras felipe.contre...@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.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-10-15 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes:

 Signed-off-by: Felipe Contreras felipe.contre...@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.contre...@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.contre...@gmail.com
Signed-off-by: Junio C Hamano gits...@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;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-10-15 Thread Jonathan Nieder
Junio C Hamano wrote:

 From: Felipe Contreras felipe.contre...@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 jrnie...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-10-15 Thread Junio C Hamano
Jonathan Nieder jrnie...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-10-15 Thread Felipe Contreras
Junio C Hamano wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:
 
  Signed-off-by: Felipe Contreras felipe.contre...@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.contre...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html