r110568 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r110567‎ | r110568 | r110569 >
Date:14:15, 2 February 2012
Author:catrope
Status:ok (Comments)
Tags:miscextensions 
Comment:
Fix bug 33768, introduced in r104041: saveChanges() needs to be run even if $changedStrings is empty. Not doing so causes fallback weirdness.
Modified paths:
  • /trunk/extensions/LocalisationUpdate/LocalisationUpdate.class.php (modified) (history)

Diff [purge]

Index: trunk/extensions/LocalisationUpdate/LocalisationUpdate.class.php
@@ -410,14 +410,16 @@
411411 $changedStrings = array_diff_assoc( $base_messages, $compare_messages );
412412
413413 // If we want to save the differences.
414 - if ( $saveResults && !empty( $changedStrings ) && is_array( $changedStrings ) ) {
 414+ // HACK: because of the hack in saveChanges(), we need to call that function
 415+ // even if $changedStrings is empty. So comment out the $changedStrings checks below.
 416+ if ( $saveResults /* && !empty( $changedStrings ) && is_array( $changedStrings )*/ ) {
415417 self::myLog( "--Checking languagecode {$langcode}--", $verbose );
416418 // Save the differences.
417419 $updates = self::saveChanges( $changedStrings, $forbiddenKeys, $compare_messages, $base_messages, $langcode, $verbose );
418420 self::myLog( "{$updates} messages updated for {$langcode}.", $verbose );
419 - } elseif ( $saveResults ) {
 421+ } /*elseif ( $saveResults ) {
420422 self::myLog( "--{$langcode} hasn't changed--", $verbose );
421 - }
 423+ }*/
422424
423425 self::saveHash( $basefile, $basehash );
424426
@@ -629,14 +631,16 @@
630632 $changedStrings = array_diff_assoc( $messages, $compare_messages[$language] );
631633
632634 // If we want to save the changes.
633 - if ( $saveResults === true && !empty( $changedStrings ) && is_array( $changedStrings ) ) {
 635+ // HACK: because of the hack in saveChanges(), we need to call that function
 636+ // even if $changedStrings is empty. So comment out the $changedStrings checks below.
 637+ if ( $saveResults === true /*&& !empty( $changedStrings ) && is_array( $changedStrings )*/ ) {
634638 self::myLog( "--Checking languagecode {$language}--", $verbose );
635639 // The save them
636640 $updates = self::saveChanges( $changedStrings, $forbiddenKeys, $compare_messages[$language], $messages, $language, $verbose );
637641 self::myLog( "{$updates} messages updated for {$language}.", $verbose );
638 - } elseif($saveResults === true) {
 642+ }/* elseif($saveResults === true) {
639643 self::myLog( "--{$language} hasn't changed--", $verbose );
640 - }
 644+ }*/
641645 }
642646
643647 // And log some stuff.

Follow-up revisions

RevisionCommit summaryAuthorDate
r1105701.18wmf1: MFT r110568catrope14:18, 2 February 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r104041Followup r103763: per Liangent's CR comment, there was a bug where an unchang...catrope15:05, 23 November 2011

Comments

#Comment by 😂 (talk | contribs)   15:00, 2 February 2012

Not your fault, but we shouldn't need empty() here.

#Comment by Aaron Schulz (talk | contribs)   00:58, 3 February 2012

A part of me died while reading this file.

#Comment by 😂 (talk | contribs)   01:00, 3 February 2012

And which part would that be?

#Comment by Aaron Schulz (talk | contribs)   01:01, 3 February 2012

You don't want to know...

#Comment by Catrope (talk | contribs)   16:17, 8 February 2012

As it should have. I've added comments to this file before saying it should be totally rewritten. Hopefully the i18n team will do that some time.

Status & tagging log