Thanks for the patch, and sorry for the late response.
I have just a couple of notes below...
On Fri, Dec 06, 2013 at 10:38:46AM +1100, Zoltan Klinger wrote:
> diff --git a/diff.c b/diff.c
> index e34bf97..a7d5a47 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2899,11 +2899,16 @@ static void run_external_diff(const char *pgm,
> struct diff_filespec *one,
> struct diff_filespec *two,
> const char *xfrm_msg,
> - int complete_rewrite)
> + int complete_rewrite,
> + struct diff_options *o)
Very minor nit -- "o" is a very terse variable name.
Maybe "opts"?
> {
> const char *spawn_arg[10];
> int retval;
> const char **arg = &spawn_arg[0];
> + struct diff_queue_struct *q = &diff_queued_diff;
> + const char *env[3] = { NULL };
> + char env_counter[50];
> + char env_total[50];
Hard-coded 50; what's the length of the maximum signed int?
> diff --git a/diff.h b/diff.h
> index e342325..42bd34c 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -164,6 +164,8 @@ struct diff_options {
> diff_prefix_fn_t output_prefix;
> int output_prefix_length;
> void *output_prefix_data;
> +
> + int diff_path_counter;
> };
Since these are already "diff_options" it seems redundant to call
the struct entry the "diff_path_counter" when "path_count"
should be specific enough. Would it make sense to rename it?
These are tiny nitpicky style notes; it looks good otherwise.
Thanks,
--
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
