r77452 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r77451‎ | r77452 | r77453 >
Date:03:04, 30 November 2010
Author:philip
Status:resolved (Comments)
Tags:
Comment:
Select a friendly sub-language while acquiring system messages if the uselang has multiple variants available for use.
Modified paths:
  • /trunk/phase3/includes/MessageCache.php (modified) (history)
  • /trunk/phase3/languages/Language.php (modified) (history)

Diff [purge]

Index: trunk/phase3/languages/Language.php
@@ -1545,11 +1545,11 @@
15461546 }
15471547
15481548 function getMessage( $key ) {
1549 - return self::$dataCache->getSubitem( $this->mCode, 'messages', $key );
 1549+ return self::$dataCache->getSubitem( $this->getCodeForMessage(), 'messages', $key );
15501550 }
15511551
15521552 function getAllMessages() {
1553 - return self::$dataCache->getItem( $this->mCode, 'messages' );
 1553+ return self::$dataCache->getItem( $this->getCodeForMessage(), 'messages' );
15541554 }
15551555
15561556 function iconv( $in, $out, $string ) {
@@ -2764,6 +2764,18 @@
27652765 function getCode() {
27662766 return $this->mCode;
27672767 }
 2768+
 2769+ /**
 2770+ * Get langcode for message
 2771+ * Some language, like Chinese (zh, without any suffix), has multiple
 2772+ * interface languages, we could choose a better one for user.
 2773+ * Inherit class can override this function if necessary.
 2774+ *
 2775+ * @return string
 2776+ */
 2777+ function getCodeForMessage() {
 2778+ return $this->getPreferredVariant();
 2779+ }
27682780
27692781 function setCode( $code ) {
27702782 $this->mCode = $code;
Index: trunk/phase3/includes/MessageCache.php
@@ -550,7 +550,7 @@
551551 throw new MWException( "Bad lang code $langcode given" );
552552 }
553553
554 - $langcode = $lang->getCode();
 554+ $langcode = $lang->getCodeForMessage();
555555
556556 $message = false;
557557

Follow-up revisions

RevisionCommit summaryAuthorDate
r80492Follow up r77452. Follow mark's suggest to remove getCodeForMessage() and exp...philip07:59, 18 January 2011
r82246Follow r77452, r80492. Avoid to find sub-languages when load css/js messages.philip14:18, 16 February 2011
r82253revert r77452, r80492 and r82246.philip16:07, 16 February 2011

Comments

#Comment by Nikerabbit (talk | contribs)   06:40, 30 November 2010

I don't understand why you are putting the logic in the MessageCache instead of the language object construction. Does this handle database messages at all?

#Comment by PhiLiP (talk | contribs)   07:06, 30 November 2010

I just replaced getCode() to getCodeForMessage() for acquiring messages from MediaWiki namespace in the MessageCache method.

#Comment by Nikerabbit (talk | contribs)   07:39, 30 November 2010

But $lang is used on line 583. I think it's confusing when the same logic is split into different files.

#Comment by PhiLiP (talk | contribs)   10:16, 30 November 2010

Do you mean I should migrate the "Try the MediaWiki namespace" part to Language class? But it existed before I committed this reversion...

#Comment by MarkAHershberger (talk | contribs)   16:52, 12 January 2011

Why not replace getCode() with getPreferredVariant() instead of creating the wrapper getCodeForMessage(). The wrapper increases execution time slightly with no obvious benefit.

#Comment by PhiLiP (talk | contribs)   18:50, 12 January 2011

Variant for LanguageConverter, uselang for Interface. But I need to consider the both in order to get the best language for users and prevent confusions. That's the main reason why I decided to define and use a new method instead the existed one.

The other reason is, we couldn't know if there're languages which handled getPreferredVariant() and getCodeForMessage() differently, let's leave this option open.

#Comment by MarkAHershberger (talk | contribs)   19:09, 12 January 2011

Sorry, the first paragraph completely confuses me. So, instead of preventing confusion, you're introducing it.

Since you introduced getCodeForMessage(), you should have a distinct use case. I would prefer you not "leave this option open" unless you have an actual use case. The person the introducing the use case would have the chance to create an interface that is best suited to them.

#Comment by PhiLiP (talk | contribs)   19:30, 12 January 2011

Apologize for my unclear description. I mean I don't want people to be confused with getPreferredVariant() and getCode()/getCodeForMessage(). They are used in different way and should not be confusion.

I'll change getCodeForMessage() to getPreferredVariant() tomorrow. Somebody disallow me to program now.

#Comment by MarkAHershberger (talk | contribs)   19:46, 12 January 2011

I apologize for my tone, I was getting frustrated trying to understand this change and your explanation.

Since you don't want people to become confused with how you (will be) using getPreferredVariant(), I suggest you comment the code and explain why you're using getPreferredVariant() instead of getCode().

#Comment by PhiLiP (talk | contribs)   08:01, 18 January 2011

Sorry I remained it unfixed until today: r80492.

Status & tagging log