r61661 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r61660‎ | r61661 | r61662 >
Date:13:39, 29 January 2010
Author:raymond
Status:ok (Comments)
Tags:
Comment:
* (bug 22181) Do not truncate if the ellipsis actually make the string longer
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/languages/Language.php (modified) (history)

Diff [purge]

Index: trunk/phase3/languages/Language.php
@@ -2153,6 +2153,7 @@
21542154 if ( strlen( $string ) <= abs( $length ) ) {
21552155 return $string;
21562156 }
 2157+ $stringOriginal = $string;
21572158 if( $length > 0 ) {
21582159 $string = substr( $string, 0, $length );
21592160 $char = ord( $string[strlen( $string ) - 1] );
@@ -2166,7 +2167,13 @@
21672168 # We chopped in the middle of a character; remove it
21682169 $string = $m[1];
21692170 }
2170 - return $string . $ellipsis;
 2171+ # Do not truncate if the ellipsis actually make the string longer. Bug 22181
 2172+ if ( strlen( $string ) + strlen( $ellipsis ) < strlen( $stringOriginal ) ) {
 2173+ return $string . $ellipsis;
 2174+ } else {
 2175+ return $stringOriginal;
 2176+ }
 2177+
21712178 } else {
21722179 $string = substr( $string, $length );
21732180 $char = ord( $string[0] );
@@ -2174,7 +2181,12 @@
21752182 # We chopped in the middle of a character; remove the whole thing
21762183 $string = preg_replace( '/^[\x80-\xbf]+/', '', $string );
21772184 }
2178 - return $ellipsis . $string;
 2185+ # Do not truncate if the ellipsis actually make the string longer. Bug 22181
 2186+ if ( strlen( $string ) + strlen( $ellipsis ) < strlen( $stringOriginal ) ) {
 2187+ return $ellipsis . $string;
 2188+ } else {
 2189+ return $stringOriginal;
 2190+ }
21792191 }
21802192 }
21812193
Index: trunk/phase3/RELEASE-NOTES
@@ -724,6 +724,7 @@
725725 and Chick skins
726726 * Fixed bug involving unclosed "-{" markup in the language converter
727727 * (bug 21870) No longer include Google logo from an external server on wiki error.
 728+* (bug 22181) Do not truncate if the ellipsis actually make the string longer
728729
729730 == API changes in 1.16 ==
730731

Comments

#Comment by Platonides (talk | contribs)   17:33, 29 January 2010

What about doing $string = $ellipsis . $string / $string = $string . $ellipsis and move the new if/else out of both branches?

In the second case the ord + if could be removed altoghether: $string = preg_replace( '/^[\x80-\xbf]*/', StringUtils::escapeRegexReplacement($ellipsis), $string );

#Comment by Raymond (talk | contribs)   21:22, 11 February 2010

First part done in r62326

Second part should be done by someone with better regex knowledge

#Comment by Platonides (talk | contribs)   21:53, 11 February 2010

It's not an issue of knowing what the regex does. The line I provided is equivalent to the else. The reason why we might not do it is that it may not provide a benefit doing it that way, and it does make it harder to read.

Status & tagging log