Tim Starling has uploaded a new change for review.
https://gerrit.wikimedia.org/r/287170
Change subject: BlockLevelPass: minor changes due to initial code review
......................................................................
BlockLevelPass: minor changes due to initial code review
* Improve some comments
* In getCommon() use min() to improve readability
* Use strict equals where possible
* Use camel case in variable names
* Remove the "case 0" optimisation, made sense in PHP 4 but those are
compile-time constants now and are presumably treated as such in both
PHP and HHVM.
* oLine -> inputLine, no idea what "o" stood for
* pos -> colonPos, lt -> ltPos: descriptive
* stack -> level, paragraphStack -> pendingPTag: they're not stacks
* Remove "m" prefix from member variable names
Change-Id: I6c1144c792ba3e1935be88a009a6d6c110d11090
---
M includes/parser/BlockLevelPass.php
1 file changed, 109 insertions(+), 107 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/70/287170/1
diff --git a/includes/parser/BlockLevelPass.php
b/includes/parser/BlockLevelPass.php
index 7c6d330..cbacd34 100644
--- a/includes/parser/BlockLevelPass.php
+++ b/includes/parser/BlockLevelPass.php
@@ -23,9 +23,9 @@
* @ingroup Parser
*/
class BlockLevelPass {
- private $mDTopen = false;
- private $mInPre = false;
- private $mLastSection = '';
+ private $DTopen = false;
+ private $inPre = false;
+ private $lastSection = '';
private $linestart;
private $text;
@@ -43,29 +43,34 @@
* Make lists from lines starting with ':', '*', '#', etc.
*
* @param string $text
- * @param bool $linestart Whether or not this is at the start of a line.
+ * @param bool $lineStart Whether or not this is at the start of a line.
* @return string The lists rendered as HTML
*/
- public static function doBlockLevels( $text, $linestart ) {
- $pass = new self( $text, $linestart );
+ public static function doBlockLevels( $text, $lineStart ) {
+ $pass = new self( $text, $lineStart );
return $pass->execute();
}
- private function __construct( $text, $linestart ) {
+ /**
+ * Private constructor
+ */
+ private function __construct( $text, $lineStart ) {
$this->text = $text;
- $this->linestart = $linestart;
+ $this->lineStart = $lineStart;
}
/**
+ * If a pre or p is open, return the corresponding close tag and update
+ * the state. If no tag is open, return an empty string.
* @return string
*/
private function closeParagraph() {
$result = '';
- if ( $this->mLastSection != '' ) {
- $result = '</' . $this->mLastSection . ">\n";
+ if ( $this->lastSection !== '' ) {
+ $result = '</' . $this->lastSection . ">\n";
}
- $this->mInPre = false;
- $this->mLastSection = '';
+ $this->inPre = false;
+ $this->lastSection = '';
return $result;
}
@@ -79,14 +84,10 @@
* @return int
*/
private function getCommon( $st1, $st2 ) {
- $fl = strlen( $st1 );
- $shorter = strlen( $st2 );
- if ( $fl < $shorter ) {
- $shorter = $fl;
- }
+ $shorter = min( strlen( $st1 ), strlen( $st2 ) );
for ( $i = 0; $i < $shorter; ++$i ) {
- if ( $st1[$i] != $st2[$i] ) {
+ if ( $st1[$i] !== $st2[$i] ) {
break;
}
}
@@ -94,8 +95,7 @@
}
/**
- * These next three functions open, continue, and close the list
- * element appropriate to the prefix character passed into them.
+ * Open the list item element identified by the prefix character.
*
* @param string $char
*
@@ -112,7 +112,7 @@
$result .= "<dl><dd>";
} elseif ( ';' === $char ) {
$result .= "<dl><dt>";
- $this->mDTopen = true;
+ $this->DTopen = true;
} else {
$result = '<!-- ERR 1 -->';
}
@@ -121,7 +121,7 @@
}
/**
- * TODO: document
+ * Close the current list item and open the next one.
* @param string $char
*
* @return string
@@ -131,14 +131,14 @@
return "</li>\n<li>";
} elseif ( ':' === $char || ';' === $char ) {
$close = "</dd>\n";
- if ( $this->mDTopen ) {
+ if ( $this->DTopen ) {
$close = "</dt>\n";
}
if ( ';' === $char ) {
- $this->mDTopen = true;
+ $this->DTopen = true;
return $close . '<dt>';
} else {
- $this->mDTopen = false;
+ $this->DTopen = false;
return $close . '<dd>';
}
}
@@ -146,7 +146,7 @@
}
/**
- * @todo Document
+ * Close the current list item identified by the prefix character.
* @param string $char
*
* @return string
@@ -157,8 +157,8 @@
} elseif ( '#' === $char ) {
$text = "</li></ol>";
} elseif ( ':' === $char ) {
- if ( $this->mDTopen ) {
- $this->mDTopen = false;
+ if ( $this->DTopen ) {
+ $this->DTopen = false;
$text = "</dt></dl>";
} else {
$text = "</dd></dl>";
@@ -168,8 +168,11 @@
}
return $text;
}
- /**#@-*/
+ /**
+ * Execute the pass.
+ * @return string
+ */
private function execute() {
$text = $this->text;
# Parsing through the text line by line. The main thing
@@ -178,16 +181,16 @@
$textLines = StringUtils::explode( "\n", $text );
$lastPrefix = $output = '';
- $this->mDTopen = $inBlockElem = false;
+ $this->DTopen = $inBlockElem = false;
$prefixLength = 0;
- $paragraphStack = false;
+ $pendingPTag = false;
$inBlockquote = false;
- foreach ( $textLines as $oLine ) {
- # Fix up $linestart
- if ( !$this->linestart ) {
- $output .= $oLine;
- $this->linestart = true;
+ foreach ( $textLines as $inputLine ) {
+ # Fix up $lineStart
+ if ( !$this->lineStart ) {
+ $output .= $inputLine;
+ $this->lineStart = true;
continue;
}
# * = ul
@@ -196,33 +199,33 @@
# : = dd
$lastPrefixLength = strlen( $lastPrefix );
- $preCloseMatch = preg_match( '/<\\/pre/i', $oLine );
- $preOpenMatch = preg_match( '/<pre/i', $oLine );
+ $preCloseMatch = preg_match( '/<\\/pre/i', $inputLine );
+ $preOpenMatch = preg_match( '/<pre/i', $inputLine );
# If not in a <pre> element, scan for and figure out
what prefixes are there.
- if ( !$this->mInPre ) {
+ if ( !$this->inPre ) {
# Multiple prefixes may abut each other for
nested lists.
- $prefixLength = strspn( $oLine, '*#:;' );
- $prefix = substr( $oLine, 0, $prefixLength );
+ $prefixLength = strspn( $inputLine, '*#:;' );
+ $prefix = substr( $inputLine, 0, $prefixLength
);
# eh?
# ; and : are both from definition-lists, so
they're equivalent
# for the purposes of determining whether or
not we need to open/close
# elements.
$prefix2 = str_replace( ';', ':', $prefix );
- $t = substr( $oLine, $prefixLength );
- $this->mInPre = (bool)$preOpenMatch;
+ $t = substr( $inputLine, $prefixLength );
+ $this->inPre = (bool)$preOpenMatch;
} else {
# Don't interpret any other prefixes in
preformatted text
$prefixLength = 0;
$prefix = $prefix2 = '';
- $t = $oLine;
+ $t = $inputLine;
}
# List generation
if ( $prefixLength && $lastPrefix === $prefix2 ) {
# Same as the last item, so no need to deal
with nesting or opening stuff
$output .= $this->nextItem( substr( $prefix, -1
) );
- $paragraphStack = false;
+ $pendingPTag = false;
if ( substr( $prefix, -1 ) === ';' ) {
# The one nasty exception: definition
lists work like this:
@@ -240,7 +243,7 @@
# Either open or close a level...
$commonPrefixLength = $this->getCommon(
$prefix, $lastPrefix );
- $paragraphStack = false;
+ $pendingPTag = false;
# Close all the prefixes which aren't shared.
while ( $commonPrefixLength < $lastPrefixLength
) {
@@ -279,13 +282,13 @@
# If we have no prefixes, go to paragraph mode.
if ( 0 == $prefixLength ) {
# No prefix (not in list)--go to paragraph mode
- # XXX: use a stack for nestable elements like
span, table and div
- $openmatch = preg_match(
+ # @todo consider using a stack for nestable
elements like span, table and div
+ $openMatch = preg_match(
'/(?:<table|<h1|<h2|<h3|<h4|<h5|<h6|<pre|<tr|'
.
'<p|<ul|<ol|<dl|<li|<\\/tr|<\\/td|<\\/th)/iS',
$t
);
- $closematch = preg_match(
+ $closeMatch = preg_match(
'/(?:<\\/table|<\\/h1|<\\/h2|<\\/h3|<\\/h4|<\\/h5|<\\/h6|'
.
'<td|<th|<\\/?blockquote|<\\/?div|<hr|<\\/pre|<\\/p|<\\/mw:|'
. Parser::MARKER_PREFIX
@@ -293,12 +296,12 @@
$t
);
- if ( $openmatch || $closematch ) {
- $paragraphStack = false;
+ if ( $openMatch || $closeMatch ) {
+ $pendingPTag = false;
# @todo bug 5718: paragraph closed
$output .= $this->closeParagraph();
if ( $preOpenMatch && !$preCloseMatch )
{
- $this->mInPre = true;
+ $this->inPre = true;
}
$bqOffset = 0;
while ( preg_match(
'/<(\\/?)blockquote[\s>]/i', $t,
@@ -307,53 +310,53 @@
$inBlockquote =
!$bqMatch[1][0]; // is this a close tag?
$bqOffset = $bqMatch[0][1] +
strlen( $bqMatch[0][0] );
}
- $inBlockElem = !$closematch;
- } elseif ( !$inBlockElem && !$this->mInPre ) {
+ $inBlockElem = !$closeMatch;
+ } elseif ( !$inBlockElem && !$this->inPre ) {
if ( ' ' == substr( $t, 0, 1 )
- && ( $this->mLastSection ===
'pre' || trim( $t ) != '' )
+ && ( $this->lastSection ===
'pre' || trim( $t ) != '' )
&& !$inBlockquote
) {
# pre
- if ( $this->mLastSection !==
'pre' ) {
- $paragraphStack = false;
+ if ( $this->lastSection !==
'pre' ) {
+ $pendingPTag = false;
$output .=
$this->closeParagraph() . '<pre>';
- $this->mLastSection =
'pre';
+ $this->lastSection =
'pre';
}
$t = substr( $t, 1 );
} else {
# paragraph
if ( trim( $t ) === '' ) {
- if ( $paragraphStack ) {
- $output .=
$paragraphStack . '<br />';
- $paragraphStack
= false;
-
$this->mLastSection = 'p';
+ if ( $pendingPTag ) {
+ $output .=
$pendingPTag . '<br />';
+ $pendingPTag =
false;
+
$this->lastSection = 'p';
} else {
- if (
$this->mLastSection !== 'p' ) {
+ if (
$this->lastSection !== 'p' ) {
$output
.= $this->closeParagraph();
-
$this->mLastSection = '';
-
$paragraphStack = '<p>';
+
$this->lastSection = '';
+
$pendingPTag = '<p>';
} else {
-
$paragraphStack = '</p><p>';
+
$pendingPTag = '</p><p>';
}
}
} else {
- if ( $paragraphStack ) {
- $output .=
$paragraphStack;
- $paragraphStack
= false;
-
$this->mLastSection = 'p';
- } elseif (
$this->mLastSection !== 'p' ) {
+ if ( $pendingPTag ) {
+ $output .=
$pendingPTag;
+ $pendingPTag =
false;
+
$this->lastSection = 'p';
+ } elseif (
$this->lastSection !== 'p' ) {
$output .=
$this->closeParagraph() . '<p>';
-
$this->mLastSection = 'p';
+
$this->lastSection = 'p';
}
}
}
}
}
# somewhere above we forget to get out of pre block
(bug 785)
- if ( $preCloseMatch && $this->mInPre ) {
- $this->mInPre = false;
+ if ( $preCloseMatch && $this->inPre ) {
+ $this->inPre = false;
}
- if ( $paragraphStack === false ) {
+ if ( $pendingPTag === false ) {
$output .= $t;
if ( $prefixLength === 0 ) {
$output .= "\n";
@@ -367,9 +370,9 @@
$output .= "\n";
}
}
- if ( $this->mLastSection != '' ) {
- $output .= '</' . $this->mLastSection . '>';
- $this->mLastSection = '';
+ if ( $this->lastSection !== '' ) {
+ $output .= '</' . $this->lastSection . '>';
+ $this->lastSection = '';
}
return $output;
@@ -386,37 +389,36 @@
* @return string The position of the ':', or false if none found
*/
private function findColonNoLinks( $str, &$before, &$after ) {
- $pos = strpos( $str, ':' );
- if ( $pos === false ) {
+ $colonPos = strpos( $str, ':' );
+ if ( $colonPos === false ) {
# Nothing to find!
return false;
}
- $lt = strpos( $str, '<' );
- if ( $lt === false || $lt > $pos ) {
+ $ltPos = strpos( $str, '<' );
+ if ( $ltPos === false || $ltPos > $colonPos ) {
# Easy; no tag nesting to worry about
- $before = substr( $str, 0, $pos );
- $after = substr( $str, $pos + 1 );
- return $pos;
+ $before = substr( $str, 0, $colonPos );
+ $after = substr( $str, $colonPos + 1 );
+ return $colonPos;
}
# Ugly state machine to walk through avoiding tags.
$state = self::COLON_STATE_TEXT;
- $stack = 0;
+ $level = 0;
$len = strlen( $str );
for ( $i = 0; $i < $len; $i++ ) {
$c = $str[$i];
switch ( $state ) {
- # (Using the number is a performance hack for common
cases)
- case 0: # self::COLON_STATE_TEXT:
+ case self::COLON_STATE_TEXT:
switch ( $c ) {
case "<":
# Could be either a <start> tag or an
</end> tag
$state = self::COLON_STATE_TAGSTART;
break;
case ":":
- if ( $stack == 0 ) {
+ if ( $level === 0 ) {
# We found it!
$before = substr( $str, 0, $i );
$after = substr( $str, $i + 1 );
@@ -426,35 +428,35 @@
break;
default:
# Skip ahead looking for something
interesting
- $colon = strpos( $str, ':', $i );
- if ( $colon === false ) {
+ $colonPos = strpos( $str, ':', $i );
+ if ( $colonPos === false ) {
# Nothing else interesting
return false;
}
- $lt = strpos( $str, '<', $i );
- if ( $stack === 0 ) {
- if ( $lt === false || $colon <
$lt ) {
+ $ltPos = strpos( $str, '<', $i );
+ if ( $level === 0 ) {
+ if ( $ltPos === false ||
$colonPos < $ltPos ) {
# We found it!
- $before = substr( $str,
0, $colon );
- $after = substr( $str,
$colon + 1 );
+ $before = substr( $str,
0, $colonPos );
+ $after = substr( $str,
$colonPos + 1 );
return $i;
}
}
- if ( $lt === false ) {
+ if ( $ltPos === false ) {
# Nothing else interesting to
find; abort!
# We're nested, but there's no
close tags left. Abort!
break 2;
}
# Skip ahead to next tag start
- $i = $lt;
+ $i = $ltPos;
$state = self::COLON_STATE_TAGSTART;
}
break;
- case 1: # self::COLON_STATE_TAG:
+ case self::COLON_STATE_TAG:
# In a <tag>
switch ( $c ) {
case ">":
- $stack++;
+ $level++;
$state = self::COLON_STATE_TEXT;
break;
case "/":
@@ -465,7 +467,7 @@
# ignore
}
break;
- case 2: # self::COLON_STATE_TAGSTART:
+ case self::COLON_STATE_TAGSTART:
switch ( $c ) {
case "/":
$state = self::COLON_STATE_CLOSETAG;
@@ -481,11 +483,11 @@
$state = self::COLON_STATE_TAG;
}
break;
- case 3: # self::COLON_STATE_CLOSETAG:
+ case self::COLON_STATE_CLOSETAG:
# In a </tag>
if ( $c === ">" ) {
- $stack--;
- if ( $stack < 0 ) {
+ $level--;
+ if ( $level < 0 ) {
wfDebug( __METHOD__ . ":
Invalid input; too many close tags\n" );
return false;
}
@@ -501,7 +503,7 @@
$state = self::COLON_STATE_TAG;
}
break;
- case 5: # self::COLON_STATE_COMMENT:
+ case self::COLON_STATE_COMMENT:
if ( $c === "-" ) {
$state = self::COLON_STATE_COMMENTDASH;
}
@@ -524,8 +526,8 @@
throw new MWException( "State machine error in
" . __METHOD__ );
}
}
- if ( $stack > 0 ) {
- wfDebug( __METHOD__ . ": Invalid input; not enough
close tags (stack $stack, state $state)\n" );
+ if ( $level > 0 ) {
+ wfDebug( __METHOD__ . ": Invalid input; not enough
close tags (level $level, state $state)\n" );
return false;
}
return false;
--
To view, visit https://gerrit.wikimedia.org/r/287170
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6c1144c792ba3e1935be88a009a6d6c110d11090
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Tim Starling <tstarl...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
