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

From: Stefan Beller <[email-masked]>
Date: Sat, 12 Oct 2013 11:08:16 +0200

On 10/12/2013 09:07 AM, Felipe Contreras wrote:
> Signed-off-by: Felipe Contreras <[email-masked]>
> ---
>  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