From: Junio C Hamano Date: Mon, 17 Oct 2011 23:20:14 GMT Subject: Re: [PATCH/RFC 1/2] gitweb: change format_diff_line() to remove leading SP from $diff_class Message-ID: <7vehybjjdd.fsf@alter.siamese.dyndns.org> In-Reply-To: <201110180007.31008.jnareb@gmail.com> Jakub Narebski writes: > 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. > 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. >> 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.