r46639 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r46638‎ | r46639 | r46640 >
Date:11:43, 31 January 2009
Author:werdna
Status:reverted (Comments)
Tags:
Comment:
Fix r46628 -- I'd misunderstood the nature of the hack. People wanted to append the string to be truncated to an empty string, not the reverse.
Modified paths:
  • /trunk/phase3/includes/parser/CoreParserFunctions.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/parser/CoreParserFunctions.php
——@@ -381,7 +381,11 @@
382382 static function pad( $string, $length, $padding = '0', $direction = STR_PAD_RIGHT ) {
383383 $lengthOfPadding = mb_strlen( $padding );
384384 if ( $lengthOfPadding == 0 ) return $string;
385 - if ( $length < mb_strlen( $string ) ) return $string;
 385+
 386+ // Thwart attempts to use this function to truncate strings.
 387+ // We don't want people implementing ParserFunctions in template,
 388+ // for performance and usability reasons.
 389+ if ($lengthOfPadding > $length && $string == '') return $string;
386390
387391 # The remaining length to add counts down to 0 as padding is added
388392 $length = min( $length, 500 ) - mb_strlen( $string );

Follow-up revisions

RevisionCommit summaryAuthorDate
r47411Revert r46628, r46639 for now "Don't allow padding parser functions to be use...brion02:24, 18 February 2009

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r46628Don't allow padding parser functions to be used to truncate strings. This bre...werdna00:46, 31 January 2009

Comments

#Comment by Simetrical (talk | contribs)   00:22, 1 February 2009

$string == '' seems fragile. You could make $string whitespace, for instance, or contrive some other way to strip it from the left. Why is that condition there in the first place? It seems like $lengthOfPadding > $length would suffice.

That said, I'm not sure there's any point in trying to kill w:Template:Trunc. This change might remove useful and non-hacky functionality somehow, and really, the hack isn't so ugly. It's a lot nicer than {{qif}} ― and we didn't respond to that by removing parameter defaults, we added #if. Other templates like w:Template:Strlen still exist too.

IMO, at this point there's no mileage in trying to kill one or two rarely-used templates that may or may not contribute to reduced usability. It's a drop in the bucket of the horrifying state of template usability on enwiki. You're not going to accomplish anything useful and you're going to annoy users. (As for performance, that's unlikely to be a concern in this specific case.)

#Comment by Brion VIBBER (talk | contribs)   02:42, 18 February 2009

Reverted this for now in r47411.

Status & tagging log