Last modified: 2010-05-15 15:38:31 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T3931, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 1931 - Parser is a little dirty.
Parser is a little dirty.
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
1.5.x
All All
: Normal normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-04-20 01:36 UTC by David Majnemer
Modified: 2010-05-15 15:38 UTC (History)
0 users

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Bugfix for the bugs! (5.18 KB, patch)
2005-04-20 02:03 UTC, David Majnemer
Details

Description David Majnemer 2005-04-20 01:36:37 UTC
Stepped thru the parsers code for some kicks and saw some stuff that was wrong.
Here it is for all to see:

$stripState = NULL;
global $fnord; $fnord = 1;
//$text = $this->strip( $text, $this->mStripState );
// VOODOO MAGIC FIX! Sometimes the above segfaults in PHP5.
$x =& $this->mStripState;
$text = $this->strip( $text, $x );

The $stripState var is not used, ever. I am positive this is supposed to be
$this->mStripState.

		foreach ( $this->mTagHooks as $tag => $callback ) {
			$ext_contents[$tag] = array();
			$text = Parser::extractTags( $tag, $text, $ext_content[$tag], $uniq_prefix );
			foreach( $ext_content[$tag] as $marker => $content ) {
				if ( $render ) {
					$ext_content[$tag][$marker] = $callback( $content );
				} else {
					$ext_content[$tag][$marker] = "<$tag>$content</$tag>";
				}
			}
		}
You spawn the $ext_contents[$tag] arrays, they should read $ext_content[$tag]

at Tidy, you should really really consider spawning $pipes = array(); before
calling proc_open so it is nicer to look at. You do nothing with proc_close,
consider just making a cold call instead of assigning it a variable.

doTableStuff could have $matches spawned before the regex op. Just makes it nice
to read.

internalParse never uses its second function variable.

replaceExternalLinks could have $m2 spawned before the regex just for prettyness.

replaceFreeExternalLinks could have the same done with $m and it's $m2.

replaceInternalLinks: the same stuff.

replaceInternalLinks: globals out $wgLang and $wgDisableLangConversion without
using them, you assign $redirect to MagicWord::get ( MAG_REDIRECT ) without
using $redirect afterwards.

It would be nice if doBlockLevels could instantate $term and $t2 before its call
to findColonNoLinks where those vals are filled, $lastLine is spawned and never
used.

replaceVariables never uses $wgScript and $wgArticlePath which it globalizes.

braceSubstitution could spawn the arrays that hold the matches for regexs before
the regex functions are called.

formatHeadings, same deal.

formatHeadings, globalizes $wgLinkHolders and $wgInterwikiLinkHolders without
using them.

formatHeadings, has $refer[$headlineCount] = $canonized_headline. I am nearly
positive that this should be $refers.

formatHeadings, $toclines, never used.

pstPass2 has $oldtzs, this should almost definatly be $oldtz.

pstPass2, please spawn the arrays that will hold regexs before the regex
function is called.

pstPass2, if I am wrong about $oldtzs being $oldtz, then $oldtz has no purpose
at all.

replaceLinkHolders, $current is never defined thus never set. Thus if ( !isset(
$current ) ) will always be true.

renderImageGallery, spawn arrays before regex.

This is all for now, feel free to disagree with any of this. :)
Comment 1 David Majnemer 2005-04-20 02:03:14 UTC
Created attachment 437 [details]
Bugfix for the bugs!

Here is a bugfix for most of the bugs. Have fun!
Comment 2 Ævar Arnfjörð Bjarmason 2005-04-21 06:32:35 UTC
Applied with some changes to REL1_4 and HEAD.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links