Re: [PATCHv5 16/17] diff: buffer all output if asked to

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]