Update documentation of what a "section" is
authorThiemo Mättig <thiemo.maettig@wikimedia.de>
Thu, 12 Jun 2014 14:05:18 +0000 (16:05 +0200)
committerThiemo Mättig <thiemo.maettig@wikimedia.de>
Thu, 12 Jun 2014 16:13:23 +0000 (18:13 +0200)
There are so many slightly different understandings of what a
"section" is or can be. I'm aware the documentation was improved
just a few weeks ago. I still find it incomplete and confusing.

1. I renamed it to $sectionId to make it more clear what it
really is.

2. Sections are usually numbers. 0, 1 and so on. There is no
reason to disallow the use of ints or even floats (this works
because the string representation of 0.0 is "0"). The code never
disallowed numbers.

3. 'T1' never was supported, as far as I can tell. 'T-1' is
supported. See Parser::extractSections().

4. null and false and '' all mean "the whole page" in
WikiPage::replaceSectionAtRev() but for some reason this meaning got
lost in WikitextContent::replaceSection(). I made it the same again.

Change-Id: Icc3997722d2ed742bf7703cd7c06d09199225720

includes/WikiPage.php
includes/content/AbstractContent.php
includes/content/Content.php
includes/content/WikitextContent.php
includes/htmlform/HTMLForm.php
includes/parser/Parser.php
languages/LanguageConverter.php

index cedbcf9..c4d1bf3 100644 (file)
@@ -1471,7 +1471,9 @@ class WikiPage implements Page, IDBAccessObject {
        }
 
        /**
-        * @param string|null|bool $section Null/false, a section number (0, 1, 2, T1, T2, ...) or "new".
+        * @param string|number|null|bool $sectionId Section identifier as a number or string
+        * (e.g. 0, 1 or 'T-1'), null/false or an empty string for the whole page
+        * or 'new' for a new section.
         * @param string $text New text of the section.
         * @param string $sectionTitle New section's subject, only if $section is "new".
         * @param string $edittime Revision timestamp or null to use the current revision.
@@ -1481,13 +1483,13 @@ class WikiPage implements Page, IDBAccessObject {
         *
         * @deprecated since 1.21, use replaceSectionAtRev() instead
         */
