On Thu, Nov 22, 2018 at 10:58 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > There's a regression related to this that I wanted to send a headsup > for, but don't have time to fix today. Now range-diff in format-patch > includes --stat output. See e.g. my > https://public-inbox.org/git/20181122132823.9883-1-avarab@xxxxxxxxx/ Umf. Unfortunate fallout from [1]. > diff --git a/builtin/log.c b/builtin/log.c > @@ -1094,9 +1094,12 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, > if (rev->rdiff1) { > + const int oldfmt = rev->diffopt.output_format; > fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title); > + rev->diffopt.output_format &= ~(DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY); > show_range_diff(rev->rdiff1, rev->rdiff2, > rev->creation_factor, 1, &rev->diffopt); > + rev->diffopt.output_format = oldfmt; > } > } A few questions/observations: Does this same "fix" need to be applied to the --interdiff case just above this --range-diff block? Does the same "fix" need to be applied to the --interdiff and --range-diff cases in log-tree.c:show_log()? Aside from fixing the broken --no-patches option[2], a goal of the series was to some day make --stat do something useful. Doesn't this "fix" go against that goal? The way this change needs to be spread around at various locations is making it feel like a BandAid "fix" rather than a proper solution. It seems like it should be fixed at a different level, though I'm not sure yet if that level is higher (where the options get set) or lower (where they actually affect the operation). Until we figure out the answers to these questions, I wonder if a more sensible short-term solution would be to back out [1] and just keep [2], which fixed the --no-patches regression. > diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh > @@ -248,8 +248,10 @@ test_expect_success 'dual-coloring' ' > for prev in topic master..topic > do > test_expect_success "format-patch --range-diff=$prev" ' > + cat actual && Debugging session gunk? > git format-patch --stdout --cover-letter --range-diff=$prev \ > master..unmodified >actual && > + ! grep "a => b" actual && > grep "= 1: .* s/5/A" actual && [1]: https://public-inbox.org/git/20181113185558.23438-4-avarab@xxxxxxxxx/ [2]: https://public-inbox.org/git/20181113185558.23438-3-avarab@xxxxxxxxx/
