[PATCHv3 0/4] gitweb: Match highlighting in project search

[PATCHv3 0/4] gitweb: Match highlighting in project search

[Jakub Narebski]

These patches are separated from second part of previous version of
this series

  "[PATCHv2 0/8] gitweb: Faster and improved project search"
  http://thread.gmane.org/gmane.comp.version-control.git/190852

It is meant to replace 'jn/gitweb-hilite-regions' in pu.

This series adds highlighting of matching string to project search.
The per-project 'commit' and 'grep' search already implement match
highlighting.  Additional goal is to provide beginnings of match
highlighting framework, to be used thorough gitweb, reducing code
duplication and unifying handling of match highlighting, improving
it.

The implementation of match highlighting in this series can highlight
correctly multiple matches in single line, as opposed to hand-rolled
code for highlighting in 'grep' and 'commit' search types for 'search'
action.


Previous version of this series had as a last patch proof-of-concept
of using match highlighting infrastructure for per-project search,
namely 'grep' search:

  gitweb: Use esc_html_match_hl() in 'grep' search
  http://thread.gmane.org/gmane.comp.version-control.git/190852/focus=190860

This version does not include it, among others because of question
on how to implement -nbsp=>1 version of match highlighting (see first
patch in series for comments).  It would be as a separate patch series.


Jakub Narebski (4):
  gitweb: Introduce esc_html_match_hl and esc_html_hl_regions
  gitweb: Highlight matched part of project name when searching
    projects
  gitweb: Highlight matched part of project description when searching
    projects
  gitweb: Highlight matched part of shortened project description

 gitweb/gitweb.perl |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 92 insertions(+), 3 deletions(-)

-- 
1.7.9

[PATCHv3 1/4] gitweb: Introduce esc_html_match_hl and esc_html_hl_regions

[Jakub Narebski]

The esc_html_match_hl() subroutine added in this commit will be used
to highlight *all* matches of given regexp, using 'match' class.
Ultimately it is to be used in all match highlighting, starting
with project search, which does not have it yet.

It uses the esc_html_hl_regions() subroutine, which is meant to
highlight in a given string a list of regions (given as a list of
[ beg, end ] pairs of positions in string), using HTML <span> element
with given class.  It could probably be used in other places that
do highlighting of part of ready line, like highlighting of changes
in a diff (diff refinement highlighting).


Implementation and enhancement notes:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
* Currently esc_html_hl_regions() subroutine doesn't accept any
  parameters, like esc_html() does.  We might want for example to
  pass  nbsp=>1  to it.

  It can easily be done with the following code:

    my %opts = grep { ref($_) ne "ARRAY" } @sel;
    @sel     = grep { ref($_) eq "ARRAY" } @sel;

  This allow adding parameters after or before regions, e.g.:

    esc_html_hl_regions("foo bar", "mark", [ 0, 3 ], -nbsp => 1);

* esc_html_hl_regions() escapes like esc_html(); if we wanted to
  highlight with esc_path(), we could pass subroutine reference
  to now named esc_gen_hl_regions().

    esc_html_hl_regions("foo bar", "mark", \&esc_path, [ 0, 3 ]);

  Note that this way we can handle -nbsp=>1 case automatically,
  e.g.

    esc_html_hl_regions("foo bar", "mark",
                        sub { esc_html(@_, -nbsp=>1) },
                        [ 0, 3 ]);

* Alternate solution for highlighting region of a string would be to
  use the idea that strings are to be HTML-escaped, and references to
  scalars are HTML (like in the idea for generic committags).

  This would require modifying gitweb code or esc_html to get list of
  fragments, e.g.:

    esc_html(\'<span class="mark">', 'foo', \'</span>', ' bar',
             { -nbsp => 1 });

  or

    esc_html([\'<span class="mark">', 'foo', \'</span>', ' bar'],
             -nbsp=>1);

  esc_html_match_hl() could be then simple wrapper around "match
  formatter", e.g.

    esc_html([ render_match_hl($str, $regexp) ], -nbsp=>1);


Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This commit is here mainly for the notes in the commit message,
otherwise it could have been squashed with the next commit.  Having
it as a separate commit might help with reviewing.

Changes from v2 (patch was not present in v1):
* Patch includes esc_html_match_hl() subroutine (the interface), and
  not only the esc_html_hl_regions() "engine".

* Slightly expanded "Implementation and enhancement notes" section in
  the commit message.

* Added not about possible future use of esc_html_hl_regions()
  in commit message

 gitweb/gitweb.perl |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4ceb1a6..b790f67 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1715,6 +1715,47 @@ sub chop_and_escape_str {
 	}
 }
 
