Re: [RFC PATCH 3/5] format-patch: extend --range-diff to accept revision range

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 17, 2018, 10:44 UTC
Message-ID: <nycvar.QRO.7.76.6.1807171243410.71@tvgsbejvaqbjf.bet>
In-Reply-To: <20180530080325.37520-4-sunshine@sunshineco.com>

Hi Eric,

On Wed, 30 May 2018, Eric Sunshine wrote:

Show 24 quoted lines

> diff --git a/builtin/log.c b/builtin/log.c
> index 460c31a293..e38cf06050 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -995,10 +995,20 @@ static char *find_branch_name(struct rev_info *rev)
>  
>  static void infer_diff_ranges(struct argv_array *args,
>  			      const char *prev,
> +			      struct commit *origin,
>  			      struct commit *head)
>  {
> -	argv_array_pushf(args, "%s...%s", prev,
> -			 oid_to_hex(&head->object.oid));
> +	if (strstr(prev, "..")) {
> +		if (!origin)
> +			die(_("failed to infer range-diff ranges"));
> +		argv_array_push(args, prev);
> +		argv_array_pushf(args, "%s..%s",
> +				 oid_to_hex(&origin->object.oid),
> +				 oid_to_hex(&head->object.oid));
> +	} else {
> +		argv_array_pushf(args, "%s...%s", prev,
> +				 oid_to_hex(&head->object.oid));
> +	}

This would be easier to read if the order was inverted:

	if (!strstr(...))
		...
	else if (!origin)
		die(...)
	else {
		...
	}

Otherwise, it makes sense to me.

Thanks, Dscho

Show 27 quoted lines

>  }
>  
>  static int get_range_diff(struct strbuf *sb,
> @@ -1059,7 +1069,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
>  	/* might die from bad user input so try before creating cover letter */
>  	if (range_diff) {
>  		struct argv_array ranges = ARGV_ARRAY_INIT;
> -		infer_diff_ranges(&ranges, range_diff, head);
> +		infer_diff_ranges(&ranges, range_diff, origin, head);
>  		if (get_range_diff(&diff, &ranges))
>  			die(_("failed to generate range-diff"));
>  		argv_array_clear(&ranges);
> diff --git a/t/t7910-branch-diff.sh b/t/t7910-branch-diff.sh
> index edbd69b6f8..c0e8668ed9 100755
> --- a/t/t7910-branch-diff.sh
> +++ b/t/t7910-branch-diff.sh
> @@ -155,5 +155,6 @@ format_patch () {
>  }
>  
>  format_patch 'B...C' topic
> +format_patch 'A..B A..C' master..topic
>  
>  test_done
> -- 
> 2.17.1.1235.ge295dfb56e
> 
>