Re: [PATCH/RFC 1/2] gitweb: change format_diff_line() to remove leading SP from $diff_class

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 17, 2011, 23:20 UTC
Message-ID: <7vehybjjdd.fsf@alter.siamese.dyndns.org>
In-Reply-To: <201110180007.31008.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

Show 10 quoted lines

> O.K.
>
>>                 [" {$num_sign}", ""],
>
> O.K.
>
>>                 ["[+ ]{$num_sign}", "add"],
>>                 ["[- ]{$num_sign}", "rem"],
>
> It would be slightly different to what current code does.

> Current code for combined diff uses "add" if there is at least one '+',
> "rem" if there are no '+' and at least one '-', and context otherwise.

The only possible difference would be for a line with all blank, but because there is an additional explicit rule for context, the behaviour is the same. In a combined diff, you will never see + and - together on the same line.

> I wonder if with sufficiently evil merge we can have a line that is
> added (changed) in some children, and removed in other, i.e. pluses
> and minuses combined.

The logic in combine-diff.c::dump_sline() was written in such a way to avoid such a confusing output.

Show 5 quoted lines

> Nb. we can put regexp here, not only stringification of regexp.
> i.e.
>
>                   [qr/[+ ]{$num_sign}/, "add"],
>                   [qr/[- ]{$num_sign}/, "rem"],

That would be a good change.

Show 6 quoted lines

>> Also don't we want to use "context" or something for the css class for the
>> context lines, instead of assuming that we won't want to paint it in any
>> special color?
>
> Right.  We use "diff" class without anything else for context, but probably
> it would be better to state this explicitly.