Do not chop HTML tags in commit search result
PATCH 1/1 Do not chop HTML tags in commit search result
Hi there,
Thanks for Git! It's a great program. I encountered an annoying bug with gitweb 1.5.4.1, when searching for commits, if the search string is too long, the generated HTML is munged leading to an ill-formed XHTML document.
Here is the patch, hope it helps:
gitweb/gitweb.perl +0 -0
@@ -3780,7 +3780,10 @@ sub git_search_grep_body { my $trail = esc_html($3) || ""; $trail = chop_str($trail, 30, 10); my $text = "$lead<span
class=\"match\">$match</span>$trail";
- print chop_str($text, 80, 5) . "<br/>\n";
+ # Do not chop $text as match can be
long, and we don't want to
+ # munge HTML tags!
+ #print chop_str($text, 80, 5) . "<br/>\n";
+ print $text . "<br/>\n";
}
}
print "</td>\n" .Signature · 1 line
1.5.4.1-dirty
Quoted 16 lines · Jean-Baptiste Quenot
... I encountered an annoying bug
with gitweb 1.5.4.1, when searching for commits, if the search string is too long, the generated HTML is munged leading to an ill-formed XHTML document.Here is the patch, hope it helps:
gitweb/gitweb.perl +0
-0@@ -3780,7 +3780,10 @@ sub git_search_grep_body { my $trail = esc_html($3) || ""; $trail = chop_str($trail, 30, 10); my $text = "$lead<span
Quoted 2 lines
class=\"match\">$match</span>$trail";
chop_str($text, 80, 5). "<br/>\n";
I think esc_html() and chop_str() are backwards here. If $3 is overlong it is cut in the middle of some markup. Even though chop_str() claims to be "HTML aware", I do not think it is. It seems to know about "&entities;" but not mark-ups.
There are quite a many instances of esc_html() first then chop_str() in that function, and I think they all deserve to be fixed.
my $comment = $co{'comment'};
foreach my $line (@$comment) {
if ($line =~ m/^(.*)($search_regexp)(.*)$/i) {
my $lead = esc_html($1) || "";
$lead = chop_str($lead, 30, 10);
my $match = esc_html($2) || "";
my $trail = esc_html($3) || "";
$trail = chop_str($trail, 30, 10);
my $text = "$lead<span class=\"match\">$match</span>$trail";
print chop_str($text, 80, 5) . "<br/>\n";
}
}I think this is trying to fit the result on a line, showing the match sandwitched by not-too-long part taken from leading and trailing context ($lead and $trail can be chomped aggressively than $match). But $lead and $trail are escaped then chomped which is already wrong.
I think the body of that if() would be better written like this:
my ($lead, $match, $trail) = ($1, $2, $3);
$match = chop_str($match, 70, $slop); # in case it is very long...
$contextlen = (80 - len($match)) / 2; # and the remainder...
if ($contextlen > 30) { $contextlen = 30 }; # but not too much
$trail = chop_str($trail, $contextlen, $slop);
$lead = chop_str($lead, $contextlen, $slop);
$lead = esc_html($lead);
$match = esc_html($match);
$trail = esc_html($trail);
print "$lead<span ...>$match</span>$trail";PATCH 1/1 gitweb: Better chopping in commit search results
From: Junio C Hamano <[email protected]>
Subject: [PATCH] gitweb: Better chopping in commit search results
When searching commit messages (commit search), if matched string is too long, the generated HTML was munged leading to an ill-formed XHTML document.
Now gitweb chop leading, trailing and matched parts, HTML escapes those parts, then composes and marks up match info. HTML output is never chopped. Limiting matched info to 80 columns (with slop) is now done by dividing remaining characters after chopping match equally to leading and trailing part, not by chopping composed and HTML marked output.
---
This is just slightly reworked Junio's patch; probably should be marked as from Junio, so I'm trying to send it as it.
Strange that StGit always sends patches (stg mail) as if repo owner was their author, regardless of path/commit author (I think; unless "stg edit" cannot change authorship).
gitweb/gitweb.perl | 24 +++++++++++++++--------- 1 files changed, 15 insertions(+), 9 deletions(-)
gitweb/gitweb.perl +15 -9
@@ -3784,18 +3784,24 @@ sub git_search_grep_body { print "<td title=\"$co{'age_string_age'}\"><i>$co{'age_string_date'}</i></td>\n" . "<td><i>" . $author . "</i></td>\n" . "<td>" . - $cgi->a({-href => href(action=>"commit", hash=>$co{'id'}), -class => "list subject"}, - chop_and_escape_str($co{'title'}, 50) . "<br/>"); + $cgi->a({-href => href(action=>"commit", hash=>$co{'id'}), + -class => "list subject"}, + chop_and_escape_str($co{'title'}, 50) . "<br/>"); my $comment = $co{'comment'}; foreach my $line (@$comment) { if ($line =~ m/^(.*)($search_regexp)(.*)$/i) { - my $lead = esc_html($1) || ""; - $lead = chop_str($lead, 30, 10); - my $match = esc_html($2) || ""; - my $trail = esc_html($3) || ""; - $trail = chop_str($trail, 30, 10); - my $text = "$lead<span class=\"match\">$match</span>$trail"; - print chop_str($text, 80, 5) . "<br/>\n"; + my ($lead, $match, $trail) = ($1, $2, $3); + $match = chop_str($match, 70, 5); # in case match is very long + my $contextlen = (80 - len($match))/2; # is left for the remainder + $contextlen = 30 if ($contextlen > 30); # but not too much + $lead = chop_str($lead, $contextlen, 10); + $trail = chop_str($trail, $contextlen, 10); + + $lead = esc_html($lead); + $match = esc_html($match); + $trail = esc_html($trail); + + print "$lead<span class=\"match\">$match</span>$trail<br />"; } } print "</td>\n" .
Re: [PATCH] gitweb: Better chopping in commit search results
Quoted 7 lines · Jakub Narebski
From: Junio C Hamano <[email protected]>
Subject: [PATCH] gitweb: Better chopping in commit search resultsWhen searching commit messages (commit search), if matched string is too long, the generated HTML was munged leading to an ill-formed XHTML document.
Quoted 6 lines
Now gitweb chop leading, trailing and matched parts, HTML escapes those parts, then composes and marks up match info. HTML output is never chopped. Limiting matched info to 80 columns (with slop) is now done by dividing remaining characters after chopping match equally to leading and trailing part, not by chopping composed and HTML marked output.
Could somebody test this with very long search string, as that was how the issue initially came up, to see (1) if it really fixes the "mark-up chopped in the middle" issue, (2) and how the actual output looks like?
Regarding the latter, I have a slight suspicion that chopping the tail of the middle part and showing very little context may not produce a very useful output.
For example, if you are looking for "very long ... and how" in the first paragraph of message (if it were all on a single line), wouldn't you want to see:
...st this with <<very long ... and how>> the actual out...
rather than:
Could som... <<very long search stri...>> the actual out...
in the result?
That is, it any chopping ever needs to happen, I suspect a more useful way to shorten the output would be to:
- divide the available space to give enough space to give context for head and tail part.
- chop head from the left, if needed, with leading ellipsis;
- chop tail from the right, if needed, with trailing ellipsis;
- chop search string from both ends, if needed, with leading and trailing ellipses.
Hmm?
Re: [PATCH] gitweb: Better chopping in commit search results
Quoted 16 lines · Junio C Hamano
Quoted 7 lines · Jakub Narebski
From: Junio C Hamano <[email protected]>
Subject: [PATCH] gitweb: Better chopping in commit search resultsWhen searching commit messages (commit search), if matched string is too long, the generated HTML was munged leading to an ill-formed XHTML document.
Quoted 6 lines
Now gitweb chop leading, trailing and matched parts, HTML escapes those parts, then composes and marks up match info. HTML output is never chopped. Limiting matched info to 80 columns (with slop) is now done by dividing remaining characters after chopping match equally to leading and trailing part, not by chopping composed and HTML marked output.
Quoted 3 lines
Could somebody test this with very long search string, as that was how the issue initially came up, to see (1) if it really fixes the "mark-up chopped in the middle" issue, (2) [...]
The bug in question was cause by the chop _after_ doing HTML markup. Now gitweb chops, then HTML escapes, and chops no more. There is no way this bug can happen now.
BTW if commit messages follows "wrap at 76 column" convention it is not easy to test this condition... :-)
But you are right that output should be improved...
Quoted 11 lines
For example, if you are looking for "very long ... and how" in the first paragraph of message (if it were all on a single line), wouldn't you want to see:
...st this with <<very long ... and how>> the actual out...
rather than:
Could som... <<very long search stri...>> the actual out...
in the result?
...but I think it is better left for another patch.
P.S. When testing this commit I have noticed that currently, probably due to some misquoting, or interaction between escapemeta and quoting, searching for messages which contain "'" (apostrophe), e.g. "don't" currently doesn't work. Will investigate...
Signature · 2 lines
Jakub Narebski Poland
Re: [PATCH] gitweb: Better chopping in commit search results
Quoted 8 lines · Jakub Narebski
Quoted 6 lines · Junio C Hamano
For example, if you are looking for "very long ... and how" in the first paragraph of message (if it were all on a single line), wouldn't you want to see:
...st this with <<very long ... and how>> the actual out...
Quoted 7 lines
Quoted 5 lines
rather than:
Could som... <<very long search stri...>> the actual out...
in the result?
...but I think it is better left for another patch.
End here is proposed improved chop_str which can do chopping at beginning, in the middle, and (as it used to do) at the end.
Some questions about the code:
* should we divide slop in two also when chopping in the middle?
* what should extra option be named, and what should be names of posible values of this option (the option deciding where to chop)
* $add_len has default value if not provided, or if 0 (!), or if ''; you have to use chop_str($str, 20, undef, -pos=>'center') trick to use it with extra options.
* can the code be improved? I'm not Perl expert.
-- >8 --
sub chop_str {
my $str = shift;
my $len = shift;
my $add_len = shift || 10;
# supported opts:
# * -pos => 'left' | 'center' | 'right', defaults to 'right'
# denotes where (which part) to chop
my %opts = @_;
# allow only $len chars, but don't cut a word if it would fit in $add_len
# if it doesn't fit, cut it if it's still longer than the dots we would add
# remove chopped character entities entirely
# when chopping in the middle, distribute $len into left and right part
if (defined $opts{'-pos'} && $opts{'-pos'} eq 'center') {
$len = int($len/2);
}
# regexps: ending and beginning with word part up to $add_len
my $endre = qr/.{0,$len}[^ \/\-_:\.@]{0,$add_len}/;
my $begre = qr/[^ \/\-_:\.@]{0,$add_len}.{0,$len}/;
if (defined $opts{'-pos'} && $opts{'-pos'} eq 'left') {
$str =~ m/^(.*?)($begre)$/;
my ($lead, $body) = ($1, $2);
if (length($lead) > 4) {
if ($lead =~ m/&[^;]*$/) {
$body =~ s/^[^;]*;//;
}
$lead = "... ";
}
return "$lead$body";
} elsif (defined $opts{'-pos'} && $opts{'-pos'} eq 'center') {
$str =~ m/^($endre)(.*)$/;
my ($left, $str) = ($1, $2);
$str =~ m/^(.*?)($begre)$/;
my ($mid, $right) = ($1, $2);
if (length($mid) > 5) {
$left =~ s/&[^;]*$//;
if ($mid =~ m/&[^;]*$/) {
$right =~ s/^[^;]*;//;
}
$mid = " ... ";
}
return "$left$mid$right";
} else {
$str =~ m/^($endre)(.*)$/;
my $body = $1;
my $tail = $2;
if (length($tail) > 4) {
$body =~ s/&[^;]*$//;
$tail = " ...";
}
return "$body$tail";
}
}-- >8 --
Example usage:
chop_str($str, 15, 5, -pos=>'center')
Signature · 2 lines
Jakub Narebski Poland
[RFC/PATCH] gitweb: Option to chop at beginning and in the middle in chop_str
Add support for '-cut' option to chop_str subroutine, to cut at the beginning (from the left side of the string), in the middle (center of the string), or at the end (from the right side of the string) which is the default:
chop_str(somestring, len, slop, -pos=>'left') -> ... string chop_str(somestring, len, slop, -pos=>'center') -> som ... ing chop_str(somestring, len, slop, -pos=>'right') -> somestr ...
Simplify passing all arguments to chop_str in chop_and_escape_str subroutine. This was needed to pass additional options to chop_str.
Make use of new feature of chop_str to better cut matched string and its context in match info for searching commit messages (commit search), as proposed by Junio C Hamano. For example, if you are looking for "very long ... and how" in the first paragraph of message (if it were all on a single line), you would now see:
...st this with <<very long ... and how>> the actual out...
instead of:
Could som... <<very long search stri...>> the actual out...
(where <<something>> denotes emphasized / colored fragment).
---
And here it is as a patch to gitweb to play with. I agree with Junio that the output is better, but I'm not sure if it is worth the complication in code; perhaps is.
gitweb/gitweb.perl | 74 +++++++++++++++++++++++++++++++++++++++++----------- 1 files changed, 58 insertions(+), 16 deletions(-)
gitweb/gitweb.perl +58 -16
@@ -848,32 +848,74 @@ sub project_in_list { ## ---------------------------------------------------------------------- ## HTML aware string manipulation +# cut (chop) string to given length, with additional slop, +# optionally from left and in the middle. sub chop_str { my $str = shift; my $len = shift; my $add_len = shift || 10; + # supported opts: + # * -cut => 'left' | 'center' | 'right', defaults to 'right' + # denotes where (which part) to chop + my %opts = @_; # allow only $len chars, but don't cut a word if it would fit in $add_len # if it doesn't fit, cut it if it's still longer than the dots we would add - $str =~ m/^(.{0,$len}[^ \/\-_:\.@]{0,$add_len})(.*)/; - my $body = $1; - my $tail = $2; - if (length($tail) > 4) { - $tail = " ..."; - $body =~ s/&[^;]*$//; # remove chopped character entities + # remove chopped character entities entirely + + # when chopping in the middle, distribute $len into left and right part + if (defined $opts{'-cut'} && $opts{'-cut'} eq 'center') { + $len = int($len/2); + } + + # regexps: ending and beginning with word part up to $add_len + my $endre = qr/.{0,$len}[^ \/\-_:\.@]{0,$add_len}/; + my $begre = qr/[^ \/\-_:\.@]{0,$add_len}.{0,$len}/; + + if (defined $opts{'-cut'} && $opts{'-cut'} eq 'left') { + $str =~ m/^(.*?)($begre)$/; + my ($lead, $body) = ($1, $2); + if (length($lead) > 4) { + if ($lead =~ m/&[^;]*$/) { + $body =~ s/^[^;]*;//; + } + $lead = "... "; + } + return "$lead$body"; + + } elsif (defined $opts{'-cut'} && $opts{'-cut'} eq 'center') { + $str =~ m/^($endre)(.*)$/; + my ($left, $str) = ($1, $2); + $str =~ m/^(.*?)($begre)$/; + my ($mid, $right) = ($1, $2); + if (length($mid) > 5) { + $left =~ s/&[^;]*$//; + if ($mid =~ m/&[^;]*$/) { + $right =~ s/^[^;]*;//; + } + $mid = " ... "; + } + return "$left$mid$right"; + + } else { + $str =~ m/^($endre)(.*)$/; + my $body = $1; + my $tail = $2; + if (length($tail) > 4) { + $body =~ s/&[^;]*$//; + $tail = " ..."; + } + return "$body$tail"; } - return "$body$tail"; } # takes the same arguments as chop_str, but also wraps a <span> around the # result with a title attribute if it does get chopped. Additionally, the # string is HTML-escaped. sub chop_and_escape_str { - my $str = shift; - my $len = shift; - my $add_len = shift || 10; + my ($str) = @_; - my $chopped = chop_str($str, $len, $add_len); + my $chopped = chop_str(@_); if ($chopped eq $str) { return esc_html($chopped); } else {
@@ -3791,11 +3833,11 @@ sub git_search_grep_body { foreach my $line (@$comment) { if ($line =~ m/^(.*)($search_regexp)(.*)$/i) { my ($lead, $match, $trail) = ($1, $2, $3); - $match = chop_str($match, 70, 5); # in case match is very long - my $contextlen = int((80 - length($match))/2); # for the remainder - $contextlen = 30 if ($contextlen > 30); # but not too much - $lead = chop_str($lead, $contextlen, 10); - $trail = chop_str($trail, $contextlen, 10); + $match = chop_str($match, 70, 5, -cut=>'center'); + my $contextlen = int((80 - length($match))/2); + $contextlen = 30 if ($contextlen > 30); + $lead = chop_str($lead, $contextlen, 10, -cut=>'left'); + $trail = chop_str($trail, $contextlen, 10, -cut=>'right'); $lead = esc_html($lead); $match = esc_html($match);
PATCH v2 1/1 gitweb: Option to chop at beginning and in the middle in chop_str
In reply to Jakub NarebskiAdd support for '-cut' option to chop_str subroutine, to cut at the beginning (from the left side of the string), in the middle (center of the string), or at the end (from the right side of the string) which is the default:
chop_str(somestring, len, slop, -cut=>'left') -> ' ...string' chop_str(somestring, len, slop, -cut=>'center') -> 'som ... ing' chop_str(somestring, len, slop, -cut=>'right') -> 'somestr... '
While at it return from chop_str early if given string is so short that chop_str couldn't shorten it, and simplify regexp used. Make ellipsis (ots) stick to shorthened fragment for cutting at ends.
Simplify passing all arguments to chop_str in chop_and_escape_str subroutine. This was needed to pass additional options to chop_str.
Make use of new feature of chop_str to better cut matched string and its context in match info for searching commit messages (commit search), as proposed by Junio C Hamano. For example, if you are looking for "very long ... and how" in the first paragraph of message (if it were all on a single line), you would now see:
...st this with <<very long ... and how>> the actual out...
instead of:
Could som... <<very long search stri...>> the actual out...
(where <<something>> denotes emphasized / colored fragment).
---
This is second version of the patch, with regexp simplified, and early return. Rudimentarly tested.
gitweb/gitweb.perl | 77 +++++++++++++++++++++++++++++++++++++++++----------- 1 files changed, 61 insertions(+), 16 deletions(-)
gitweb/gitweb.perl +61 -16
@@ -848,32 +848,77 @@ sub project_in_list { ## ---------------------------------------------------------------------- ## HTML aware string manipulation +# Try to chop given string on a word boundary between position +# $len and $len+$add_len. If there is no word boundary there, +# chop at $len+$add_len. Do not chop if chopped part plus ellipsis +# (marking chopped part) would be longer than given string. sub chop_str { my $str = shift; my $len = shift; my $add_len = shift || 10; + my $where = shift || 'right' # 'left' | 'center' | 'right' # allow only $len chars, but don't cut a word if it would fit in $add_len # if it doesn't fit, cut it if it's still longer than the dots we would add - $str =~ m/^(.{0,$len}[^ \/\-_:\.@]{0,$add_len})(.*)/; - my $body = $1; - my $tail = $2; - if (length($tail) > 4) { - $tail = " ..."; - $body =~ s/&[^;]*$//; # remove chopped character entities + # remove chopped character entities entirely + + # when chopping in the middle, distribute $len into left and right part + # return early if chupping wouldn't make string shorter + if ($where eq 'center') { + return $str if ($len + 5 >= length($str)); # filler is length 5 + $len = int($len/2); + } else { + return $str if ($len + 4 >= length($str)); # filler is length 4 + } + + # regexps: ending and beginning with word part up to $add_len + my $endre = qr/.{$len}\w{0,$add_len}/; + my $begre = qr/\w{0,$add_len}.{$len}/; + + if ($where eq 'left') { + $str =~ m/^(.*?)($begre)$/; + my ($lead, $body) = ($1, $2); + if (length($lead) > 4) { + if ($lead =~ m/&[^;]*$/) { + $body =~ s/^[^;]*;//; + } + $lead = " ..."; + } + return "$lead$body"; + + } elsif ($where eq 'center') { + $str =~ m/^($endre)(.*)$/; + my ($left, $str) = ($1, $2); + $str =~ m/^(.*?)($begre)$/; + my ($mid, $right) = ($1, $2); + if (length($mid) > 5) { + $left =~ s/&[^;]*$//; + if ($mid =~ m/&[^;]*$/) { + $right =~ s/^[^;]*;//; + } + $mid = " ... "; + } + return "$left$mid$right"; + + } else { + $str =~ m/^($endre)(.*)$/; + my $body = $1; + my $tail = $2; + if (length($tail) > 4) { + $body =~ s/&[^;]*$//; + $tail = "... "; + } + return "$body$tail"; } - return "$body$tail"; } # takes the same arguments as chop_str, but also wraps a <span> around the # result with a title attribute if it does get chopped. Additionally, the # string is HTML-escaped. sub chop_and_escape_str { - my $str = shift; - my $len = shift; - my $add_len = shift || 10; + my ($str) = @_; - my $chopped = chop_str($str, $len, $add_len); + my $chopped = chop_str(@_); if ($chopped eq $str) { return esc_html($chopped); } else {
@@ -3791,11 +3836,11 @@ sub git_search_grep_body { foreach my $line (@$comment) { if ($line =~ m/^(.*)($search_regexp)(.*)$/i) { my ($lead, $match, $trail) = ($1, $2, $3); - $match = chop_str($match, 70, 5); # in case match is very long - my $contextlen = int((80 - length($match))/2); # for the remainder - $contextlen = 30 if ($contextlen > 30); # but not too much - $lead = chop_str($lead, $contextlen, 10); - $trail = chop_str($trail, $contextlen, 10); + $match = chop_str($match, 70, 5, 'center'); + my $contextlen = int((80 - length($match))/2); + $contextlen = 30 if ($contextlen > 30); + $lead = chop_str($lead, $contextlen, 10, 'left'); + $trail = chop_str($trail, $contextlen, 10, 'right'); $lead = esc_html($lead); $match = esc_html($match);
Signature · 2 lines
Stacked GIT 0.14.1 git version 1.5.4.2
Re: [RFC/PATCH v2] gitweb: Option to chop at beginning and in the middle in chop_str
Quoted 6 lines · Jakub Narebski
Make use of new feature of
chop_strto better cut matched string and its context in match info for searching commit messages (commit search), as proposed by Junio C Hamano. For example, if you are looking for "very long ... and how" in the first paragraph of this message (if it were all on a single line), you would now see:
Quoted 7 lines
...st this with <<very long ... and how>> the actual out...
instead of:
Could som... <<very long search stri...>> the actual out...
(where <<something>> denotes emphasized / colored fragment).
This part needs rewritten; the first paragraph of what message is that?
Also I think the subject is wrong. Yes, it is adding an option to an internal subroutine but who cares? The net effect the "gitweb" users see is that the way the grep result is shown differently, hopefully in a more understandable way, and that change is not _optional_ at all.
The code looks easier to read than before, but I may be partial ;-)
PATCH v3 1/1 gitweb: Better cutting matched string and its context
Quoted 10 lines · Junio C Hamano
Quoted 8 lines · Jakub Narebski
Make use of new feature of
chop_strto better cut matched string and its context in match info for searching commit messages (commit search), as proposed by Junio C Hamano. For example, if you are looking for "very long ... and how" in the first paragraph of this message (if it were all on a single line), you would now see:...st this with <<very long ... and how>> the actual out...
Quoted 7 lines
Quoted 5 lines
instead of:
Could som... <<very long search stri...>> the actual out...
(where <<something>> denotes emphasized / colored fragment).
This part needs rewritten; the first paragraph of what message is that?
Ooops. Sorry about that. I have forgot to transport context when doing copy'n'paste.
BTW. the example was subtly wrong: you can search _lines_, like grep, you cannot search multiline.
Quoted 5 lines
Also I think the subject is wrong. Yes, it is adding an option to an internal subroutine but who cares? The net effect the "gitweb" users see is that the way the grep result is shown differently, hopefully in a more understandable way, and that change is not _optional_ at all.
Below there is corrected commit message (rewritten to match code).
Quoted 2 lines
The code looks easier to read than before, but I may be partial ;-)
Further improved, IMVHO.
BTW. I have queued 3-patch series further improving gitweb search, to be sent soon...
.....................................................................
-- >8 --
From: Jakub Narebski <[email protected]>
Subject: [PATCH] gitweb: Better cutting matched string and its context
Improve look of commit search output ('search' view) by better cutting of matched string and its context in match info, as proposed by Junio Hamano. For example, if you are looking for "very long search string" in the following line:
Could somebody test this with very long search string, and see how
you would now see:
...this with <<very long ... string>>, and see...
instead of:
Could som... <<very long search...>>, and see...
(where <<something>> denotes emphasized / colored fragment; matched fragment to be more exact).
For this feature support for fourth [optional] parameter to chop_str subroutine was added. This fourth parameter is used to denote where to cut string to make it shorter. chop_str can now cut at the beginning (from the _left_ side of the string), in the middle (_center_ of the string), or at the end (from the _right_ side of the string); cutting from right is the default:
chop_str(somestring, len, slop, 'left') -> ' ...string' chop_str(somestring, len, slop, 'center') -> 'som ... ing' chop_str(somestring, len, slop, 'right') -> 'somestr... '
If you want to use default slop (default additional length), use undef as value for third parameter to chop_str.
While at it, return from chop_str early if given string is so short that chop_str couldn't shorten it. Simplify also regexp used by chop_str. Make ellipsis (dots) stick to shortened fragment for cutting at ends, to better see which part got shortened.
Simplify passing all arguments to chop_str in chop_and_escape_str subroutine. This was needed to pass additional options to chop_str.
--- gitweb/gitweb.perl | 73 ++++++++++++++++++++++++++++++++++++++++----------- 1 files changed, 57 insertions(+), 16 deletions(-)
gitweb/gitweb.perl +57 -16
@@ -848,32 +848,73 @@ sub project_in_list { ## ---------------------------------------------------------------------- ## HTML aware string manipulation +# Try to chop given string on a word boundary between position +# $len and $len+$add_len. If there is no word boundary there, +# chop at $len+$add_len. Do not chop if chopped part plus ellipsis +# (marking chopped part) would be longer than given string. sub chop_str { my $str = shift; my $len = shift; my $add_len = shift || 10; + my $where = shift || 'right'; # 'left' | 'center' | 'right' # allow only $len chars, but don't cut a word if it would fit in $add_len # if it doesn't fit, cut it if it's still longer than the dots we would add - $str =~ m/^(.{0,$len}[^ \/\-_:\.@]{0,$add_len})(.*)/; - my $body = $1; - my $tail = $2; - if (length($tail) > 4) { - $tail = " ..."; - $body =~ s/&[^;]*$//; # remove chopped character entities + # remove chopped character entities entirely + + # when chopping in the middle, distribute $len into left and right part + # return early if chopping wouldn't make string shorter + if ($where eq 'center') { + return $str if ($len + 5 >= length($str)); # filler is length 5 + $len = int($len/2); + } else { + return $str if ($len + 4 >= length($str)); # filler is length 4 + } + + # regexps: ending and beginning with word part up to $add_len + my $endre = qr/.{$len}\w{0,$add_len}/; + my $begre = qr/\w{0,$add_len}.{$len}/; + + if ($where eq 'left') { + $str =~ m/^(.*?)($begre)$/; + my ($lead, $body) = ($1, $2); + if (length($lead) > 4) { + $body =~ s/^[^;]*;// if ($lead =~ m/&[^;]*$/); + $lead = " ..."; + } + return "$lead$body"; + + } elsif ($where eq 'center') { + $str =~ m/^($endre)(.*)$/; + my ($left, $str) = ($1, $2); + $str =~ m/^(.*?)($begre)$/; + my ($mid, $right) = ($1, $2); + if (length($mid) > 5) { + $left =~ s/&[^;]*$//; + $right =~ s/^[^;]*;// if ($mid =~ m/&[^;]*$/); + $mid = " ... "; + } + return "$left$mid$right"; + + } else { + $str =~ m/^($endre)(.*)$/; + my $body = $1; + my $tail = $2; + if (length($tail) > 4) { + $body =~ s/&[^;]*$//; + $tail = "... "; + } + return "$body$tail"; } - return "$body$tail"; } # takes the same arguments as chop_str, but also wraps a <span> around the # result with a title attribute if it does get chopped. Additionally, the # string is HTML-escaped. sub chop_and_escape_str { - my $str = shift; - my $len = shift; - my $add_len = shift || 10; + my ($str) = @_; - my $chopped = chop_str($str, $len, $add_len); + my $chopped = chop_str(@_); if ($chopped eq $str) { return esc_html($chopped); } else {
@@ -3791,11 +3832,11 @@ sub git_search_grep_body { foreach my $line (@$comment) { if ($line =~ m/^(.*)($search_regexp)(.*)$/i) { my ($lead, $match, $trail) = ($1, $2, $3); - $match = chop_str($match, 70, 5); # in case match is very long - my $contextlen = int((80 - length($match))/2); # for the remainder - $contextlen = 30 if ($contextlen > 30); # but not too much - $lead = chop_str($lead, $contextlen, 10); - $trail = chop_str($trail, $contextlen, 10); + $match = chop_str($match, 70, 5, 'center'); + my $contextlen = int((80 - length($match))/2); + $contextlen = 30 if ($contextlen > 30); + $lead = chop_str($lead, $contextlen, 10, 'left'); + $trail = chop_str($trail, $contextlen, 10, 'right'); $lead = esc_html($lead); $match = esc_html($match);
Signature · 1 line
1.5.4.2
Re: [RFC/PATCH v3] gitweb: Better cutting matched string and its context
Quoted 5 lines · Jakub Narebski
Ooops. Sorry about that. I have forgot to transport context when doing copy'n'paste.
...
BTW. the example was subtly wrong: you can search _lines_, like grep, you cannot search multiline.
Yeah, the original had:
For example, if you are looking for "very long ... and how" in the first paragraph of message (if it were all on a single line), wouldn't you want to see:
and you dropped "if it were all on a single line" part, as you forgot to transport context when doing copy-n-paste ;-)
Quoted 1 line
Below there is corrected commit message (rewritten to match code).
Thanks. Will take a look later; I am at day-job today.
Re: [PATCH] gitweb: Better chopping in commit search results
In reply to Jakub NarebskiQuoted 3 lines · Jakub Narebski
# regexps: ending and beginning with word part up to $add_len
my $endre = qr/.{0,$len}[^ \/\-_:\.@]{0,$add_len}/;
my $begre = qr/[^ \/\-_:\.@]{0,$add_len}.{0,$len}/;I have no idea what these line noise characters inside [] are. Did you mean something like "\w"?
I have a suspicion that it may be easier to read and could be even more efficient to split an overlong line at word boundaries and to remove elements from the end you are removing from until it fits.
sub chop_whence {
my ($line, $max, $slop, $where) = @_;
my $len = length($line);
if ($len < $max + $slop) {
return $line;
}
# Cut at word boundaries
my @split = split(/\b/, $line);
my $filler = "...";
while ((2 < @split)) {
my $removed;
my $splice_at;
if ($where eq 'left') {
$removed = shift @split;
} elsif ($where eq 'right') {
$removed = pop @split;
} else {
my $splice_at = int($#split / 2);
$removed = splice(@split, $splice_at, 1);
}
$len -= length($removed) + length($filler);
if ($len < $max + $slop) {
if ($where eq 'left') {
unshift @split, $filler;
} elsif ($where eq 'right') {
push @split, $filler;
} else {
my $splice_at = int($#split / 2);
splice(@split, $splice_at, 0, $filler);
}
return join('', @split);
}
}
# give up
return $line;
}Re: [PATCH] gitweb: Better chopping in commit search results
Quoted 8 lines · Junio C Hamano
Quoted 3 lines · Jakub Narebski
# regexps: ending and beginning with word part up to $add_len my $endre = qr/.{0,$len}[^ \/\-_:\.@]{0,$add_len}/; my $begre = qr/[^ \/\-_:\.@]{0,$add_len}.{0,$len}/;I have no idea what these line noise characters inside [] are. Did you mean something like "\w"?
They were in original chop_str, written by Kay Sievers if I have checked correctly, I have only repeated it. But changing it to "\w" might be a good idea.
Quoted 7 lines
I have a suspicion that it may be easier to read and could be even more efficient to split an overlong line at word boundaries and to remove elements from the end you are removing from until it fits.
sub chop_whence { my ($line, $max, $slop, $where) = @_;
IMHO it is neither easier to read, nor more efficient. And changes semantic a bit.
Original chop_str used to mean:
chop_str($str, $len, $add_len) means: Try to chop on a word boundary between position $len and $len+$add_len. If there is no word boundary between $len and $len+$add_len, chop at $add_len, and replace chopped part by " ...". Do not chop if to be replaced part is shorter than ellipsis, i.e. " ..." replacement.
(I think I add this description to gitweb). With this description the code is I think quite obvious.
Signature · 2 lines
Jakub Narebski Poland
Re: [PATCH] gitweb: Better chopping in commit search results
In reply to Jakub NarebskiQuoted 3 lines · Jakub Narebski
Strange that StGit always sends patches (stg mail) as if repo owner was their author, regardless of path/commit author (I think; unless "stg edit" cannot change authorship).
It'll always put the sender in the From: header in the mail, obviously. But the patch mail template contains a "%(fromauth)s" string, which will be replaced by "From: Real Author <[email protected]>\n", or the empty string if the patch author is the same as the sender.
Your problem might be that you're using an old template.
Signature · 2 lines
Karl Hasselström, [email protected] www.treskal.com/kalle
Re: [PATCH] gitweb: Better chopping in commit search results
Quoted 6 lines · Karl Hasselström
Quoted 3 lines · Jakub Narebski
Strange that StGit always sends patches (stg mail) as if repo owner was their author, regardless of path/commit author (I think; unless "stg edit" cannot change authorship).
Quoted 7 lines
It'll always put the sender in the From: header in the mail, obviously. But the patch mail template contains a "%(fromauth)s" string, which will be replaced by "From: Real Author <[email protected]>\n", or the empty string if the patch author is the same as the sender.
Your problem might be that you're using an old template.
Thanks a lot. That was it.
Signature · 2 lines
Jakub Narebski Poland
Quoted 6 lines · Jean-Baptiste Quenot
Thanks for Git! It's a great program. I encountered an annoying bug with gitweb 1.5.4.1, when searching for commits, if the search string is too long, the generated HTML is munged leading to an ill-formed XHTML document.
Here is the patch, hope it helps:
It would be better if the patch wasn't wrapped and whitespace corrupted.
Quoted 6 lines
gitweb/gitweb.perl +0
-0@@ -3780,7 +3780,10 @@ sub git_search_grep_body { my $trail = esc_html($3) || "";
gitweb/gitweb.perl sub git_search_grep_body { · line 3789
Quoted 10 lines
gitweb/gitweb.perl +4
-1$trail = chop_str($trail, 30, 10); my $text = "$lead<span class=\"match\">$match</span>$trail"; - print chop_str($text, 80, 5) . "<br/>\n"; + # Do not chop $text as match can be long, and we don't want to + # munge HTML tags! + #print chop_str($text, 80, 5) . "<br/>\n"; + print $text . "<br/>\n"; } } print "</td>\n" .
While this might be good bigfix, I think it is not a good solution. If we select to show only neighbourhood of the match, we should probably chop trailing text, or both leading (cutting at beginning) and trailing, perhaps even match if it is overly long.
Or we could alternatively show all lines of commit message, and only mark matching fragment... but that I think would have to wait for refactoring of generation of log-like views (log, shorlog, history, rss) in gitweb.
Signature · 3 lines
Jakub Narebski Poland ShadeHawk on #git