* [PATCH] symbolic-ref: trivial style fix @ 2013-09-21 16:29 Felipe Contreras 2013-10-15 22:16 ` Junio C Hamano 0 siblings, 1 reply; 6+ messages in thread From: Felipe Contreras @ 2013-09-21 16:29 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] symbolic-ref: trivial style fix 2013-09-21 16:29 [PATCH] symbolic-ref: trivial style fix Felipe Contreras @ 2013-10-15 22:16 ` Junio C Hamano 2013-10-15 23:16 ` Jonathan Nieder 2013-10-16 3:33 ` Felipe Contreras 0 siblings, 2 replies; 6+ messages in thread From: Junio C Hamano @ 2013-10-15 22:16 UTC (permalink / raw) To: Felipe Contreras; +Cc: git 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; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] symbolic-ref: trivial style fix 2013-10-15 22:16 ` Junio C Hamano @ 2013-10-15 23:16 ` Jonathan Nieder 2013-10-15 23:34 ` Junio C Hamano 2013-10-16 3:33 ` Felipe Contreras 1 sibling, 1 reply; 6+ messages in thread From: Jonathan Nieder @ 2013-10-15 23:16 UTC (permalink / raw) To: Junio C Hamano; +Cc: Felipe Contreras, git 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] symbolic-ref: trivial style fix 2013-10-15 23:16 ` Jonathan Nieder @ 2013-10-15 23:34 ` Junio C Hamano 0 siblings, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2013-10-15 23:34 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Felipe Contreras, git 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] symbolic-ref: trivial style fix 2013-10-15 22:16 ` Junio C Hamano 2013-10-15 23:16 ` Jonathan Nieder @ 2013-10-16 3:33 ` Felipe Contreras 2013-10-16 21:38 ` Junio C Hamano 1 sibling, 1 reply; 6+ messages in thread From: Felipe Contreras @ 2013-10-16 3:33 UTC (permalink / raw) To: Junio C Hamano, Felipe Contreras; +Cc: git 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] symbolic-ref: trivial style fix 2013-10-16 3:33 ` Felipe Contreras @ 2013-10-16 21:38 ` Junio C Hamano 0 siblings, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2013-10-16 21:38 UTC (permalink / raw) To: Felipe Contreras; +Cc: git 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-16 21:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-09-21 16:29 [PATCH] symbolic-ref: trivial style fix Felipe Contreras 2013-10-15 22:16 ` Junio C Hamano 2013-10-15 23:16 ` Jonathan Nieder 2013-10-15 23:34 ` Junio C Hamano 2013-10-16 3:33 ` Felipe Contreras 2013-10-16 21:38 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).