Re: [PATCH 16/20] revision: trivial style fixes

2013-10-12 Thread Felipe Contreras
Stefan Beller wrote:
> While talking about trivia, what is the projects stance on
> such constructs here?
>   while (foo)
>   bar();
>   foobar();
> 
> I've seen an empty line between the bar(); and the foobar(); often, which 
> suits readability.
> What's the stance on such an empty line here?

I don't think there's any guideline on that. Personally I choose depending on
the context.


void test(void)
{
while (foo)
bar();
foobar();
}



while (meh) {
/*
 * bunch of stuff
 */
}

while (foo)
bar();

foobar();


Cheers.

-- 
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


Re: [PATCH 16/20] revision: trivial style fixes

2013-10-12 Thread Stefan Beller
On 10/12/2013 09:07 AM, Felipe Contreras wrote:
> Signed-off-by: Felipe Contreras 
> ---
>  revision.c | 14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/revision.c b/revision.c
> index 0173e01..4049867 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1503,7 +1503,7 @@ struct cmdline_pathspec {
>  static void append_prune_data(struct cmdline_pathspec *prune, const char 
> **av)
>  {
>   while (*av) {
> - ALLOC_GROW(prune->path, prune->nr+1, prune->alloc);
> + ALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);
>   prune->path[prune->nr++] = *(av++);
>   }
>  }
> @@ -1515,7 +1515,7 @@ static void read_pathspec_from_stdin(struct rev_info 
> *revs, struct strbuf *sb,
>   int len = sb->len;
>   if (len && sb->buf[len - 1] == '\n')
>   sb->buf[--len] = '\0';
> - ALLOC_GROW(prune->path, prune->nr+1, prune->alloc);
> + ALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);
>   prune->path[prune->nr++] = xstrdup(sb->buf);
>   }
>  }
> @@ -2118,7 +2118,7 @@ int setup_revisions(int argc, const char **argv, struct 
> rev_info *revs, struct s
>*  call init_pathspec() to set revs->prune_data here.
>* }
>*/
> - ALLOC_GROW(prune_data.path, prune_data.nr+1, prune_data.alloc);
> + ALLOC_GROW(prune_data.path, prune_data.nr + 1, 
> prune_data.alloc);
>   prune_data.path[prune_data.nr++] = NULL;
>   parse_pathspec(&revs->prune_data, 0, 0,
>  revs->prefix, prune_data.path);
> @@ -2971,7 +2971,7 @@ static struct commit *get_revision_internal(struct 
> rev_info *revs)
>   if (revs->max_count) {
>   c = get_revision_1(revs);
>   if (c) {
> - while (0 < revs->skip_count) {
> + while (revs->skip_count > 0) {
>   revs->skip_count--;
>   c = get_revision_1(revs);
>   if (!c)
> @@ -2986,9 +2986,8 @@ static struct commit *get_revision_internal(struct 
> rev_info *revs)
>   if (c)
>   c->object.flags |= SHOWN;
>  
> - if (!revs->boundary) {
> + if (!revs->boundary)
>   return c;
> - }
>  
>   if (!c) {
>   /*
> @@ -3034,9 +3033,8 @@ struct commit *get_revision(struct rev_info *revs)
>  
>   if (revs->reverse) {
>   reversed = NULL;
> - while ((c = get_revision_internal(revs))) {
> + while ((c = get_revision_internal(revs)))
>   commit_list_insert(c, &reversed);
> - }

While talking about trivia, what is the projects stance on
such constructs here?
while (foo)
bar();
foobar();

I've seen an empty line between the bar(); and the foobar(); often, which suits 
readability.
What's the stance on such an empty line here?

>   revs->commits = reversed;
>   revs->reverse = 0;
>   revs->reverse_output_stage = 1;
> 

Thanks,
Stefan

--
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


[PATCH 16/20] revision: trivial style fixes

2013-10-12 Thread Felipe Contreras
Signed-off-by: Felipe Contreras 
---
 revision.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/revision.c b/revision.c
index 0173e01..4049867 100644
--- a/revision.c
+++ b/revision.c
@@ -1503,7 +1503,7 @@ struct cmdline_pathspec {
 static void append_prune_data(struct cmdline_pathspec *prune, const char **av)
 {
while (*av) {
-   ALLOC_GROW(prune->path, prune->nr+1, prune->alloc);
+   ALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);
prune->path[prune->nr++] = *(av++);
}
 }
@@ -1515,7 +1515,7 @@ static void read_pathspec_from_stdin(struct rev_info 
*revs, struct strbuf *sb,
int len = sb->len;
if (len && sb->buf[len - 1] == '\n')
sb->buf[--len] = '\0';
-   ALLOC_GROW(prune->path, prune->nr+1, prune->alloc);
+   ALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);
prune->path[prune->nr++] = xstrdup(sb->buf);
}
 }
@@ -2118,7 +2118,7 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
 *  call init_pathspec() to set revs->prune_data here.
 * }
 */
-   ALLOC_GROW(prune_data.path, prune_data.nr+1, prune_data.alloc);
+   ALLOC_GROW(prune_data.path, prune_data.nr + 1, 
prune_data.alloc);
prune_data.path[prune_data.nr++] = NULL;
parse_pathspec(&revs->prune_data, 0, 0,
   revs->prefix, prune_data.path);
@@ -2971,7 +2971,7 @@ static struct commit *get_revision_internal(struct 
rev_info *revs)
if (revs->max_count) {
c = get_revision_1(revs);
if (c) {
-   while (0 < revs->skip_count) {
+   while (revs->skip_count > 0) {
revs->skip_count--;
c = get_revision_1(revs);
if (!c)
@@ -2986,9 +2986,8 @@ static struct commit *get_revision_internal(struct 
rev_info *revs)
if (c)
c->object.flags |= SHOWN;
 
-   if (!revs->boundary) {
+   if (!revs->boundary)
return c;
-   }
 
if (!c) {
/*
@@ -3034,9 +3033,8 @@ struct commit *get_revision(struct rev_info *revs)
 
if (revs->reverse) {
reversed = NULL;
-   while ((c = get_revision_internal(revs))) {
+   while ((c = get_revision_internal(revs)))
commit_list_insert(c, &reversed);
-   }
revs->commits = reversed;
revs->reverse = 0;
revs->reverse_output_stage = 1;
-- 
1.8.4-fc

--
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