threads / discuss / 29750

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

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

## tl;dr

5 messages between Feb 27, 2012 and Feb 27, 2012.

replies: 4people: 1as markdown or json

Jakub Narebski· Feb 27, 2012, 01:55 UTC · lore

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
Jakub Narebski· Feb 27, 2012, 01:55 UTC · re: Jakub Narebski · lore

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

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
Jakub Narebski· Feb 27, 2012, 01:55 UTC · re: Jakub Narebski · lore

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

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
Jakub Narebski· Feb 27, 2012, 01:55 UTC · re: Jakub Narebski · lore

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

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
Jakub Narebski· Feb 27, 2012, 01:55 UTC · re: Jakub Narebski · lore

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

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

← back to recent threads