{"thread":{"id":"29750","subject":"[PATCHv3 0/4] gitweb: Match highlighting in project search","startedAt":"2012-02-27T01:55:18Z","lastAt":"2012-02-27T01:55:22Z","messageCount":5,"participants":["Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"185487","messageId":"1330307722-25351-1-git-send-email-jnareb@gmail.com","threadId":"29750","inReplyTo":null,"subject":"[PATCHv3 0/4] gitweb: Match highlighting in project search","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-27T01:55:18Z","receivedAt":"2012-02-27T01:55:18Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"These patches are separated from second part of previous version of\nthis series\n\n  \"[PATCHv2 0/8] gitweb: Faster and improved project search\"\n  http://thread.gmane.org/gmane.comp.version-control.git/190852\n\nIt is meant to replace 'jn/gitweb-hilite-regions' in pu.\n\nThis series adds highlighting of matching string to project search.\nThe per-project 'commit' and 'grep' search already implement match\nhighlighting.  Additional goal is to provide beginnings of match\nhighlighting framework, to be used thorough gitweb, reducing code\nduplication and unifying handling of match highlighting, improving\nit.\n\nThe implementation of match highlighting in this series can highlight\ncorrectly multiple matches in single line, as opposed to hand-rolled\ncode for highlighting in 'grep' and 'commit' search types for 'search'\naction.\n\n\nPrevious version of this series had as a last patch proof-of-concept\nof using match highlighting infrastructure for per-project search,\nnamely 'grep' search:\n\n  gitweb: Use esc_html_match_hl() in 'grep' search\n  http://thread.gmane.org/gmane.comp.version-control.git/190852/focus=190860\n\nThis version does not include it, among others because of question\non how to implement -nbsp=>1 version of match highlighting (see first\npatch in series for comments).  It would be as a separate patch series.\n\n\nJakub Narebski (4):\n  gitweb: Introduce esc_html_match_hl and esc_html_hl_regions\n  gitweb: Highlight matched part of project name when searching\n    projects\n  gitweb: Highlight matched part of project description when searching\n    projects\n  gitweb: Highlight matched part of shortened project description\n\n gitweb/gitweb.perl |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++--\n 1 files changed, 92 insertions(+), 3 deletions(-)\n\n-- \n1.7.9\n"},{"id":"185490","messageId":"1330307722-25351-2-git-send-email-jnareb@gmail.com","threadId":"29750","inReplyTo":"1330307722-25351-1-git-send-email-jnareb@gmail.com","subject":"[PATCHv3 1/4] gitweb: Introduce esc_html_match_hl and esc_html_hl_regions","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-27T01:55:19Z","receivedAt":"2012-02-27T01:55:19Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"The esc_html_match_hl() subroutine added in this commit will be used\nto highlight *all* matches of given regexp, using 'match' class.\nUltimately it is to be used in all match highlighting, starting\nwith project search, which does not have it yet.\n\nIt uses the esc_html_hl_regions() subroutine, which is meant to\nhighlight in a given string a list of regions (given as a list of\n[ beg, end ] pairs of positions in string), using HTML <span> element\nwith given class.  It could probably be used in other places that\ndo highlighting of part of ready line, like highlighting of changes\nin a diff (diff refinement highlighting).\n\n\nImplementation and enhancement notes:\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n* Currently esc_html_hl_regions() subroutine doesn't accept any\n  parameters, like esc_html() does.  We might want for example to\n  pass  nbsp=>1  to it.\n\n  It can easily be done with the following code:\n\n    my %opts = grep { ref($_) ne \"ARRAY\" } @sel;\n    @sel     = grep { ref($_) eq \"ARRAY\" } @sel;\n\n  This allow adding parameters after or before regions, e.g.:\n\n    esc_html_hl_regions(\"foo bar\", \"mark\", [ 0, 3 ], -nbsp => 1);\n\n* esc_html_hl_regions() escapes like esc_html(); if we wanted to\n  highlight with esc_path(), we could pass subroutine reference\n  to now named esc_gen_hl_regions().\n\n    esc_html_hl_regions(\"foo bar\", \"mark\", \\&esc_path, [ 0, 3 ]);\n\n  Note that this way we can handle -nbsp=>1 case automatically,\n  e.g.\n\n    esc_html_hl_regions(\"foo bar\", \"mark\",\n                        sub { esc_html(@_, -nbsp=>1) },\n                        [ 0, 3 ]);\n\n* Alternate solution for highlighting region of a string would be to\n  use the idea that strings are to be HTML-escaped, and references to\n  scalars are HTML (like in the idea for generic committags).\n\n  This would require modifying gitweb code or esc_html to get list of\n  fragments, e.g.:\n\n    esc_html(\\'<span class=\"mark\">', 'foo', \\'</span>', ' bar',\n             { -nbsp => 1 });\n\n  or\n\n    esc_html([\\'<span class=\"mark\">', 'foo', \\'</span>', ' bar'],\n             -nbsp=>1);\n\n  esc_html_match_hl() could be then simple wrapper around \"match\n  formatter\", e.g.\n\n    esc_html([ render_match_hl($str, $regexp) ], -nbsp=>1);\n\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis commit is here mainly for the notes in the commit message,\notherwise it could have been squashed with the next commit.  Having\nit as a separate commit might help with reviewing.\n\nChanges from v2 (patch was not present in v1):\n* Patch includes esc_html_match_hl() subroutine (the interface), and\n  not only the esc_html_hl_regions() \"engine\".\n\n* Slightly expanded \"Implementation and enhancement notes\" section in\n  the commit message.\n\n* Added not about possible future use of esc_html_hl_regions()\n  in commit message\n\n gitweb/gitweb.perl |   41 +++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 41 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 4ceb1a6..b790f67 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1715,6 +1715,47 @@ sub chop_and_escape_str {\n \t}\n }\n \n+# Highlight selected fragments of string, using given CSS class,\n+# and escape HTML.  It is assumed that fragments do not overlap.\n+# Regions are passed as list of pairs (array references).\n+#\n+# Example: esc_html_hl_regions(\"foobar\", \"mark\", [ 0, 3 ]) returns\n+# '<span class=\"mark\">foo</span>bar'\n+sub esc_html_hl_regions {\n+\tmy ($str, $css_class, @sel) = @_;\n+\treturn esc_html($str) unless @sel;\n+\n+\tmy $out = '';\n+\tmy $pos = 0;\n+\n+\tfor my $s (@sel) {\n+\t\t$out .= esc_html(substr($str, $pos, $s->[0] - $pos))\n+\t\t\tif ($s->[0] - $pos > 0);\n+\t\t$out .= $cgi->span({-class => $css_class},\n+\t\t                   esc_html(substr($str, $s->[0], $s->[1] - $s->[0])));\n+\n+\t\t$pos = $s->[1];\n+\t}\n+\t$out .= esc_html(substr($str, $pos))\n+\t\tif ($pos < length($str));\n+\n+\treturn $out;\n+}\n+\n+# highlight match (if any), and escape HTML\n+sub esc_html_match_hl {\n+\tmy ($str, $regexp) = @_;\n+\treturn esc_html($str) unless defined $regexp;\n+\n+\tmy @matches;\n+\twhile ($str =~ /$regexp/g) {\n+\t\tpush @matches, [$-[0], $+[0]];\n+\t}\n+\treturn esc_html($str) unless @matches;\n+\n+\treturn esc_html_hl_regions($str, 'match', @matches);\n+}\n+\n ## ----------------------------------------------------------------------\n ## functions returning short strings\n \n-- \n1.7.9\n"},{"id":"185488","messageId":"1330307722-25351-3-git-send-email-jnareb@gmail.com","threadId":"29750","inReplyTo":"1330307722-25351-1-git-send-email-jnareb@gmail.com","subject":"[PATCHv3 2/4] gitweb: Highlight matched part of project name when searching projects","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-27T01:55:20Z","receivedAt":"2012-02-27T01:55:20Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Use esc_html_match_hl() introduced in previous commit to escape HTML\nand mark match, using span element with 'match' class.  Currently only\nthe 'path' part (i.e. the project name) is highlighted; match might be\non the project description.  Highlighting match in description is left\nfor next commit.\n\nThe code makes use of the fact that defined $search_regexp means that\nthere was search going on.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nChanges from v2:\n* Introducing esc_html_match_hl() had been put together with\n  introduction of esc_html_hl_regions() in previuos commit.\n  Commit message got updated accordingly.\n\nChanges from v1:\n* Main part of esc_html_match_hl() got split into esc_html_hl_regions(),\n  which was introduced in previous commit.\n\n gitweb/gitweb.perl |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex b790f67..829526f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5407,7 +5407,9 @@ sub git_project_list_rows {\n \t\t\tprint \"</td>\\n\";\n \t\t}\n \t\tprint \"<td>\" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>\"summary\"),\n-\t\t                        -class => \"list\"}, esc_html($pr->{'path'})) . \"</td>\\n\" .\n+\t\t                        -class => \"list\"},\n+\t\t                       esc_html_match_hl($pr->{'path'}, $search_regexp)) .\n+\t\t      \"</td>\\n\" .\n \t\t      \"<td>\" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>\"summary\"),\n \t\t                        -class => \"list\", -title => $pr->{'descr_long'}},\n \t\t                        esc_html($pr->{'descr'})) . \"</td>\\n\" .\n-- \n1.7.9\n"},{"id":"185489","messageId":"1330307722-25351-4-git-send-email-jnareb@gmail.com","threadId":"29750","inReplyTo":"1330307722-25351-1-git-send-email-jnareb@gmail.com","subject":"[PATCHv3 3/4] gitweb: Highlight matched part of project description when searching projects","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-27T01:55:21Z","receivedAt":"2012-02-27T01:55:21Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Use esc_html_match_hl() from earlier commit to mark match in the\n_whole_ description when searching projects.\n\nCurrently, with this commit, when searching projects there is always\nshown full description of a project, and not a shortened one (like for\nordinary projects list view), even if the match is on project name and\nnot project description.  Because we always show full description of a\nproject, and not possibly shortened name, there is no need for having\nfull description on mouseover via title attribute.\n\nShowing full description when there is match on it is useful to avoid\nsituation where match is in shortened, invisible part.  On the other\nhand that makes project search different than projects list view; also\nthere can be problems with overly-long project descriptions.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nChanges from v2 and v1:\n* Code updated to not use -title attribute in project search; we don't\n  need to show full project description on mouseover, as we always\n  show description in full, not shortened.\n\n* Commit message updated to changes in previous commits.\n\n* Removed notes (in commit message) about trouble with highlighting\n  match of full description using shortened one; we add it in next\n  commit, and it is not very complicated.\n\n gitweb/gitweb.perl |    8 ++++++--\n 1 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 829526f..03a8dd9 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5411,8 +5411,12 @@ sub git_project_list_rows {\n \t\t                       esc_html_match_hl($pr->{'path'}, $search_regexp)) .\n \t\t      \"</td>\\n\" .\n \t\t      \"<td>\" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>\"summary\"),\n-\t\t                        -class => \"list\", -title => $pr->{'descr_long'}},\n-\t\t                        esc_html($pr->{'descr'})) . \"</td>\\n\" .\n+\t\t                        -class => \"list\",\n+\t\t                        $search_regexp ? () : -title => $pr->{'descr_long'}},\n+\t\t                        $search_regexp\n+\t\t                        ? esc_html_match_hl($pr->{'descr_long'}, $search_regexp)\n+\t\t                        : esc_html($pr->{'descr'})) .\n+\t\t      \"</td>\\n\" .\n \t\t      \"<td><i>\" . chop_and_escape_str($pr->{'owner'}, 15) . \"</i></td>\\n\";\n \t\tprint \"<td class=\\\"\". age_class($pr->{'age'}) . \"\\\">\" .\n \t\t      (defined $pr->{'age_string'} ? $pr->{'age_string'} : \"No commits\") . \"</td>\\n\" .\n-- \n1.7.9\n"},{"id":"185491","messageId":"1330307722-25351-5-git-send-email-jnareb@gmail.com","threadId":"29750","inReplyTo":"1330307722-25351-1-git-send-email-jnareb@gmail.com","subject":"[PATCHv4 4/4] gitweb: Highlight matched part of shortened project description","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-02-27T01:55:22Z","receivedAt":"2012-02-27T01:55:22Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Previous commit make gitweb use esc_html_match_hl() to mark match in\nthe _whole_ description of a project when searching projects.\n\nThis commit makes gitweb highlight match in _shortened_ description,\nbased on match in whole description, using esc_html_match_hl_chopped()\nsubroutine.\n\nIf match is in removed (chopped) part, even partially, then trailing\n\"... \" is highlighted.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nIt was marked as RFC, because I am not sure if it is right way to\nhighlight match in shortened string, or if we better use full string,\nor full string if match is in chopped part.\n\nChanges from v3:\n* The relation between esc_html_match_hl() and esc_html_match_hl_chopped()\n  got changed; instead of esc_html_match_hl() being wrapper and\n  esc_html_match_hl_chopped() doing all the work, common part is\n  refactored into matchpos_list(), and both esc_html_match_hl* are\n  kept separate.  It results in a bit of code duplication, but\n  hopefully it also makes code more clear (it is e.g. less nested).\n\n* Brings back -title that was removed in previous commit, to show full\n  description on mouseover (partial revert of previous commit).\n\nChanges from v2:\n* Harden esc_html_match_hl_chopped() against calling with both\n  $chopped and $regexp undefined (even though it wouldn't happen\n  with current code).\n\n* esc_html_match_hl_chopped() uses now esc_html_hl_regions(),\n  like esc_html_match_hl() used to do.\n\nChanges from v1:\n* Instead of esc_html_match_hl_chopped() duplicating much od code in\n  esc_html_match_hl(), make esc_html_match_hl() call the *_chopped()\n  one with $chopped set to undef.\n\n  Now managing highlighting of $chopped part is just a matter of\n  adjusting and filtering @matches to apply to $chopped rather than\n  original $str where match was performed.\n\n  As a side issue when match span past chop point current code uses\n  one selection <span class+match\">foo... </span> and not two\n  <span class=\"match\">foo</span><span class=\"match\">... </span>\n\n gitweb/gitweb.perl |   52 +++++++++++++++++++++++++++++++++++++++++++++++-----\n 1 files changed, 47 insertions(+), 5 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 03a8dd9..1fc5361 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1742,20 +1742,61 @@ sub esc_html_hl_regions {\n \treturn $out;\n }\n \n-# highlight match (if any), and escape HTML\n-sub esc_html_match_hl {\n+# return positions of beginning and end of each match\n+sub matchpos_list {\n \tmy ($str, $regexp) = @_;\n-\treturn esc_html($str) unless defined $regexp;\n+\treturn unless (defined $str && defined $regexp);\n \n \tmy @matches;\n \twhile ($str =~ /$regexp/g) {\n \t\tpush @matches, [$-[0], $+[0]];\n \t}\n+\treturn @matches;\n+}\n+\n+# highlight match (if any), and escape HTML\n+sub esc_html_match_hl {\n+\tmy ($str, $regexp) = @_;\n+\treturn esc_html($str) unless defined $regexp;\n+\n+\tmy @matches = matchpos_list($str, $regexp);\n \treturn esc_html($str) unless @matches;\n \n \treturn esc_html_hl_regions($str, 'match', @matches);\n }\n \n+\n+# highlight match (if any) of shortened string, and escape HTML\n+sub esc_html_match_hl_chopped {\n+\tmy ($str, $chopped, $regexp) = @_;\n+\treturn esc_html_match_hl($str, $regexp) unless defined $chopped;\n+\n+\tmy @matches = matchpos_list($str, $regexp);\n+\treturn esc_html($chopped) unless @matches;\n+\n+\t# filter matches so that we mark chopped string\n+\tmy $tail = \"... \"; # see chop_str\n+\tunless ($chopped =~ s/\\Q$tail\\E$//) {\n+\t\t$tail = '';\n+\t}\n+\tmy $chop_len = length($chopped);\n+\tmy $tail_len = length($tail);\n+\tmy @filtered;\n+\n+\tfor my $m (@matches) {\n+\t\tif ($m->[0] > $chop_len) {\n+\t\t\tpush @filtered, [ $chop_len, $chop_len + $tail_len ] if ($tail_len > 0);\n+\t\t\tlast;\n+\t\t} elsif ($m->[1] > $chop_len) {\n+\t\t\tpush @filtered, [ $m->[0], $chop_len + $tail_len ];\n+\t\t\tlast;\n+\t\t}\n+\t\tpush @filtered, $m;\n+\t}\n+\n+\treturn esc_html_hl_regions($chopped . $tail, 'match', @filtered);\n+}\n+\n ## ----------------------------------------------------------------------\n ## functions returning short strings\n \n@@ -5412,9 +5453,10 @@ sub git_project_list_rows {\n \t\t      \"</td>\\n\" .\n \t\t      \"<td>\" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>\"summary\"),\n \t\t                        -class => \"list\",\n-\t\t                        $search_regexp ? () : -title => $pr->{'descr_long'}},\n+\t\t                        -title => $pr->{'descr_long'}},\n \t\t                        $search_regexp\n-\t\t                        ? esc_html_match_hl($pr->{'descr_long'}, $search_regexp)\n+\t\t                        ? esc_html_match_hl_chopped($pr->{'descr_long'},\n+\t\t                                                    $pr->{'descr'}, $search_regexp)\n \t\t                        : esc_html($pr->{'descr'})) .\n \t\t      \"</td>\\n\" .\n \t\t      \"<td><i>\" . chop_and_escape_str($pr->{'owner'}, 15) . \"</i></td>\\n\";\n-- \n1.7.9\n"}]}