Page MenuHomePhabricator

DateFormatter::reformat called unnecessarily
Closed, InvalidPublic

Description

Author: wclark

Description:
In Parser::internalParse, a call is made to DateFormatter::reformat even when the user's preference is set to 'default' which means that date formats should remain unchanged. This can be confirmed by noticing that (although the date format remains unchanged) a comma is inserted into linked dates that are in MDY format.

For example, the following wikitext:

[[March 30]] [[2007]]

Will be rendered in the page as:

[[March 30]], [[2007]]

While it's true that the comma is supposed to be there anyway, there's no reason to use the reformat method to add it (it should just be added to the wikitext directly.. and the actions of DateFormatter::reformat actually hide this minor typo from casual readers, making it harder to notice and fix.)

In line 889 of Parser.php, this line:

if( $this->mOptions->getUseDynamicDates() ) {

Should be changed to this:

if( $this->mOptions->getUseDynamicDates() and $this-mOptions->getDateFormat() != 'default') {

That will prevent DateFormatter::reformat from being called unnecessarily and both wasting CPU cycles (by checking against all the various date format regexes when no actual date transformations will be applied) and hiding a minor grammatical error that should be fixed directly.


Version: 1.15.x
Severity: minor

Details

Reference
bz18376

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:31 PM
bzimport added a project: MediaWiki-Parser.
bzimport set Reference to bz18376.
bzimport added a subscriber: Unknown Object (MLST).

wclark wrote:

Well actually it should be changed to this:

if( $this->mOptions->getUseDynamicDates() and $this->mOptions->getDateFormat() != 'default' ) {

..but who wants syntactically correct code?

wclark wrote:

This bug is now irrelevant because DynamicDates is going to be disabled (on en-wiki at least) and there are problems (listed elsewhere) with the approach, anyway.