[v4,0/3] range-diff fixes

Message

Ævar Arnfjörð Bjarmason Nov. 9, 2018, 10:18 a.m. UTC

Addresses feedback on v3, especially Eric's suggestion to split out
the behavior change (which I was not aware of) into a 3/3. Diff with
v3:

1:  23295d7806 ! 1:  5399e57513 range-diff doc: add a section about output stability
    @@ -29,11 +29,11 @@
     +be machine-readable.
     +
     +This is particularly true when passing in diff options. Currently some
    -+options like `--stat` can as an emergent effect produce output that's
    -+quite useless in the context of `range-diff`. Future versions of
    -+`range-diff` may learn to interpret such options in a manner specifc
    -+to `range-diff` (e.g. for `--stat` summarizing how the diffstat
    -+changed).
    ++options like `--stat` can, as an emergent effect, produce output
    ++that's quite useless in the context of `range-diff`. Future versions
    ++of `range-diff` may learn to interpret such options in a manner
    ++specific to `range-diff` (e.g. for `--stat` producing human-readable
    ++output which summarizes how the diffstat changed).
      
      CONFIGURATION
      -------------
2:  b21bd273f5 ! 2:  e56975df6c range-diff: fix regression in passing along diff options
    @@ -4,8 +4,10 @@
     
         In 73a834e9e2 ("range-diff: relieve callers of low-level configuration
         burden", 2018-07-22) we broke passing down options like --no-patch,
    -    --stat etc. Fix that regression, and add a test for some of these
    -    options being passed down.
    +    --stat etc.
    +
    +    Fix that regression, and add a test asserting the pre-73a834e9e2
    +    behavior for some of these diff options.
     
         As noted in a change leading up to this ("range-diff doc: add a
         section about output stability", 2018-11-07) the output is not meant
    @@ -14,7 +16,9 @@
     
         See
         https://public-inbox.org/git/[e-mail]/
    -    for a further explanation of the regression.
    +    for a further explanation of the regression. The fix here is not the
    +    same as in Johannes's on-list patch, for reasons that'll be explained
    +    in a follow-up commit.
     
         The quoting of "EOF" here mirrors that of an earlier test. Perhaps
         that should be fixed, but let's leave that up to a later cleanup
    @@ -30,8 +34,7 @@
      
      		memcpy(&opts, diffopt, sizeof(opts));
     -		opts.output_format = DIFF_FORMAT_PATCH;
    -+		if (!opts.output_format)
    -+			opts.output_format = DIFF_FORMAT_PATCH;
    ++		opts.output_format |= DIFF_FORMAT_PATCH;
      		opts.flags.suppress_diff_headers = 1;
      		opts.flags.dual_color_diffed_diffs = dual_color;
      		opts.output_prefix = output_prefix_cb;
    @@ -55,20 +58,43 @@
     +'
     +
     +test_expect_success 'changed commit with --stat diff option' '
    ++	four_spaces="    " &&
     +	git range-diff --no-color --stat topic...changed >actual &&
     +	cat >expected <<-EOF &&
     +	1:  4de457d = 1:  a4b3333 s/5/A/
     +	     a => b | 0
     +	     1 file changed, 0 insertions(+), 0 deletions(-)
    ++	$four_spaces
     +	2:  fccce22 = 2:  f51d370 s/4/A/
     +	     a => b | 0
     +	     1 file changed, 0 insertions(+), 0 deletions(-)
    ++	$four_spaces
     +	3:  147e64e ! 3:  0559556 s/11/B/
     +	     a => b | 0
     +	     1 file changed, 0 insertions(+), 0 deletions(-)
    ++	$four_spaces
    ++	    @@ -10,7 +10,7 @@
    ++	      9
    ++	      10
    ++	     -11
    ++	    -+B
    ++	    ++BB
    ++	      12
    ++	      13
    ++	      14
     +	4:  a63e992 ! 4:  d966c5c s/12/B/
     +	     a => b | 0
     +	     1 file changed, 0 insertions(+), 0 deletions(-)
    ++	$four_spaces
    ++	    @@ -8,7 +8,7 @@
    ++	     @@
    ++	      9
    ++	      10
    ++	    - B
    ++	    + BB
    ++	     -12
    ++	     +B
    ++	      13
     +	EOF
     +	test_cmp expected actual
     +'
-:  ---------- > 3:  edfef733c7 range-diff: make diff option behavior (e.g. --stat) consistent

Ævar Arnfjörð Bjarmason (3):
  range-diff doc: add a section about output stability
  range-diff: fix regression in passing along diff options
  range-diff: make diff option behavior (e.g. --stat) consistent

 Documentation/git-range-diff.txt | 17 +++++++++++++++++
 range-diff.c                     |  3 ++-
 t/t3206-range-diff.sh            | 31 +++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 1 deletion(-)

Comments

Johannes Schindelin Nov. 9, 2018, 4:32 p.m. UTC | #1

Hi Ævar,

On Fri, 9 Nov 2018, Ævar Arnfjörð Bjarmason wrote:

> Addresses feedback on v3, especially Eric's suggestion to split out
> the behavior change (which I was not aware of) into a 3/3.

For the record, I am fine with this iteration, too.

Ciao,
Dscho