jenkins-bot has submitted this change and it was merged. Change subject: Api summary parameter should not override auto comment ......................................................................
Api summary parameter should not override auto comment This change means that we will always show the auto comment and as much of the user provided summary as possible when given Bug: T97247 Change-Id: Ia375a67185c0b320cf03ae8fcc0db2a08d0d094e --- M repo/includes/SummaryFormatter.php M repo/tests/phpunit/includes/SummaryFormatterTest.php 2 files changed, 50 insertions(+), 36 deletions(-) Approvals: Jonas Kress (WMDE): Looks good to me, but someone else must approve Thiemo Mättig (WMDE): Looks good to me, approved Addshore: Looks good to me, but someone else must approve jenkins-bot: Verified diff --git a/repo/includes/SummaryFormatter.php b/repo/includes/SummaryFormatter.php index a453b4b..deafa34 100644 --- a/repo/includes/SummaryFormatter.php +++ b/repo/includes/SummaryFormatter.php @@ -14,7 +14,6 @@ use Wikibase\DataModel\Services\EntityId\EntityIdParsingException; use Wikibase\DataModel\Snak\Snak; use Wikibase\Lib\SnakFormatter; -use Wikibase\Repo\WikibaseRepo; /** * Formatter for Summary objects @@ -26,6 +25,7 @@ * @author John Erling Blad * @author Daniel Kinzler * @author Tobias Gritschacher < [email protected] > + * @author Adam Shorland */ class SummaryFormatter { @@ -64,8 +64,12 @@ * * @throws InvalidArgumentException */ - public function __construct( EntityIdFormatter $idFormatter, ValueFormatter $valueFormatter, - SnakFormatter $snakFormatter, Language $language, EntityIdParser $idParser + public function __construct( + EntityIdFormatter $idFormatter, + ValueFormatter $valueFormatter, + SnakFormatter $snakFormatter, + Language $language, + EntityIdParser $idParser ) { if ( $snakFormatter->getFormat() !== SnakFormatter::FORMAT_PLAIN ) { throw new InvalidArgumentException( @@ -239,33 +243,32 @@ /** * Merge the total summary * - * @param string $comment autocomment part, will be placed in a block comment - * @param string $summary human readable string to be appended after the autocomment part + * @param string $autoComment autocomment part, will be placed in a block comment + * @param string $autoSummary human readable string to be appended after the autocomment part + * @param string $userSummary user provided summary to be appended after the autoSummary * * @return string to be used for the summary */ - private function assembleSummaryString( $comment, $summary ) { - $normalizer = WikibaseRepo::getDefaultInstance()->getStringNormalizer(); - - $comment = $normalizer->trimToNFC( $comment ); - $summary = $normalizer->trimToNFC( $summary ); + private function assembleSummaryString( $autoComment, $autoSummary, $userSummary ) { $mergedString = ''; - if ( $comment !== '' ) { - $mergedString .= '/* ' . $comment . ' */'; + $autoComment = $this->stringNormalizer->trimToNFC( $autoComment ); + $autoSummary = $this->stringNormalizer->trimToNFC( $autoSummary ); + $userSummary = $this->stringNormalizer->trimToNFC( $userSummary ); + + if ( $autoComment !== '' ) { + $mergedString .= '/* ' . $autoComment . ' */ '; } - if ( $summary !== '' ) { - if ( $mergedString !== '' ) { - // Having a space after the comment is commonly known from section edits - $mergedString .= ' '; - } - $mergedString .= $this->language->truncate( - $summary, - SUMMARY_MAX_LENGTH - strlen( $mergedString ) - ); + + if ( $autoSummary !== '' && $userSummary !== '' ) { + $mergedString .= $this->language->commaList( array( $autoSummary, $userSummary ) ); + } elseif ( $autoSummary !== '' ) { + $mergedString .= $autoSummary; + } elseif ( $userSummary !== '' ) { + $mergedString .= $userSummary; } // leftover entities should be removed, but its not clear how this shall be done - return $mergedString; + return $this->language->truncate( rtrim( $mergedString ), SUMMARY_MAX_LENGTH ); } /** @@ -280,18 +283,11 @@ public function formatSummary( Summary $summary ) { $userSummary = $summary->getUserSummary(); - if ( !is_null( $userSummary ) ) { - $autoSummary = $userSummary; - } else { - $autoSummary = self::formatAutoSummary( $summary ); - } - - $autoComment = $this->formatAutoComment( $summary ); - $autoComment = $this->stringNormalizer->trimToNFC( $autoComment ); - $autoSummary = $this->stringNormalizer->trimToNFC( $autoSummary ); - - $totalSummary = self::assembleSummaryString( $autoComment, $autoSummary ); - return $totalSummary; + return $this->assembleSummaryString( + $this->formatAutoComment( $summary ), + $this->formatAutoSummary( $summary ), + $userSummary === null ? '' : $userSummary + ); } } diff --git a/repo/tests/phpunit/includes/SummaryFormatterTest.php b/repo/tests/phpunit/includes/SummaryFormatterTest.php index ddd0156..908dfec 100644 --- a/repo/tests/phpunit/includes/SummaryFormatterTest.php +++ b/repo/tests/phpunit/includes/SummaryFormatterTest.php @@ -377,7 +377,7 @@ array( 'x', 'y' ), array( 'A', 'B' ), 'can I haz world domination?', - '/* summarytest-testing:2|nl|x|y */ can I haz world domination?' + '/* summarytest-testing:2|nl|x|y */ A, B, can I haz world domination?' ), 'Trimming' => array( 'summarytest', @@ -386,7 +386,7 @@ array( ' autoArg0 ', ' autoArg1 ' ), array( ' userArg0 ', ' userArg1 ' ), ' userSummary ', - '/* summarytest-testing:2|de| autoArg0 | autoArg1 */ userSummary' + '/* summarytest-testing:2|de| autoArg0 | autoArg1 */ userArg0 , userArg1, userSummary' ), 'User summary only' => array( 'summarytest', @@ -397,6 +397,24 @@ 'can I haz world domination?', '/* summarytest:0| */ can I haz world domination?' ), + 'User summary w/o arguments' => array( + 'summarytest', + 'testing', + 'de', + array( 'autoArg0', 'autoArg1' ), + null, + 'userSummary', + '/* summarytest-testing:0|de|autoArg0|autoArg1 */ userSummary' + ), + 'User summary w/o auto comment arguments' => array( + 'summarytest', + 'testing', + 'de', + null, + array( 'userArg0', 'userArg1' ), + 'userSummary', + '/* summarytest-testing:2|de */ userArg0, userArg1, userSummary' + ), 'Array arguments' => array( 'summarytest', 'testing', -- To view, visit https://gerrit.wikimedia.org/r/232037 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia375a67185c0b320cf03ae8fcc0db2a08d0d094e Gerrit-PatchSet: 10 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Addshore <[email protected]> Gerrit-Reviewer: Addshore <[email protected]> Gerrit-Reviewer: Aude <[email protected]> Gerrit-Reviewer: Bene <[email protected]> Gerrit-Reviewer: Jonas Kress (WMDE) <[email protected]> Gerrit-Reviewer: Lydia Pintscher <[email protected]> Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]> Gerrit-Reviewer: Tobias Gritschacher <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
