r104041 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r104040‎ | r104041 | r104042 >
Date:15:05, 23 November 2011
Author:catrope
Status:ok (Comments)
Tags:
Comment:
Followup r103763: per Liangent's CR comment, there was a bug where an unchanged message would cause the fallback language to be accessed instead. Per my CR comment, this can be solved by explicitly storing unchanged messages (but not storing messages whose English translations have changed). I've also revised the logic for carrying over stored messages from the cache file: previously messages would never be removed from the cache file, which would cause issues when people deliberately remove messages to let the fallback kick in. The logic has now been changed to remove messages from the LU cache if they've been removed from trunk, but to keep them (at their cached version) if they still exist in trunk but their English translation has changed.
Modified paths:
  • /trunk/extensions/LocalisationUpdate/LocalisationUpdate.class.php (modified) (history)

Diff [purge]

Index: trunk/extensions/LocalisationUpdate/LocalisationUpdate.class.php
@@ -450,21 +450,30 @@
451451 return 0;
452452 }
453453
454 - $new_messages = self::readFile( $langcode );
 454+ $new_messages = array();
455455
456 - foreach ( $changedStrings as $key => $value ) {
457 - // If this message wasn't changed in English.
458 - if ( !isset( $forbiddenKeys[$key] ) ) {
 456+ //foreach ( $changedStrings as $key => $value ) {
 457+ // HACK for r103763 CR: store all messages, even unchanged ones
 458+ // TODO this file is a mess and needs to be rewritten
 459+ foreach ( array_merge( array_keys( $base_messages, $compare_messages ) as $key ) {
 460+ // Only update the translation if this message wasn't changed in English
 461+ if ( !isset( $forbiddenKeys[$key] ) && isset( $base_messages[$key] ) ) {
459462 $new_messages[$key] = $base_messages[$key];
460463
461 - // Output extra logmessages when needed.
462 - if ( $verbose ) {
463 - $oldmsg = isset( $compare_messages[$key] ) ? "'{$compare_messages[$key]}'" : 'not set';
464 - self::myLog( "Updated message {$key} from $oldmsg to '{$base_messages[$key]}'", $verbose );
 464+ if ( !isset( $compare_messages[$key] ) || $compare_messages[$key] !== $base_messages[$key] ) {
 465+ // Output extra logmessages when needed.
 466+ if ( $verbose ) {
 467+ $oldmsg = isset( $compare_messages[$key] ) ? "'{$compare_messages[$key]}'" : 'not set';
 468+ self::myLog( "Updated message {$key} from $oldmsg to '{$base_messages[$key]}'", $verbose );
 469+ }
 470+
 471+ // Update the counter.
 472+ $updates++;
465473 }
466 -
467 - // Update the counter.
468 - $updates++;
 474+ } else if ( isset( $forbiddenKeys[$key] ) && isset( $compare_messages[$key] ) ) {
 475+ // The message was changed in English, but a previous translation still exists in the cache.
 476+ // Use that previous translation rather than falling back to the .i18n.php file
 477+ $new_messages[$key] = $compare_messages[$key];
469478 }
470479 }
471480 self::writeFile( $langcode, $new_messages );

Follow-up revisions

RevisionCommit summaryAuthorDate
r104206Fix syntax error from r104041reedy00:06, 25 November 2011
r110568Fix bug 33768, introduced in r104041: saveChanges() needs to be run even if $...catrope14:15, 2 February 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r103763(bug 31982) Handle fallback languages in LocalisationUpdate. Modified patch b...catrope15:25, 20 November 2011

Comments

#Comment by Umherirrender (talk | contribs)   15:43, 23 November 2011

Sounds like bug 26325 is also fixed with this change.

#Comment by Liangent (talk | contribs)   15:45, 23 November 2011

It's not fixed. See my comment on r103763.

#Comment by Liangent (talk | contribs)   15:46, 23 November 2011

More exactly, not completely fixed. ie. only fixed for messages that fall back to a non-English language.

#Comment by Catrope (talk | contribs)   09:32, 24 November 2011

Hah, nice edge case. I'll be away and/or busy with other things for the next few days, so I'll revisit this on Wednesday if no one else does before then.

#Comment by Liangent (talk | contribs)   08:51, 10 December 2011

There is another way to go: iterate over all keys in English where English text is not changed when updating (= building cache), then calculate (resolve fallback now) its message text in every language and store it in cache. When recaching the message, simply merging cached data into main data is fine.

#Comment by Catrope (talk | contribs)   12:04, 14 December 2011

Hmm, that would work, but requires rewriting parts of LU. I'll keep this on my TODO list but I'm gonna deploy these changes now. There's still a bug in fallback handling, but that's better than having no fallback handling whatsoever.

#Comment by Liangent (talk | contribs)   12:30, 14 December 2011

Anyway it could be nice if it can be seen in production asap. Some people cannot be more eager because some badly translated messages are just after many articles in ArticleFeedback.

Status & tagging log