Format multiple autocomments in edit summaries

Before r39373, all autocomments in an edit summary were formatted. In
fixing a bug with page titles containing "/*" this was accidentally
broken.

To use a single preg_replace_callback call to replace multiple
autocomments, we need to make sure that the match of one autocomment
doesn't overlap the match of another, which means we can't have "(.*)"
before and after. But we do still need to detect whether there is
anything before or after. "(?=(.?))" and "(?<=(.?))" would do nicely,
except the latter isn't actually supported. "(?=(.))?" and "(?<=(.))?"
work too, but older versions of PCRE don't support that. They do,
however, support "(?:(?=(.)))?" and "(?:(?<=(.)))?", so that's what
we'll go with.

This change does change the values for $pre and $post passed to the
FormatAutocomments hook; extensions need to be updated to accept (and
not prepend/append) booleans for these parameters.

Bug: T18530
Bug: T70361
Change-Id: I36c3a9e548a4ef72f93974bb35f9add8c29e9287

diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25
index ce50eac..4a6ee94 100644 (file)
--- a/RELEASE-NOTES-1.25
+++ b/RELEASE-NOTES-1.25
@@ -113,6 +113,8 @@ production.
 ** Title::moveTo() was deprecated. Use the MovePage class instead.
 ** Title::isValidMoveOperation() broken down into MovePage::isValidMove()
    and MovePage::checkPermissions().
+* (T18530) Multiple autocomments are now formatted in an edit summary.
+* (T70361) Autocomments containing "/*" are parsed correctly.
 * The Special:WhatLinksHere page linked from 'Number of redirects to this page'
   on action=info about a file page does not list file links anymore.
 * (T78637) Search bar is not autofocused unless it is empty so that proper scrolling using arrow keys is possible.
@@ -284,6 +286,8 @@ changes to languages because of Bugzilla reports.
    However, this difference is unlikely to arise in practice.
 * (T67278) RFC, PMID, and ISBN "magic links" must be surrounded by non-word
   characters on both sides.
+* The FormatAutocomments hook will now receive $pre and $post as booleans,
+  rather than as strings that must be prepended or appended to $comment.
 * (T30950, T31025) RFC, PMID, and ISBN "magic links" can no longer contain
   newlines; but they can contain   and other non-newline whitespace.
 

diff --git a/docs/hooks.txt b/docs/hooks.txt
index 8d024d6..b48067b 100644 (file)
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -1253,9 +1253,9 @@ $reason: reason
 'FormatAutocomments': When an autocomment is formatted by the Linker.
  &$comment: Reference to the accumulated comment. Initially null, when set the
    default code will be skipped.
- $pre: Initial part of the parsed comment before the call to the hook.
+ $pre: Boolean, true if there is text before this autocomment
  $auto: The extracted part of the parsed comment before the call to the hook.
- $post: The final part of the parsed comment before the call to the hook.
+ $post: Boolean, true if there is text after this autocomment
  $title: An optional title object used to links to sections. Can be null.
  $local: Boolean indicating whether section links should refer to local page.
 

diff --git a/includes/Linker.php b/includes/Linker.php
index ac4bb99..2bc36b1 100644 (file)
--- a/includes/Linker.php
+++ b/includes/Linker.php
@@ -1312,6 +1312,7 @@ class Linker {
 
        /**
         * Converts autogenerated comments in edit summaries into section links.
+        *
         * The pattern for autogen comments is / * foo * /, which makes for
         * some nasty regex.
         * We look for all comments, match any text before and after the comment,
@@ -1324,14 +1325,28 @@ class Linker {
         * @return string Formatted comment
         */
        private static function formatAutocomments( $comment, $title = null, $local = false ) {
-               return preg_replace_callback(
-                       '!(.*)/\*\s*(.*?)\s*\*/(.*)!',
-                       function ( $match ) use ( $title, $local ) {
+               // @todo $append here is something of a hack to preserve the status
+               // quo. Someone who knows more about bidi and such should decide
+               // (1) what sane rendering even *is* for an LTR edit summary on an RTL
+               // wiki, both when autocomments exist and when they don't, and
+               // (2) what markup will make that actually happen.
+               $append = '';
+               $comment = preg_replace_callback(
+                       // To detect the presence of content before or after the
+                       // auto-comment, we use capturing groups inside optional zero-width
+                       // assertions. But older versions of PCRE can't directly make
+                       // zero-width assertions optional, so wrap them in a non-capturing
+                       // group.
+                       '!(?:(?<=(.)))?/\*\s*(.*?)\s*\*/(?:(?=(.)))?!',
+                       function ( $match ) use ( $title, $local, &$append ) {
                                global $wgLang;
 
-                               $pre = $match[1];
+                               // Ensure all match positions are defined
+                               $match += array( '', '', '', '' );
+
+                               $pre = $match[1] !== '';
                                $auto = $match[2];
-                               $post = $match[3];
+                               $post = $match[3] !== '';
                                $comment = null;
                                Hooks::run( 'FormatAutocomments', array( &$comment, $pre, $auto, $post, $title, $local ) );
                                if ( $comment === null ) {
@@ -1361,7 +1376,7 @@ class Linker {
                                        }
                                        if ( $pre ) {
                                                # written summary $presep autocomment (summary /* section */)
-                                               $pre .= wfMessage( 'autocomment-prefix' )->inContentLanguage()->escaped();
+                                               $pre = wfMessage( 'autocomment-prefix' )->inContentLanguage()->escaped();
                                        }
                                        if ( $post ) {
                                                # autocomment $postsep written summary (/* section */ summary)
@@ -1369,12 +1384,14 @@ class Linker {
                                        }
                                        $auto = '<span class="autocomment">' . $auto . '</span>';
                                        $comment = $pre . $link . $wgLang->getDirMark()
-                                               . '<span dir="auto">' . $auto . $post . '</span>';
+                                               . '<span dir="auto">' . $auto;
+                                       $append .= '</span>';
                                }
                                return $comment;
                        },
                        $comment
                );
+               return $comment . $append;
        }
 
        /**

diff --git a/tests/phpunit/includes/LinkerTest.php b/tests/phpunit/includes/LinkerTest.php
index 7b84107..fc88a55 100644 (file)
--- a/tests/phpunit/includes/LinkerTest.php
+++ b/tests/phpunit/includes/LinkerTest.php
@@ -149,9 +149,13 @@ class LinkerTest extends MediaWikiLangTestCase {
                                "pre /* autocomment */ post",
                        ),
                        array(
-                               '/* autocomment */ multiple? <a href="/wiki/Special:BlankPage#autocomment2" title="Special:BlankPage">→</a>‎<span dir="auto"><span class="autocomment">autocomment2: </span> </span>',
+                               '<a href="/wiki/Special:BlankPage#autocomment" title="Special:BlankPage">→</a>‎<span dir="auto"><span class="autocomment">autocomment: </span> multiple? <a href="/wiki/Special:BlankPage#autocomment2" title="Special:BlankPage">→</a>‎<span dir="auto"><span class="autocomment">autocomment2: </span> </span></span>',
                                "/* autocomment */ multiple? /* autocomment2 */ ",
                        ),
+                       array(
+                               '<a href="/wiki/Special:BlankPage#autocomment_containing_.2F.2A" title="Special:BlankPage">→</a>‎<span dir="auto"><span class="autocomment">autocomment containing /*: </span> T70361</span>',
+                               "/* autocomment containing /* */ T70361"
+                       ),
                        array(
                                '<a href="#autocomment">→</a>‎<span dir="auto"><span class="autocomment">autocomment</span></span>',
                                "/* autocomment */",