r86927 MediaWiki - Code Review archive

Is this the way we are going, adding language parameter to each function individually?

I'm not sure if this is the best solution or not. Some admins on Commons requested this addition to #time specifically, so I went ahead and added it. You probably have a better idea of what would make sense for the long term, however. Let me know if you have thoughts on how to implement this functionality in a better way.

There isn't better way, at least not now. I know there are many magic words that would be useful with language parameter, so I've been dreaming of some sort of general solution to solve it all at once. It may just be an utopia though.

Crazy idea: a parser function to force output language, wrap it around anything?

{{#uselang:de|Die Uhr ist jetzt {{#time}}}}

(May not be a good idea. May cause your grandmother to explode. May eat your dog's lunch and replace it with tofu.)

I tried this idea in my working copy, and my grandmother spontaneously combusted. It's all your fault!

Could that be modified to apply to mothers-in-law instead of grandmothers?

We should make it configurable. Perhaps a $wgCombustableFamilyMembers array?

That wouldn't cover the common use case of combusting the sysadmin if mediawiki is outdated (to be run from cron).

Just occurred to me that this is not as useful as it seems unless also the predefined date formats for each language are accessible.

Can you elaborate?

Generally speaking, you shouldn't rearrange method parameters like you did here:

-	public static function time( $parser, $format = '', $date = '', $local = false ) {
+	public static function time( $parser, $format = '', $date = '', $language = '', $local = false ) {

Just add language to the end.

I would have except that this is a strange case. The 'local' param is only used by another function (localTime/#timel) and not by the parser function per se ({{#time)). Since 'language' should actually be used in the parser function call, but not 'local', I have to put 'language' first.

In other words, this is a supported call to #time: 1945 but never: 1945 (even though it was technically possible), as this is covered by #timel

You may want to refer to http://www.mediawiki.org/wiki/Help:Extension:ParserFunctions#.23time for more info.

Oops, all my examples got parsed. Must use preview button...

This is a supported call:
{{#time:F Y|December 1945|fr}}
but never:
{{#time:F Y|December 1945|true}}

I'm not sure if my previous explanation makes any sense, so let me try it again...

Before this rev, the only params actually used by the parser function #time were $format and $date. The $local param was tacked on so that #timel/localtime() could just piggyback off the time function instead of having redundant code. The $local param was never intended for use by #time (since {{#time:Y|1945|true}} is the same as {{#timel:Y|1945}} and {{#time:Y|1945|false}} is the same as {{#time:Y|1945}}) and the parameter's presence is not reflected in the documentation for the parser function. Since I want the $language parameter to actually be used by #time, I put it at the end of the parser function parameters (but not the end of the underlying function's parameters). That way it can be used by #time without forcing people to start inserting a '|false' into the call before the language parameter. Since #timel/localtime() is the only thing that actually uses the $local param in time() (I checked), I only have to change the call there to make sure nothing breaks. Hope that makes sense.

It is much clear :D

Yes, thank you for clearing that up.

For the use user language option. Doesn't that need to communicate to the parser that the content varies by userlanguage so that the cache varies by user language? (aka by calling $parser->getOptions()->getUserLang() ). Otherwise its not so much the language of the current user but the language of the last user to cause a page render.

Ah that's probably true. Didn't think about that.

Hopefully this is fixed in r87305. I don't know much about how the language caching works, so let me know if this is wrong. Also, I'm wondering if this is really even a good idea. Will this cause too much cache fragmentation if people start using it widely? Thoughts?

Hey Bawolff, can you take a look at the follow-up diffs and let me know if anything else needs to be fixed? Thanks!

Try saving a page with {{#time:j F| +1000000000000 days}}. You'll get the following meltdown:

Notice: Undefined offset: 89 in D:\Projects\MediaWiki\languages\Language.php on line 608

Internal error
Non-string key given

Backtrace:

#0 D:\Projects\MediaWiki\includes\GlobalFunctions.php(694): MessageCache->get(NULL, true, Object(Language))
#1 D:\Projects\MediaWiki\includes\GlobalFunctions.php(818): wfMsgGetKey(NULL, true, Object(Language), false)
#2 D:\Projects\MediaWiki\languages\Language.php(596): wfMsgExt(NULL, Array)
#3 D:\Projects\MediaWiki\languages\Language.php(608): Language->getMessageFromDB(NULL)
#4 D:\Projects\MediaWiki\languages\Language.php(898): Language->getMonthName('90')
#5 D:\Projects\MediaWiki\extensions\ParserFunctions\ParserFunctions_body.php(513): Language->sprintfDate('j F', '273790901804271...')
#6 [internal function]: ExtParserFunctions::time(Object(Parser), 'j F', '+1000000000000 ...')
#7 D:\Projects\MediaWiki\includes\parser\Parser.php(3192): call_user_func_array('ExtParserFuncti...', Array)
#8 D:\Projects\MediaWiki\includes\parser\Preprocessor_DOM.php(986): Parser->braceSubstitution(Array, Object(PPFrame_DOM))
#9 D:\Projects\MediaWiki\includes\parser\Parser.php(2991): PPFrame_DOM->expand(Object(PPNode_DOM), 0)
#10 D:\Projects\MediaWiki\includes\parser\Parser.php(1153): Parser->replaceVariables('<big>'''MediaWi...')
#11 D:\Projects\MediaWiki\includes\parser\Parser.php(315): Parser->internalParse('<big>'''MediaWi...')
#12 D:\Projects\MediaWiki\includes\Article.php(4272): Parser->parse('<big>'''MediaWi...', Object(Title), Object(ParserOptions), true, true, 5567)
#13 D:\Projects\MediaWiki\includes\Article.php(4249): Article->getOutputFromWikitext('<big>'''MediaWi...', true, Object(ParserOptions))
#14 D:\Projects\MediaWiki\includes\Article.php(1542): Article->outputWikiText('<big>'''MediaWi...', true, Object(ParserOptions))
#15 D:\Projects\MediaWiki\includes\Article.php(4525): Article->doViewParse()
#16 D:\Projects\MediaWiki\includes\PoolCounter.php(163): PoolWorkArticleView->doWork()
#17 D:\Projects\MediaWiki\includes\Article.php(1065): PoolCounterWork->execute()
#18 D:\Projects\MediaWiki\includes\Wiki.php(486): Article->view()
#19 D:\Projects\MediaWiki\includes\Wiki.php(104): MediaWiki->performAction(Object(Article))
#20 D:\Projects\MediaWiki\index.php(145): MediaWiki->performRequestForTitle(Object(Article))
#21 {main}

Cute. The code excepts only four digit years everywhere.

Fixed in r87305 (only for #time, not for Language in general).

After talking with Platonides, I've redone the language handling. See r87308. The magic language switching is gone, but you can still accomplish the same thing by passing {{int:lang}} as the language param on Commons (the main place this is needed).

It doesn't work at translatewiki, see r87308.

int:lang doesn't exist on most wikis. I put in some defensive programming so that it fails gracefully in r92571 and r92572.