-       public function replaceSection( $section, $text, $sectionTitle = '',
+       public function replaceSection( $sectionId, $text, $sectionTitle = '',
                $edittime = null
        ) {
                ContentHandler::deprecated( __METHOD__, '1.21' );
 
                //NOTE: keep condition in sync with condition in replaceSectionContent!
-               if ( strval( $section ) == '' ) {
+               if ( strval( $sectionId ) === '' ) {
                        // Whole-page edit; let the whole text through
                        return $text;
                }
@@ -1500,7 +1502,7 @@ class WikiPage implements Page, IDBAccessObject {
                // could even make section title, but that's not required.
                $sectionContent = ContentHandler::makeContent( $text, $this->getTitle() );
 
-               $newContent = $this->replaceSectionContent( $section, $sectionContent, $sectionTitle,
+               $newContent = $this->replaceSectionContent( $sectionId, $sectionContent, $sectionTitle,
                        $edittime );
 
                return ContentHandler::getContentText( $newContent );
@@ -1521,7 +1523,9 @@ class WikiPage implements Page, IDBAccessObject {
        }
 
        /**
-        * @param string|null|bool $section Null/false, a section number (0, 1, 2, T1, T2, ...) or "new".
+        * @param string|number|null|bool $sectionId Section identifier as a number or string
+        * (e.g. 0, 1 or 'T-1'), null/false or an empty string for the whole page
+        * or 'new' for a new section.
         * @param Content $sectionContent New content of the section.
         * @param string $sectionTitle New section's subject, only if $section is "new".
         * @param string $edittime Revision timestamp or null to use the current revision.
@@ -1532,12 +1536,12 @@ class WikiPage implements Page, IDBAccessObject {
         * @since 1.21
         * @deprecated since 1.24, use replaceSectionAtRev instead
         */
-       public function replaceSectionContent( $section, Content $sectionContent, $sectionTitle = '',
+       public function replaceSectionContent( $sectionId, Content $sectionContent, $sectionTitle = '',
                $edittime = null ) {
                wfProfileIn( __METHOD__ );
 
                $baseRevId = null;
-               if ( $edittime && $section !== 'new' ) {
+               if ( $edittime && $sectionId !== 'new' ) {
                        $dbw = wfGetDB( DB_MASTER );
                        $rev = Revision::loadFromTimestamp( $dbw, $this->mTitle, $edittime );
                        if ( $rev ) {
@@ -1546,11 +1550,13 @@ class WikiPage implements Page, IDBAccessObject {
                }
 
                wfProfileOut( __METHOD__ );
-               return $this->replaceSectionAtRev( $section, $sectionContent, $sectionTitle, $baseRevId );
+               return $this->replaceSectionAtRev( $sectionId, $sectionContent, $sectionTitle, $baseRevId );
        }
 
        /**
-        * @param string|null|bool $section Null/false, a section number (0, 1, 2, T1, T2, ...) or "new".
+        * @param string|number|null|bool $sectionId Section identifier as a number or string
+        * (e.g. 0, 1 or 'T-1'), null/false or an empty string for the whole page
+        * or 'new' for a new section.
         * @param Content $sectionContent New content of the section.
         * @param string $sectionTitle New section's subject, only if $section is "new".
         * @param string $baseRevId integer|null
@@ -1560,12 +1566,12 @@ class WikiPage implements Page, IDBAccessObject {
         *
         * @since 1.24
         */
-       public function replaceSectionAtRev( $section, Content $sectionContent,
+       public function replaceSectionAtRev( $sectionId, Content $sectionContent,
                $sectionTitle = '', $baseRevId = null
        ) {
                wfProfileIn( __METHOD__ );
 
-               if ( strval( $section ) == '' ) {
+               if ( strval( $sectionId ) === '' ) {
                        // Whole-page edit; let the whole text through
                        $newContent = $sectionContent;
                } else {
@@ -1576,7 +1582,7 @@ class WikiPage implements Page, IDBAccessObject {
                        }
 
                        // Bug 30711: always use current version when adding a new section
-                       if ( is_null( $baseRevId ) || $section == 'new' ) {
+                       if ( is_null( $baseRevId ) || $sectionId === 'new' ) {
                                $oldContent = $this->getContent();
                        } else {
                                // TODO: try DB_SLAVE first
@@ -1585,7 +1591,7 @@ class WikiPage implements Page, IDBAccessObject {
 
                                if ( !$rev ) {
                                        wfDebug( __METHOD__ . " asked for bogus section (page: " .
-                                               $this->getId() . "; section: $section)\n" );
+                                               $this->getId() . "; section: $sectionId)\n" );
                                        wfProfileOut( __METHOD__ );
                                        return null;
                                }
@@ -1599,7 +1605,7 @@ class WikiPage implements Page, IDBAccessObject {
                                return null;
                        }
 
-                       $newContent = $oldContent->replaceSection( $section, $sectionContent, $sectionTitle );
+                       $newContent = $oldContent->replaceSection( $sectionId, $sectionContent, $sectionTitle );
                }
 
                wfProfileOut( __METHOD__ );
index 8ba43f6..e09be56 100644 (file)
@@ -342,7 +342,7 @@ abstract class AbstractContent implements Content {
         *
         * @see Content::replaceSection
         */
-       public function replaceSection( $section, Content $with, $sectionTitle = '' ) {
+       public function replaceSection( $sectionId, Content $with, $sectionTitle = '' ) {
                return null;
        }
 
index 3286c0a..61b9254 100644 (file)
@@ -378,10 +378,9 @@ interface Content {
         *
         * @since 1.21
         *
-        * @param string $sectionId The section's ID, given as a numeric string.
-        *    The ID "0" retrieves the section before the first heading, "1" the
-        *    text between the first heading (included) and the second heading
-        *    (excluded), etc.
+        * @param string|number $sectionId Section identifier as a number or string
+        * (e.g. 0, 1 or 'T-1'). The ID "0" retrieves the section before the first heading, "1" the
+        * text between the first heading (included) and the second heading (excluded), etc.
         *
         * @return Content|bool|null The section, or false if no such section
         *    exist, or null if sections are not supported.
@@ -394,13 +393,15 @@ interface Content {
         *
         * @since 1.21
         *
-        * @param mixed $section Null/false or a section number (0, 1, 2, T1, T2...), or "new"
+        * @param string|number|null|bool $sectionId Section identifier as a number or string
+        * (e.g. 0, 1 or 'T-1'), null/false or an empty string for the whole page
+        * or 'new' for a new section.
         * @param Content $with New content of the section
         * @param string $sectionTitle New section's subject, only if $section is 'new'
         *
         * @return string|null Complete article text, or null if error
         */
-       public function replaceSection( $section, Content $with, $sectionTitle = '' );
+       public function replaceSection( $sectionId, Content $with, $sectionTitle = '' );
 
        /**
         * Returns a Content object with pre-save transformations applied (or this
index ccea916..cdf5962 100644 (file)
@@ -37,17 +37,17 @@ class WikitextContent extends TextContent {
        }
 
        /**
-        * @param string $section
+        * @param string|number $sectionId
         *
         * @return Content|bool|null
         *
         * @see Content::getSection()
         */
-       public function getSection( $section ) {
+       public function getSection( $sectionId ) {
                global $wgParser;
 
                $text = $this->getNativeData();
-               $sect = $wgParser->getSection( $text, $section, false );
+               $sect = $wgParser->getSection( $text, $sectionId, false );
 
                if ( $sect === false ) {
                        return false;
@@ -57,7 +57,7 @@ class WikitextContent extends TextContent {
        }
 
        /**
-        * @param string $section
+        * @param string|number|null|bool $sectionId
         * @param Content $with
         * @param string $sectionTitle
         *
@@ -66,7 +66,7 @@ class WikitextContent extends TextContent {
         *
         * @see Content::replaceSection()
         */
-       public function replaceSection( $section, Content $with, $sectionTitle = '' ) {
+       public function replaceSection( $sectionId, Content $with, $sectionTitle = '' ) {
                wfProfileIn( __METHOD__ );
 
                $myModelId = $this->getModel();
@@ -82,13 +82,13 @@ class WikitextContent extends TextContent {
                $oldtext = $this->getNativeData();
                $text = $with->getNativeData();
 
-               if ( $section === '' ) {
+               if ( strval( $sectionId ) === '' ) {
                        wfProfileOut( __METHOD__ );
 
                        return $with; # XXX: copy first?
                }
 
-               if ( $section == 'new' ) {
+               if ( $sectionId === 'new' ) {
                        # Inserting a new section
                        $subject = $sectionTitle ? wfMessage( 'newsectionheaderdefaultlevel' )
                                        ->rawParams( $sectionTitle )->inContentLanguage()->text() . "\n\n" : '';
@@ -101,7 +101,7 @@ class WikitextContent extends TextContent {
                        # Replacing an existing section; roll out the big guns
                        global $wgParser;
 
-                       $text = $wgParser->replaceSection( $oldtext, $section, $text );
+                       $text = $wgParser->replaceSection( $oldtext, $sectionId, $text );
                }
 
                $newContent = new WikitextContent( $text );
index 01f3ab7..175088d 100644 (file)
@@ -583,7 +583,7 @@ class HTMLForm extends ContextSource {
         * Add header text, inside the form.
         *
         * @param string $msg Complete text of message to display
-        * @param string $section The section to add the header to
+        * @param string|null $section The section to add the header to
         *
         * @return HTMLForm $this for chaining calls (since 1.20)
         */
@@ -605,7 +605,7 @@ class HTMLForm extends ContextSource {
         * @since 1.19
         *
         * @param string $msg Complete text of message to display
-        * @param string $section The section to add the header to
+        * @param string|null $section The section to add the header to
         *
         * @return HTMLForm $this for chaining calls (since 1.20)
         */
@@ -623,7 +623,7 @@ class HTMLForm extends ContextSource {
         * Add footer text, inside the form.
         *
         * @param string $msg complete text of message to display
-        * @param string $section The section to add the footer text to
+        * @param string|null $section The section to add the footer text to
         *
         * @return HTMLForm $this for chaining calls (since 1.20)
         */
@@ -645,7 +645,7 @@ class HTMLForm extends ContextSource {
         * @since 1.19
         *
         * @param string $msg Complete text of message to display
-        * @param string $section The section to add the footer text to
+        * @param string|null $section The section to add the footer text to
         *
         * @return HTMLForm $this for chaining calls (since 1.20)
         */
index 66cfd55..c1fce12 100644 (file)
@@ -5735,7 +5735,7 @@ class Parser {
         * External callers should use the getSection and replaceSection methods.
         *
         * @param string $text Page wikitext
-        * @param string $section A section identifier string of the form:
+        * @param string|number $sectionId A section identifier string of the form:
         *   "<flag1> - <flag2> - ... - <section number>"
         *
         * Currently the only recognised flag is "T", which means the target section number
@@ -5757,7 +5757,7 @@ class Parser {
         * @return string For "get", the extracted section text.
         *   for "replace", the whole page with the section replaced.
         */
-       private function extractSections( $text, $section, $mode, $newText = '' ) {
+       private function extractSections( $text, $sectionId, $mode, $newText = '' ) {
                global $wgTitle; # not generally used but removes an ugly failure mode
 
                $magicScopeVariable = $this->lock();
@@ -5767,7 +5767,7 @@ class Parser {
 
                # Process section extraction flags
                $flags = 0;
-               $sectionParts = explode( '-', $section );
+               $sectionParts = explode( '-', $sectionId );
                $sectionIndex = array_pop( $sectionParts );
                foreach ( $sectionParts as $part ) {
                        if ( $part === 'T' ) {
@@ -5876,12 +5876,14 @@ class Parser {
         * If a section contains subsections, these are also returned.
         *
         * @param string $text Text to look in
-        * @param string $section Section identifier
-        * @param string $deftext Default to return if section is not found
+        * @param string|number $sectionId Section identifier as a number or string
+        * (e.g. 0, 1 or 'T-1').
+        * @param string $defaultText Default to return if section is not found
+        *
         * @return string Text of the requested section
         */
-       public function getSection( $text, $section, $deftext = '' ) {
-               return $this->extractSections( $text, $section, "get", $deftext );
+       public function getSection( $text, $sectionId, $defaultText = '' ) {
+               return $this->extractSections( $text, $sectionId, 'get', $defaultText );
        }
 
        /**
@@ -5889,13 +5891,15 @@ class Parser {
         * specified by $section has been replaced with $text. If the target
         * section does not exist, $oldtext is returned unchanged.
         *
-        * @param string $oldtext Former text of the article
-        * @param int $section Section identifier
-        * @param string $text Replacing text
+        * @param string $oldText Former text of the article
+        * @param string|number $sectionId Section identifier as a number or string
+        * (e.g. 0, 1 or 'T-1').
+        * @param string $newText Replacing text
+        *
         * @return string Modified text
         */
-       public function replaceSection( $oldtext, $section, $text ) {
-               return $this->extractSections( $oldtext, $section, "replace", $text );
+       public function replaceSection( $oldText, $sectionId, $newText ) {
+               return $this->extractSections( $oldText, $sectionId, 'replace', $newText );
        }
 
        /**
index 65d74e9..b3a4c1a 100644 (file)
@@ -1062,8 +1062,8 @@ class LanguageConverter {
         * @param Content $content New page content
         * @param string $summary Edit summary of the edit
         * @param bool $isMinor Was the edit marked as minor?
-        * @param bool $isWatch Did the user watch this page or not?
-        * @param string|int $section
+        * @param null $isWatch Unused.
+        * @param null $section Unused.
         * @param int $flags Bitfield
         * @param Revision|null $revision New Revision object or null
         * @return bool True