Merge "Parser: Minor code cleanup"

This commit is contained in:
jenkins-bot 2024-02-16 12:40:19 +00:00 committed by Gerrit Code Review
commit 85f75fb8ed
2 changed files with 24 additions and 25 deletions

View file

@ -110,18 +110,18 @@ class CommentParser {
* that is a "void element" or "text element", except some special cases that we treat as comment * that is a "void element" or "text element", except some special cases that we treat as comment
* separators (isCommentSeparator()). * separators (isCommentSeparator()).
* *
* @param Node $node Node to start searching at. This node's children are ignored. * @param ?Node $node Node after which to start searching
* (if null, start at the beginning of the document).
* @return Node * @return Node
*/ */
private function nextInterestingLeafNode( Node $node ): Node { private function nextInterestingLeafNode( ?Node $node ): Node {
$rootNode = $this->rootNode; $rootNode = $this->rootNode;
$treeWalker = new TreeWalker( $treeWalker = new TreeWalker(
$rootNode, $rootNode,
NodeFilter::SHOW_ELEMENT | NodeFilter::SHOW_TEXT, NodeFilter::SHOW_ELEMENT | NodeFilter::SHOW_TEXT,
static function ( $n ) use ( $node, $rootNode ) { static function ( $n ) use ( $node, $rootNode ) {
// Ignore this node and its descendants // Skip past the starting node and its descendants
// (unless it's the root node, this is a special case for "fakeHeading" handling) if ( $n === $node || $n->parentNode === $node ) {
if ( $node !== $rootNode && ( $n === $node || $n->parentNode === $node ) ) {
return NodeFilter::FILTER_REJECT; return NodeFilter::FILTER_REJECT;
} }
// Ignore some elements usually used as separators or headers (and their descendants) // Ignore some elements usually used as separators or headers (and their descendants)
@ -138,7 +138,9 @@ class CommentParser {
return NodeFilter::FILTER_SKIP; return NodeFilter::FILTER_SKIP;
} }
); );
$treeWalker->currentNode = $node; if ( $node ) {
$treeWalker->currentNode = $node;
}
$treeWalker->nextNode(); $treeWalker->nextNode();
if ( !$treeWalker->currentNode ) { if ( !$treeWalker->currentNode ) {
throw new RuntimeException( 'nextInterestingLeafNode not found' ); throw new RuntimeException( 'nextInterestingLeafNode not found' );
@ -899,14 +901,13 @@ class CommentParser {
$timestampRegexps = $this->getLocalTimestampRegexps(); $timestampRegexps = $this->getLocalTimestampRegexps();
$dfParsers = $this->getLocalTimestampParsers(); $dfParsers = $this->getLocalTimestampParsers();
$curCommentEnd = $this->rootNode; $curCommentEnd = null;
$treeWalker = new TreeWalker( $treeWalker = new TreeWalker(
$this->rootNode, $this->rootNode,
NodeFilter::SHOW_ELEMENT | NodeFilter::SHOW_TEXT, NodeFilter::SHOW_ELEMENT | NodeFilter::SHOW_TEXT,
[ static::class, 'acceptOnlyNodesAllowingComments' ] [ static::class, 'acceptOnlyNodesAllowingComments' ]
); );
$lastSigNode = null;
while ( $node = $treeWalker->nextNode() ) { while ( $node = $treeWalker->nextNode() ) {
if ( $node instanceof Element && preg_match( '/^h([1-6])$/i', $node->tagName, $match ) ) { if ( $node instanceof Element && preg_match( '/^h([1-6])$/i', $node->tagName, $match ) ) {
$headingNodeAndOffset = CommentUtils::getHeadlineNodeAndOffset( $node ); $headingNodeAndOffset = CommentUtils::getHeadlineNodeAndOffset( $node );
@ -922,10 +923,8 @@ class CommentParser {
$curCommentEnd = $node; $curCommentEnd = $node;
} elseif ( $node instanceof Text && ( $match = $this->findTimestamp( $node, $timestampRegexps ) ) ) { } elseif ( $node instanceof Text && ( $match = $this->findTimestamp( $node, $timestampRegexps ) ) ) {
$warnings = []; $warnings = [];
$foundSignature = $this->findSignature( $node, $foundSignature = $this->findSignature( $node, $curCommentEnd );
$curCommentEnd === $this->rootNode ? null : $curCommentEnd );
$author = $foundSignature['username']; $author = $foundSignature['username'];
$lastSigNode = $foundSignature['nodes'][0];
if ( !$author ) { if ( !$author ) {
// Ignore timestamps for which we couldn't find a signature. It's probably not a real // Ignore timestamps for which we couldn't find a signature. It's probably not a real
@ -941,7 +940,7 @@ class CommentParser {
// Everything from the last comment up to here is the next comment // Everything from the last comment up to here is the next comment
$startNode = $this->nextInterestingLeafNode( $curCommentEnd ); $startNode = $this->nextInterestingLeafNode( $curCommentEnd );
$endNode = $lastSigNode; $endNode = $foundSignature['nodes'][0];
// Skip to the end of the "paragraph". This only looks at tag names and can be fooled by CSS, but // Skip to the end of the "paragraph". This only looks at tag names and can be fooled by CSS, but
// avoiding that would be more difficult and slower. // avoiding that would be more difficult and slower.
@ -954,7 +953,7 @@ class CommentParser {
// no way to indicate which one you're replying to (this might matter in the future for // no way to indicate which one you're replying to (this might matter in the future for
// notifications or something). // notifications or something).
CommentUtils::linearWalk( CommentUtils::linearWalk(
$lastSigNode, $endNode,
function ( string $event, Node $n ) use ( function ( string $event, Node $n ) use (
&$endNode, &$sigRanges, &$timestampRanges, &$endNode, &$sigRanges, &$timestampRanges,
$treeWalker, $timestampRegexps, $node $treeWalker, $timestampRegexps, $node

View file

@ -793,7 +793,8 @@ Parser.prototype.findSignature = function ( timestampNode, until ) {
* separators (isCommentSeparator()). * separators (isCommentSeparator()).
* *
* @private * @private
* @param {Node} node Node to start searching at. If it isn't a leaf node, its children are ignored. * @param {Node|null} node Node after which to start searching
* (if null, start at the beginning of the document).
* @return {Node} * @return {Node}
*/ */
Parser.prototype.nextInterestingLeafNode = function ( node ) { Parser.prototype.nextInterestingLeafNode = function ( node ) {
@ -804,9 +805,8 @@ Parser.prototype.nextInterestingLeafNode = function ( node ) {
// eslint-disable-next-line no-bitwise // eslint-disable-next-line no-bitwise
NodeFilter.SHOW_ELEMENT | NodeFilter.SHOW_TEXT, NodeFilter.SHOW_ELEMENT | NodeFilter.SHOW_TEXT,
function ( n ) { function ( n ) {
// Ignore this node and its descendants // Skip past the starting node and its descendants
// (unless it's the root node, this is a special case for "fakeHeading" handling) if ( n === node || n.parentNode === node ) {
if ( node !== rootNode && ( n === node || n.parentNode === node ) ) {
return NodeFilter.FILTER_REJECT; return NodeFilter.FILTER_REJECT;
} }
// Ignore some elements usually used as separators or headers (and their descendants) // Ignore some elements usually used as separators or headers (and their descendants)
@ -824,7 +824,9 @@ Parser.prototype.nextInterestingLeafNode = function ( node ) {
}, },
false false
); );
treeWalker.currentNode = node; if ( node ) {
treeWalker.currentNode = node;
}
treeWalker.nextNode(); treeWalker.nextNode();
if ( !treeWalker.currentNode ) { if ( !treeWalker.currentNode ) {
throw new Error( 'nextInterestingLeafNode not found' ); throw new Error( 'nextInterestingLeafNode not found' );
@ -874,9 +876,9 @@ Parser.prototype.buildThreadItems = function () {
); );
var curComment, range; var curComment, range;
var curCommentEnd = this.rootNode; var curCommentEnd = null;
var node, lastSigNode; var node;
while ( ( node = treeWalker.nextNode() ) ) { while ( ( node = treeWalker.nextNode() ) ) {
var match; var match;
if ( node.tagName && ( match = node.tagName.match( /^h([1-6])$/i ) ) ) { if ( node.tagName && ( match = node.tagName.match( /^h([1-6])$/i ) ) ) {
@ -895,10 +897,8 @@ Parser.prototype.buildThreadItems = function () {
curCommentEnd = node; curCommentEnd = node;
} else if ( node.nodeType === Node.TEXT_NODE && ( match = this.findTimestamp( node, timestampRegexps ) ) ) { } else if ( node.nodeType === Node.TEXT_NODE && ( match = this.findTimestamp( node, timestampRegexps ) ) ) {
var warnings = []; var warnings = [];
var foundSignature = this.findSignature( node, var foundSignature = this.findSignature( node, curCommentEnd );
curCommentEnd === this.rootNode ? null : curCommentEnd );
var author = foundSignature.username; var author = foundSignature.username;
lastSigNode = foundSignature.nodes[ 0 ];
if ( !author ) { if ( !author ) {
// Ignore timestamps for which we couldn't find a signature. It's probably not a real // Ignore timestamps for which we couldn't find a signature. It's probably not a real
@ -914,7 +914,7 @@ Parser.prototype.buildThreadItems = function () {
// Everything from the last comment up to here is the next comment // Everything from the last comment up to here is the next comment
var startNode = this.nextInterestingLeafNode( curCommentEnd ); var startNode = this.nextInterestingLeafNode( curCommentEnd );
var endNode = lastSigNode; var endNode = foundSignature.nodes[ 0 ];
// Skip to the end of the "paragraph". This only looks at tag names and can be fooled by CSS, but // Skip to the end of the "paragraph". This only looks at tag names and can be fooled by CSS, but
// avoiding that would be more difficult and slower. // avoiding that would be more difficult and slower.
@ -927,7 +927,7 @@ Parser.prototype.buildThreadItems = function () {
// no way to indicate which one you're replying to (this might matter in the future for // no way to indicate which one you're replying to (this might matter in the future for
// notifications or something). // notifications or something).
utils.linearWalk( utils.linearWalk(
lastSigNode, endNode,
// eslint-disable-next-line no-loop-func // eslint-disable-next-line no-loop-func
function ( event, n ) { function ( event, n ) {
var match2, foundSignature2; var match2, foundSignature2;