r86764 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r86763‎ | r86764 | r86765 >
Date:13:23, 23 April 2011
Author:raymond
Status:reverted (Comments)
Tags:
Comment:
Embrace comment with () only when really needed. In Special:ListFiles and ImagePage the comment is shown alone in a table cell -> no () needed
Modified paths:
  • /trunk/phase3/includes/ImagePage.php (modified) (history)
  • /trunk/phase3/includes/Linker.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialListfiles.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/ImagePage.php
@@ -1097,7 +1097,7 @@
10981098 if ( $file->isDeleted( File::DELETED_COMMENT ) ) {
10991099 $row .= '<span class="history-deleted">' . wfMsgHtml( 'rev-deleted-comment' ) . '</span>';
11001100 } else {
1101 - $row .= $this->skin->commentBlock( $description, $this->title );
 1101+ $row .= $this->skin->commentBlock( $description, $this->title, false, false );
11021102 }
11031103 $row .= '</td>';
11041104
Index: trunk/phase3/includes/Linker.php
@@ -1292,10 +1292,10 @@
12931293 * @param $comment String
12941294 * @param $title Mixed: Title object (to generate link to section in autocomment) or null
12951295 * @param $local Boolean: whether section links should refer to local page
1296 - *
 1296+ * @param $embraced Boolean: whether the formatted comment should be embraced with ()
12971297 * @return string
12981298 */
1299 - static function commentBlock( $comment, $title = null, $local = false ) {
 1299+ static function commentBlock( $comment, $title = null, $local = false, $embraced = true ) {
13001300 // '*' used to be the comment inserted by the software way back
13011301 // in antiquity in case none was provided, here for backwards
13021302 // compatability, acc. to brion -ævar
@@ -1303,7 +1303,10 @@
13041304 return '';
13051305 } else {
13061306 $formatted = self::formatComment( $comment, $title, $local );
1307 - return " <span class=\"comment\">($formatted)</span>";
 1307+ if ( $embraced ) {
 1308+ $formatted = wfMessage( 'parentheses', $formatted );
 1309+ }
 1310+ return Html::rawElement( 'span', array( 'class' => 'comment' ), $formatted );
13081311 }
13091312 }
13101313
Index: trunk/phase3/includes/specials/SpecialListfiles.php
@@ -215,7 +215,7 @@
216216 case 'img_size':
217217 return $this->getSkin()->formatSize( $value );
218218 case 'img_description':
219 - return $this->getSkin()->commentBlock( $value );
 219+ return $this->getSkin()->commentBlock( $value, null, false, false );
220220 case 'count':
221221 return intval($value)+1;
222222 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r86827Followup r86764: don't parse the comment. Spotted by SPQRobin@Translatewiki a...raymond17:58, 24 April 2011
r89134Add back space between size and comment in history pages (fix for r86764)aaron20:33, 29 May 2011
r90101Revert r86764, r89134, r86827 -- added a second opaque boolean parameter to L...brion23:50, 14 June 2011

Comments

#Comment by Bawolff (talk | contribs)   01:03, 8 May 2011

Minor issue: Previously it returned an element with a leading space before it, now it doesn't. I think this causes the history page view to have entries like:

20:23, 13 December 2010 Some user m (1,897 bytes)(Some edit summary) (undo) 

Note the missing space between the edit summary and the byte count.

#Comment by Aaron Schulz (talk | contribs)   20:39, 29 May 2011

You should grep for "commentBlock" and fix any callers. I think there are still some out there that need tweaking.

#Comment by Brion VIBBER (talk | contribs)   23:36, 14 June 2011
commentBlock( $value, null, false, false );

Adding more opaque boolean parameters is generally considered poor form.

Further, Linker::commentBlock()'s reason for existing in the first place is to add the parentheses... if you don't want them, you should call Linker::formatComment().

#Comment by Brion VIBBER (talk | contribs)   23:51, 14 June 2011

Reverted in r90101.

Status & tagging log