Re: [PATCH 6/8] gitweb: Highlight interesting parts of diff

From: Jakub Narebski <jnareb@gmail.com>
Date: Feb 12, 2012, 10:42 UTC
Message-ID: <m3lio8s57v.fsf@localhost.localdomain>
In-Reply-To: <m3y5s9rl3g.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> Michał Kiedrowicz <michal.kiedrowicz@gmail.com> writes:

Show 56 quoted lines

> > +# Highlight characters from $prefix to $suffix and escape HTML.
> > +# $str is a reference to the array of characters.
> > +sub esc_html_mark_range {
> > +	my ($str, $prefix, $suffix) = @_;
> > +
> > +	# Don't generate empty <span> element.
> > +	if ($prefix == $suffix + 1) {
> > +		return esc_html(join('', @$str), -nbsp=>1);
> > +	}
> > +
> > +	my $before = join('', @{$str}[0..($prefix - 1)]);
> > +	my $marked = join('', @{$str}[$prefix..$suffix]);
> > +	my $after = join('', @{$str}[($suffix + 1)..$#{$str}]);
> 
> Eeeeeek!  First you split into letters, in caller at that, then join?
> Why not pass striung ($str suggests string not array of characters),
> and use substr instead?
> 
> [Please disregard this and the next paragraph at first reading]
> 
> > +
> > +	return esc_html($before, -nbsp=>1) .
> > +		$cgi->span({-class=>'marked'}, esc_html($marked, -nbsp=>1)) .
> > +		esc_html($after,-nbsp=>1);
> > +}
> 
> Anyway I have send to git mailing list a patch series, which in one of
> patches adds esc_html_match_hl($str, $regexp) to highlight matches in
> a string.  Your esc_html_mark_range(), after a generalization, could
> be used as underlying "engine".
> 
> Something like this, perhaps (untested):
> 
>    # Highlight selected fragments of string, using given CSS class,
>    # and escape HTML.  It is assumed that fragments do not overlap.
>    # Regions are passed as list of pairs (array references).
>    sub esc_html_hl {
>         my ($str, $css_class, @sel) = @_;
>         return esc_html($str) unless @sel;
>    
>         my $out = '';
>         my $pos = 0;
>    
>         for my $s (@sel) {
>                 $out .= esc_html(substr($str, $pos, $s->[0] - $pos))
>                         if ($s->[0] - $pos > 0);
>                 $out .= $cgi->span({-class => $css_class},
>                                    esc_html(substr($str, $s->[0], $s->[1] - $s->[0])));
> 
>                 $pos = $m->[1];
>         }
>         $out .= esc_html(substr($str, $pos))
>                 if ($pos < length($str));
>    
>         return $out;
>    }

Actually we can accomodate both operating on string and operating on array of characters in a single subroutine. Though it can be left for later commit, anyway...

     # Highlight selected fragments of string, using given CSS class,
     # and escape HTML.  It is assumed that fragments do not overlap.
     # Regions are passed as list of pairs (array references).
     sub esc_html_hl {
          my ($sth, $css_class, @sel) = @_;

          if (!@sel) {
                if (ref($sth) eq "ARRAY") {
                        return esc_html(join('', @$sth), -nbsp=>1);
                } else {
                        return esc_html($sth, -nbsp=>1);
          }

          if (ref($sth) eq "ARRAY") {
                return esc_html_hl_gen($sth,
                        sub { 
                                my ($arr, $from, $to) = @_;
                                return join('', @{$arr}[$from..$to]);
                        },
                        scalar @{$arr}, $css_class, @sel);
           } else {
                return esc_html_hl_gen($sth,
                        sub {
                                my ($str, $from, $to) = @_;
                                if ($to < 0) { $to += lenght($str); };
                                return substr($str, $from, $to - $from);
                        },
                        length($sth), $css_class, @sel);
           }
     }

     # Highlight selected fragments of string or array of characters
     # with given length, using provided $extr subroutine to extract
     # fragment (substring)
     sub esc_html_hl_gen {
          my ($sth, $extr, $len, $css_class, @sel) = @_;
     
          my $out = '';
          my $pos = 0;
     
          for my $s (@sel) {
                $out .= esc_html($extr->($str, $pos, $s->[0]))
                        if ($s->[0] - $pos > 0);
                $out .= $cgi->span({-class => $css_class},
                                   esc_html($extr->($str, $s->[0], $s->[1])));
  
                $pos = $s->[1];
          }
          $out .= esc_html($extr->($str, $pos, $len))
                  if ($pos < $len);
     
          return $out;
     }

Or maybe I have read "Higher-Order Perl" one time too many ;-))))

-- 
Jakub Narębski