On Wed, Jun 21, 2017 at 1:13 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Stefan Beller <sbel...@google.com> writes:
>
>> Signed-off-by: Stefan Beller <sbel...@google.com>
>> ---
>> diff.c | 21 ++++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/diff.c b/diff.c
>> index 95f11ea86f..e56962b844 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -572,6 +572,7 @@ enum diff_symbol {
>> DIFF_SYMBOL_WORDS,
>> DIFF_SYMBOL_FILEPAIR,
>> DIFF_SYMBOL_HEADER,
>> + DIFF_SYMBOL_BINARY_FILES,
>> };
>> /*
>> * Flags for content lines:
>> @@ -689,6 +690,10 @@ static void emit_diff_symbol(struct diff_options *o,
>> enum diff_symbol s,
>> case DIFF_SYMBOL_HEADER:
>> fprintf(o->file, "%s", line);
>> break;
>> + case DIFF_SYMBOL_BINARY_FILES:
>> + fprintf(o->file, "%sBinary files %s differ\n",
>> + diff_line_prefix(o), line);
>> + break;
>
> It is curious why the "line" is defined to be "A and B" here. It
> could have been defined to be the whole string "Binary files A and B
> differ" or even with the terminating LF.
>
> And with that it may have been able to share concepts with the
> "HEADER" we see above. SYMBOL_HEADER becomes "oneline without
> prefix", and this one (updated to make the caller prepare the whole
> sentence) becomes "oneline with prefix".
I thought about saving memory in the buffered operation,
but for simplicity it may be better to make them the same.
Then they can even share the implementation.
With that thought, I'll check more different symbols to share
the same implementation.
>
>> @@ -2549,8 +2555,10 @@ static void builtin_diff(const char *name_a,
>> }
>> emit_diff_symbol(o, DIFF_SYMBOL_HEADER,
>> header.buf, header.len, 0);
>> - fprintf(o->file, "%sBinary files %s and %s differ\n",
>> - line_prefix, lbl[0], lbl[1]);
>> + strbuf_addf(&sb, "%s and %s", lbl[0], lbl[1]);
>> + emit_diff_symbol(o, DIFF_SYMBOL_BINARY_FILES,
>> + sb.buf, sb.len, 0);
>> + strbuf_release(&sb);
>> goto free_ab_and_return;
>> }
>> if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
>> @@ -2567,9 +2575,12 @@ static void builtin_diff(const char *name_a,
>> strbuf_reset(&header);
>> if (DIFF_OPT_TST(o, BINARY))
>> emit_binary_diff(o->file, &mf1, &mf2, line_prefix);
>> - else
>> - fprintf(o->file, "%sBinary files %s and %s differ\n",
>> - line_prefix, lbl[0], lbl[1]);
>> + else {
>> + strbuf_addf(&sb, "%s and %s", lbl[0], lbl[1]);
>> + emit_diff_symbol(o, DIFF_SYMBOL_BINARY_FILES,
>> + sb.buf, sb.len, 0);
>> + strbuf_release(&sb);
>> + }
>> o->found_changes = 1;
>> } else {
>> /* Crazy xdl interfaces.. */
