On Thu, Jun 22, 2017 at 4:30 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> On Wed, Jun 21, 2017 at 1:05 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>>
>>> static void emit_add_line(const char *reset,
>>> struct emit_callback *ecbdata,
>>> const char *line, int len)
>>> {
>>> - emit_line_checked(reset, ecbdata, line, len,
>>> - DIFF_FILE_NEW, WSEH_NEW, '+');
>>> + unsigned flags = WSEH_NEW | ecbdata->ws_rule;
>>> + if (new_blank_line_at_eof(ecbdata, line, len))
>>> + flags |= DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF;
>>> +
>>> + emit_diff_symbol(ecbdata->opt, DIFF_SYMBOL_PLUS, line, len, flags);
>>> }
>>
>> This is a bit unsatisfactory that the original code didn't even have
>> to make a call to new_blank_line_at_eof() at all when we know we are
>> not checking for / coloring whitespace errors, but the function is
>> called unconditionally in the new code.
>
> We could check if we do colored output here, I think'll just do that for now,
> but on the other hand this becomes a maintenance nightmare when we
> rely on these flags in the future e.g. for "machine parse-able coloring"
> and would markup according to the flags strictly. I guess we can fix it then.
Actually that function already has some quick return:
static int new_blank_line_at_eof(struct emit_callback *ecbdata, const
char *line, int len)
{
if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
ecbdata->blank_at_eof_in_preimage &&
ecbdata->blank_at_eof_in_postimage &&
ecbdata->blank_at_eof_in_preimage <= ecbdata->lno_in_preimage &&
ecbdata->blank_at_eof_in_postimage <= ecbdata->lno_in_postimage))
return 0;
return ws_blank_line(line, len, ecbdata->ws_rule);
}
so maybe we could 'inline' it, as there is no reasonable faster check
than what we
have in there.
I'll keep it 'a bit unsatisfactory' here and if it actually is a
performance issue, we
can fix it.
