On Wed, Jun 21, 2017 at 12:36 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
>> ---
>> diff.c | 22 +++++++++++++++++++---
>> 1 file changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/diff.c b/diff.c
>> index 2f9722b382..89466018e5 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -559,6 +559,24 @@ static void emit_line(struct diff_options *o, const char *set, const char *reset
>> emit_line_0(o, set, reset, line[0], line+1, len-1);
>> }
>>
>> +enum diff_symbol {
>> + DIFF_SYMBOL_SEPARATOR,
>
> Drop the last comma from enum?
I looked through out code base and for enums this is
actually strictly enforced, so I guess I have to play
by the rules here as I do not want to be the first
to deviate from an upheld standard.
This will be painful though as the next ~20 patches
add more symbols mostly at the end, maybe I need
to restructure that such that the last symbol stays the same
throughout the series. Thanks for that thought.
>
>> +static void emit_diff_symbol(struct diff_options *o, enum diff_symbol s,
>> + const char *line, int len)
>> +{
>> + switch (s) {
>> + case DIFF_SYMBOL_SEPARATOR:
>> + fprintf(o->file, "%s%c",
>> + diff_line_prefix(o),
>> + o->line_termination);
>> + break;
>
> As the first patch in the "diff-symbol" subseries of this topic,
> this change must seriously be justified. Why is it so important
> that a printing of an empty line must be moved to a helper function,
> which later will gain ability to show other kind of lines?
Ah yes. This got lost in comparison to the currently queued series with
diff_lines. The justification for the change was in the buffer patch,
but now we need to have the justification here.
In the old series, I had copied the same text in all these
refactoring patches, but thought to delete them in this series. The first
refactoring patch makes sense though.
Thanks,
Stefan
