From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 11 Apr 2015 20:14:28 GMT
Subject: Re: [PATCH v2 2/4] combine-diff.c: refactor: extract insert_path()
Message-ID: <xmqqfv865snv.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1428076716-4449-3-git-send-email-max@max630.net>

Max Kirillov <max@max630.net> writes:

> Signed-off-by: Max Kirillov <max@max630.net>
> ---
>  combine-diff.c | 43 ++++++++++++++++++++++++-------------------
>  1 file changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/combine-diff.c b/combine-diff.c
> index 8eb7278..a236bb5 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -22,6 +22,28 @@ static int compare_paths(const struct combine_diff_path *one,
>  				 two->path, strlen(two->path), two->mode);
>  }
>  
> +static void insert_path(struct combine_diff_path **pos, const char* path, int n, int num_parent, struct diff_filepair *queue_item)

This is overlong (and I wonder why I am not seeing the linewrap
problem with this one).  I'd suggest to split them perhaps in the
same way that compare_paths() match_string_spaces() and other
existing functions are defined?

Also the pointer-asterisk sticks to the variable name, not type,
i.e.

	static void insert_path(struct combine_diff_path **pos, const char *path,
				int n, int num_parent,
                                struct diff_filepair *queue_item)


> +{