+# 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).
+#
+# Example: esc_html_hl_regions("foobar", "mark", [ 0, 3 ]) returns
+# '<span class="mark">foo</span>bar'
+sub esc_html_hl_regions {
+	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 = $s->[1];
+	}
+	$out .= esc_html(substr($str, $pos))
+		if ($pos < length($str));
+
+	return $out;
+}
+
+# highlight match (if any), and escape HTML
+sub esc_html_match_hl {
+	my ($str, $regexp) = @_;
+	return esc_html($str) unless defined $regexp;
+
+	my @matches;
+	while ($str =~ /$regexp/g) {
+		push @matches, [$-[0], $+[0]];
+	}
+	return esc_html($str) unless @matches;
+
+	return esc_html_hl_regions($str, 'match', @matches);
+}
+
 ## ----------------------------------------------------------------------
 ## functions returning short strings
 
-- 
1.7.9

[PATCHv3 2/4] gitweb: Highlight matched part of project name when searching projects

[Jakub Narebski]

Use esc_html_match_hl() introduced in previous commit to escape HTML
and mark match, using span element with 'match' class.  Currently only
the 'path' part (i.e. the project name) is highlighted; match might be
on the project description.  Highlighting match in description is left
for next commit.

The code makes use of the fact that defined $search_regexp means that
there was search going on.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Changes from v2:
* Introducing esc_html_match_hl() had been put together with
  introduction of esc_html_hl_regions() in previuos commit.
  Commit message got updated accordingly.

Changes from v1:
* Main part of esc_html_match_hl() got split into esc_html_hl_regions(),
  which was introduced in previous commit.

 gitweb/gitweb.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b790f67..829526f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5407,7 +5407,9 @@ sub git_project_list_rows {
 			print "</td>\n";
 		}
 		print "<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
-		                        -class => "list"}, esc_html($pr->{'path'})) . "</td>\n" .
+		                        -class => "list"},
+		                       esc_html_match_hl($pr->{'path'}, $search_regexp)) .
+		      "</td>\n" .
 		      "<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
 		                        -class => "list", -title => $pr->{'descr_long'}},
 		                        esc_html($pr->{'descr'})) . "</td>\n" .
-- 
1.7.9

[PATCHv3 3/4] gitweb: Highlight matched part of project description when searching projects

[Jakub Narebski]

Use esc_html_match_hl() from earlier commit to mark match in the
_whole_ description when searching projects.

Currently, with this commit, when searching projects there is always
shown full description of a project, and not a shortened one (like for
ordinary projects list view), even if the match is on project name and
not project description.  Because we always show full description of a
project, and not possibly shortened name, there is no need for having
full description on mouseover via title attribute.

Showing full description when there is match on it is useful to avoid
situation where match is in shortened, invisible part.  On the other
hand that makes project search different than projects list view; also
there can be problems with overly-long project descriptions.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Changes from v2 and v1:
* Code updated to not use -title attribute in project search; we don't
  need to show full project description on mouseover, as we always
  show description in full, not shortened.

* Commit message updated to changes in previous commits.

* Removed notes (in commit message) about trouble with highlighting
  match of full description using shortened one; we add it in next
  commit, and it is not very complicated.

 gitweb/gitweb.perl |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 829526f..03a8dd9 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5411,8 +5411,12 @@ sub git_project_list_rows {
 		                       esc_html_match_hl($pr->{'path'}, $search_regexp)) .
 		      "</td>\n" .
 		      "<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
-		                        -class => "list", -title => $pr->{'descr_long'}},
-		                        esc_html($pr->{'descr'})) . "</td>\n" .
+		                        -class => "list",
+		                        $search_regexp ? () : -title => $pr->{'descr_long'}},
+		                        $search_regexp
+		                        ? esc_html_match_hl($pr->{'descr_long'}, $search_regexp)
+		                        : esc_html($pr->{'descr'})) .
+		      "</td>\n" .
 		      "<td><i>" . chop_and_escape_str($pr->{'owner'}, 15) . "</i></td>\n";
 		print "<td class=\"". age_class($pr->{'age'}) . "\">" .
 		      (defined $pr->{'age_string'} ? $pr->{'age_string'} : "No commits") . "</td>\n" .
-- 
1.7.9

[PATCHv4 4/4] gitweb: Highlight matched part of shortened project description

[Jakub Narebski]

Previous commit make gitweb use esc_html_match_hl() to mark match in
the _whole_ description of a project when searching projects.

