On Wed, May 24, 2017 at 7:26 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> Introduce a new option 'use_buffer' in the struct diff_options which
>> controls whether all output is buffered up until all output is available.
>> ...
>> Unconditionally enable output via buffer in this patch as it yields
>> a great opportunity for testing, i.e. all the diff tests from the
>> test suite pass without having reordering issues (i.e. only parts
>> of the output got buffered, and we forgot to buffer other parts).
>> The test suite passes, which gives confidence that we converted all
>> functions to use emit_line for output.
>>
>> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
>> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
>
> Oh, did I?
Yes, this is essentially the v4 with small indentation fixes, so I
assumed your signoff is still valid.
Which leads to explaining my workflow (which I think we
discussed that half a year ago with Dscho in a longer thread).
As soon as you apply a series I take that series and work off
that series, because you explained you would do smaller
fixups occasionally.
Patches that change drastically, I strip off your signoff and
pretend like I created them from scratch, but for those which
I barely touch (if at all), I do not remove your signoff, as
I'd be just passing them along again.
Maybe I have to rethink that strategy.
>
>> ---
>> diff.c | 155 ++++++++++++++++++++++++++++++++++++++++++++++++++---------------
>> diff.h | 41 +++++++++++++++++
>> 2 files changed, 161 insertions(+), 35 deletions(-)
>>
>> diff --git a/diff.c b/diff.c
>> index 514c5facd7..8e06206881 100644
>> --- a/diff.c
>> +++ b/diff.c
>> ...
>> @@ -2579,6 +2628,13 @@ static void builtin_diff(const char *name_a,
>> xecfg.ctxlen = strtoul(v, NULL, 10);
>> if (o->word_diff)
>> init_diff_words_data(&ecbdata, o, one, two);
>> + if (o->use_buffer) {
>> + struct diff_line e = diff_line_INIT;
>
> This ...
>
>> + e.state = DIFF_LINE_RELOAD_WS_RULE;
>> ...
>> +#define diff_line_INIT {NULL, NULL, NULL, 0, 0, 0}
>
> ... and this should be in all caps. We do not say
>
> struct strbuf buf = strbuf_INIT;
>
> and we should do the same for this new thing.
Yes. Totally agree. That is fallout from a mechanical
replace all s/buffered_patch_line/diff_line/ and the case
sensitivity got lost.
Will fix again.
Stefan