This commit makes gitweb highlight match in _shortened_ description,
based on match in whole description, using esc_html_match_hl_chopped()
subroutine.

If match is in removed (chopped) part, even partially, then trailing
"... " is highlighted.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
It was marked as RFC, because I am not sure if it is right way to
highlight match in shortened string, or if we better use full string,
or full string if match is in chopped part.

Changes from v3:
* The relation between esc_html_match_hl() and esc_html_match_hl_chopped()
  got changed; instead of esc_html_match_hl() being wrapper and
  esc_html_match_hl_chopped() doing all the work, common part is
  refactored into matchpos_list(), and both esc_html_match_hl* are
  kept separate.  It results in a bit of code duplication, but
  hopefully it also makes code more clear (it is e.g. less nested).

* Brings back -title that was removed in previous commit, to show full
  description on mouseover (partial revert of previous commit).

Changes from v2:
* Harden esc_html_match_hl_chopped() against calling with both
  $chopped and $regexp undefined (even though it wouldn't happen
  with current code).

* esc_html_match_hl_chopped() uses now esc_html_hl_regions(),
  like esc_html_match_hl() used to do.

Changes from v1:
* Instead of esc_html_match_hl_chopped() duplicating much od code in
  esc_html_match_hl(), make esc_html_match_hl() call the *_chopped()
  one with $chopped set to undef.

  Now managing highlighting of $chopped part is just a matter of
  adjusting and filtering @matches to apply to $chopped rather than
  original $str where match was performed.

  As a side issue when match span past chop point current code uses
  one selection <span class+match">foo... </span> and not two
  <span class="match">foo</span><span class="match">... </span>

 gitweb/gitweb.perl |   52 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 03a8dd9..1fc5361 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1742,20 +1742,61 @@ sub esc_html_hl_regions {
 	return $out;
 }
 
-# highlight match (if any), and escape HTML
-sub esc_html_match_hl {
+# return positions of beginning and end of each match
+sub matchpos_list {
 	my ($str, $regexp) = @_;
-	return esc_html($str) unless defined $regexp;
+	return unless (defined $str && defined $regexp);
 
 	my @matches;
 	while ($str =~ /$regexp/g) {
 		push @matches, [$-[0], $+[0]];
 	}
+	return @matches;
+}
+
+# highlight match (if any), and escape HTML
+sub esc_html_match_hl {
+	my ($str, $regexp) = @_;
+	return esc_html($str) unless defined $regexp;
+
+	my @matches = matchpos_list($str, $regexp);
 	return esc_html($str) unless @matches;
 
 	return esc_html_hl_regions($str, 'match', @matches);
 }
 
+
+# highlight match (if any) of shortened string, and escape HTML
+sub esc_html_match_hl_chopped {
+	my ($str, $chopped, $regexp) = @_;
+	return esc_html_match_hl($str, $regexp) unless defined $chopped;
+
+	my @matches = matchpos_list($str, $regexp);
+	return esc_html($chopped) unless @matches;
+
+	# filter matches so that we mark chopped string
+	my $tail = "... "; # see chop_str
+	unless ($chopped =~ s/\Q$tail\E$//) {
+		$tail = '';
+	}
+	my $chop_len = length($chopped);
+	my $tail_len = length($tail);
+	my @filtered;
+
+	for my $m (@matches) {
+		if ($m->[0] > $chop_len) {
+			push @filtered, [ $chop_len, $chop_len + $tail_len ] if ($tail_len > 0);
+			last;
+		} elsif ($m->[1] > $chop_len) {
+			push @filtered, [ $m->[0], $chop_len + $tail_len ];
+			last;
+		}
+		push @filtered, $m;
+	}
+
+	return esc_html_hl_regions($chopped . $tail, 'match', @filtered);
+}
+
 ## ----------------------------------------------------------------------
 ## functions returning short strings
 
@@ -5412,9 +5453,10 @@ sub git_project_list_rows {
 		      "</td>\n" .
 		      "<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
 		                        -class => "list",
-		                        $search_regexp ? () : -title => $pr->{'descr_long'}},
+		                        -title => $pr->{'descr_long'}},
 		                        $search_regexp
-		                        ? esc_html_match_hl($pr->{'descr_long'}, $search_regexp)
+		                        ? esc_html_match_hl_chopped($pr->{'descr_long'},
+		                                                    $pr->{'descr'}, $search_regexp)
 		                        : esc_html($pr->{'descr'})) .
 		      "</td>\n" .
 		      "<td><i>" . chop_and_escape_str($pr->{'owner'}, 15) . "</i></td>\n";
-- 
1.7.9