diff --git a/RELEASE-NOTES-1.41 b/RELEASE-NOTES-1.41 index fb77fae9ceb..e59f39e74dd 100644 --- a/RELEASE-NOTES-1.41 +++ b/RELEASE-NOTES-1.41 @@ -376,6 +376,10 @@ because of Phabricator reports. * ResourceLoader (T127268): The targets system is deprecated. Modules that have been marked as desktop or mobile only are no longer supported and will send deprecation warnings. +* TextSlotDiffRenderer::setLanguage() is deprecated, and calling it will + have no effect. Use ContentHandler::getSlotDiffRenderer(), or in subclasses, + ContentHandler::createTextSlotDiffRenderer(), to correctly inject + dependencies into TextSlotDiffRenderer. * The static methods encodeJsVar() and encodeJsCall() have been moved from the Xml class to the more appropriate MediaWiki\Html\Html one, and the old ones are now deprecated. diff --git a/autoload.php b/autoload.php index 6561381ccb5..978b001416b 100644 --- a/autoload.php +++ b/autoload.php @@ -1064,6 +1064,12 @@ $wgAutoloadLocalClasses = [ 'MediaWiki\\Diff\\Hook\\DifferenceEngineViewHeaderHook' => __DIR__ . '/includes/diff/Hook/DifferenceEngineViewHeaderHook.php', 'MediaWiki\\Diff\\Hook\\NewDifferenceEngineHook' => __DIR__ . '/includes/diff/Hook/NewDifferenceEngineHook.php', 'MediaWiki\\Diff\\Hook\\TextSlotDiffRendererTablePrefixHook' => __DIR__ . '/includes/diff/Hook/TextSlotDiffRendererTablePrefixHook.php', + 'MediaWiki\\Diff\\TextDiffer\\BaseTextDiffer' => __DIR__ . '/includes/diff/TextDiffer/BaseTextDiffer.php', + 'MediaWiki\\Diff\\TextDiffer\\ExternalTextDiffer' => __DIR__ . '/includes/diff/TextDiffer/ExternalTextDiffer.php', + 'MediaWiki\\Diff\\TextDiffer\\ManifoldTextDiffer' => __DIR__ . '/includes/diff/TextDiffer/ManifoldTextDiffer.php', + 'MediaWiki\\Diff\\TextDiffer\\PhpTextDiffer' => __DIR__ . '/includes/diff/TextDiffer/PhpTextDiffer.php', + 'MediaWiki\\Diff\\TextDiffer\\TextDiffer' => __DIR__ . '/includes/diff/TextDiffer/TextDiffer.php', + 'MediaWiki\\Diff\\TextDiffer\\Wikidiff2TextDiffer' => __DIR__ . '/includes/diff/TextDiffer/Wikidiff2TextDiffer.php', 'MediaWiki\\Diff\\WordAccumulator' => __DIR__ . '/includes/libs/Diff/WordAccumulator.php', 'MediaWiki\\EditPage\\Constraint\\AccidentalRecreationConstraint' => __DIR__ . '/includes/editpage/Constraint/AccidentalRecreationConstraint.php', 'MediaWiki\\EditPage\\Constraint\\AutoSummaryMissingSummaryConstraint' => __DIR__ . '/includes/editpage/Constraint/AutoSummaryMissingSummaryConstraint.php', diff --git a/includes/content/ContentHandler.php b/includes/content/ContentHandler.php index 34a6ba4ded7..3d4b60fce34 100644 --- a/includes/content/ContentHandler.php +++ b/includes/content/ContentHandler.php @@ -31,6 +31,7 @@ use MediaWiki\Content\Renderer\ContentParseParams; use MediaWiki\Content\Transform\PreloadTransformParams; use MediaWiki\Content\Transform\PreSaveTransformParams; use MediaWiki\Content\ValidationParams; +use MediaWiki\Diff\TextDiffer\ManifoldTextDiffer; use MediaWiki\HookContainer\HookRunner; use MediaWiki\HookContainer\ProtectedHookAccessorTrait; use MediaWiki\Logger\LoggerFactory; @@ -622,7 +623,13 @@ abstract class ContentHandler { * @since 1.32 * * @param IContextSource $context - * @param array $options of the slot diff renderer (optional) + * @param array $options An associative array of options passed to the SlotDiffRenderer: + * - diff-type: (string) The text diff format + * - contentLanguage: (string) The language code of the content language, + * to be passed to the TextDiffer constructor. This is ignored if a + * TextDiffer object is provided. + * - textDiffer: (TextDiffer) A TextDiffer object to use for text + * comparison. * @return SlotDiffRenderer */ final public function getSlotDiffRenderer( IContextSource $context, array $options = [] ) { @@ -660,7 +667,7 @@ abstract class ContentHandler { * @stable to override * * @param IContextSource $context - * @param array $options + * @param array $options See getSlotDiffRenderer() * * @return SlotDiffRenderer */ @@ -679,7 +686,7 @@ abstract class ContentHandler { * * @since 1.41 * - * @param array $options + * @param array $options See getSlotDiffRenderer() * @return TextSlotDiffRenderer */ final protected function createTextSlotDiffRenderer( array $options = [] ): TextSlotDiffRenderer { @@ -688,29 +695,34 @@ abstract class ContentHandler { $services = MediaWikiServices::getInstance(); $statsdDataFactory = $services->getStatsdDataFactory(); $slotDiffRenderer->setStatsdDataFactory( $statsdDataFactory ); - if ( isset( $options['contentLanguage'] ) ) { - $language = $services->getLanguageFactory()->getLanguage( $options['contentLanguage'] ); - } else { - $language = $services->getContentLanguage(); - } - $slotDiffRenderer->setLanguage( $language ); $slotDiffRenderer->setHookContainer( $services->getHookContainer() ); $slotDiffRenderer->setContentModel( $this->getModelID() ); - $inline = ( $options['diff-type'] ?? '' ) === 'inline'; - $engine = DifferenceEngine::getEngine(); - - if ( $engine === 'php' ) { - $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_PHP ); - } elseif ( $engine === 'wikidiff2' ) { - if ( $inline ) { - $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_WIKIDIFF2_INLINE ); - } else { - $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_WIKIDIFF2 ); - } + if ( isset( $options['textDiffer'] ) ) { + $textDiffer = $options['textDiffer']; } else { - $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_EXTERNAL, $engine ); + if ( isset( $options['contentLanguage'] ) ) { + $language = $services->getLanguageFactory()->getLanguage( $options['contentLanguage'] ); + } else { + $language = $services->getContentLanguage(); + } + $config = $services->getMainConfig(); + $textDiffer = new ManifoldTextDiffer( + RequestContext::getMain(), + $language, + $config->get( MainConfigNames::DiffEngine ), + $config->get( MainConfigNames::ExternalDiffEngine ) + ); } + $format = $options['diff-type'] ?? 'table'; + if ( !$textDiffer->hasFormat( $format ) ) { + // Maybe it would be better to throw an exception here, but at + // present, the value comes straight from user input without + // validation, so we have to fall back. + $format = 'table'; + } + $slotDiffRenderer->setFormat( $format ); + $slotDiffRenderer->setTextDiffer( $textDiffer ); return $slotDiffRenderer; } diff --git a/includes/diff/DifferenceEngine.php b/includes/diff/DifferenceEngine.php index 184e7021383..d40383d2c19 100644 --- a/includes/diff/DifferenceEngine.php +++ b/includes/diff/DifferenceEngine.php @@ -23,6 +23,7 @@ use MediaWiki\CommentFormatter\CommentFormatter; use MediaWiki\Content\IContentHandlerFactory; +use MediaWiki\Diff\TextDiffer\ManifoldTextDiffer; use MediaWiki\HookContainer\HookRunner; use MediaWiki\Html\Html; use MediaWiki\Linker\Linker; @@ -74,7 +75,7 @@ class DifferenceEngine extends ContextSource { * fixes important bugs or such to force cached diff views to * clear. */ - private const DIFF_VERSION = '1.12'; + private const DIFF_VERSION = '1.41'; /** * Revision ID for the old revision. 0 for the revision previous to $mNewid, false @@ -219,6 +220,9 @@ class DifferenceEngine extends ContextSource { */ private $slotDiffOptions = []; + /** @var ManifoldTextDiffer|null */ + private $textDiffer; + /** * @var LinkRenderer */ @@ -314,7 +318,8 @@ class DifferenceEngine extends ContextSource { $this->slotDiffRenderers[$role] = $handler->getSlotDiffRenderer( $this->getContext(), $this->slotDiffOptions + [ - 'contentLanguage' => $this->getDiffLang()->getCode() + 'contentLanguage' => $this->getDiffLang()->getCode(), + 'textDiffer' => $this->getTextDiffer() ] ); } @@ -931,8 +936,8 @@ class DifferenceEngine extends ContextSource { $notice .= Html::warningBox( $msg->parse() ); } - // Check if wikidiff2 engine is installed - if ( $this->getEngine() === 'wikidiff2' ) { + // Check if inline switcher will be needed + if ( $this->getTextDiffer()->hasFormat( 'inline' ) ) { $out->enableOOUI(); } @@ -1433,26 +1438,21 @@ class DifferenceEngine extends ContextSource { throw new BadMethodCallException( 'mOldid and mNewid must be set to get diff cache key.' ); } - $engine = $this->getEngine(); $params = [ 'diff', - $engine === 'php' ? false : $engine, // Back compat self::DIFF_VERSION, "old-{$this->mOldid}", "rev-{$this->mNewid}" ]; - if ( $engine === 'wikidiff2' ) { - $params[] = phpversion( 'wikidiff2' ); - } - + $extraKeys = []; if ( !$this->isSlotDiffRenderer ) { foreach ( $this->getSlotDiffRenderers() as $slotDiffRenderer ) { - $params = array_merge( $params, $slotDiffRenderer->getExtraCacheKeys() ); + $extraKeys = array_merge( $extraKeys, $slotDiffRenderer->getExtraCacheKeys() ); } } - - return $params; + ksort( $extraKeys ); + return array_merge( $params, array_values( $extraKeys ) ); } /** @@ -1488,11 +1488,24 @@ class DifferenceEngine extends ContextSource { } /** - * @param array $options for the difference engine. - * Accepts keys 'diff-type' and 'expand-url' + * @param array $options for the difference engine. Available options: + * - diff-type: The text diff format, e.g. "table" or "inline". If the + * specified format is not supported, the option will be ignored, so + * the site default format (table) will be used. + * - expand-url: If true, put full URLs in href attributes (for action=render) + * FIXME: expand-url is not a slot diff option, it is a DifferenceEngine option. */ public function setSlotDiffOptions( $options ) { - $this->slotDiffOptions = $options; + $validatedOptions = []; + if ( isset( $options['diff-type'] ) + && $this->getTextDiffer()->hasFormat( $options['diff-type'] ) + ) { + $validatedOptions['diff-type'] = $options['diff-type']; + } + if ( !empty( $options['expand-url'] ) ) { + $validatedOptions['expand-url'] = true; + } + $this->slotDiffOptions = $validatedOptions; } /** @@ -1556,53 +1569,14 @@ class DifferenceEngine extends ContextSource { * @internal For use by this class and within Core only. */ public static function getEngine() { - $diffEngine = MediaWikiServices::getInstance()->getMainConfig() - ->get( MainConfigNames::DiffEngine ); - $externalDiffEngine = MediaWikiServices::getInstance()->getMainConfig() - ->get( MainConfigNames::ExternalDiffEngine ); - - if ( $diffEngine === null ) { - $engines = [ 'external', 'wikidiff2', 'php' ]; + $differenceEngine = new self; + $engine = $differenceEngine->getTextDiffer()->getEngineForFormat( 'table' ); + if ( $engine === 'external' ) { + return MediaWikiServices::getInstance()->getMainConfig() + ->get( MainConfigNames::ExternalDiffEngine ); } else { - $engines = [ $diffEngine ]; + return $engine; } - - $failureReason = null; - foreach ( $engines as $engine ) { - switch ( $engine ) { - case 'external': - if ( is_string( $externalDiffEngine ) ) { - if ( is_executable( $externalDiffEngine ) ) { - return $externalDiffEngine; - } - $failureReason = 'ExternalDiffEngine config points to a non-executable'; - if ( $diffEngine === null ) { - wfDebug( "$failureReason, ignoring" ); - } - } else { - $failureReason = 'ExternalDiffEngine config is set to a non-string value'; - if ( $diffEngine === null && $externalDiffEngine ) { - wfWarn( "$failureReason, ignoring" ); - } - } - break; - - case 'wikidiff2': - if ( function_exists( 'wikidiff2_do_diff' ) ) { - return 'wikidiff2'; - } - $failureReason = 'wikidiff2 is not available'; - break; - - case 'php': - // Always available. - return 'php'; - - default: - throw new DomainException( 'Invalid value for $wgDiffEngine: ' . $engine ); - } - } - throw new UnexpectedValueException( "Cannot use diff engine '$engine': $failureReason" ); } /** @@ -1649,59 +1623,7 @@ class DifferenceEngine extends ContextSource { * @return string */ private function localiseDiff( $text ) { - $text = $this->localiseLineNumbers( $text ); - if ( $this->getEngine() === 'wikidiff2' && - version_compare( phpversion( 'wikidiff2' ), '1.5.1', '>=' ) - ) { - $text = $this->addLocalisedTitleTooltips( $text ); - } - return $text; - } - - /** - * Replace line numbers with the text in the user's language - * - * @param string $text - * - * @return string - */ - public function localiseLineNumbers( $text ) { - return preg_replace_callback( - '//', - function ( array $matches ) { - if ( $matches[1] === '1' && $this->mReducedLineNumbers ) { - return ''; - } - return $this->msg( 'lineno' )->numParams( $matches[1] )->escaped(); - }, - $text - ); - } - - /** - * Add title attributes for tooltips on various diff elements - * - * @param string $text - * @return string - */ - private function addLocalisedTitleTooltips( $text ) { - // Moved paragraph indicators. - $replacements = [ - 'class="mw-diff-movedpara-right"' => - 'class="mw-diff-movedpara-right" title="' . - $this->msg( 'diff-paragraph-moved-toold' )->escaped() . '"', - 'class="mw-diff-movedpara-left"' => - 'class="mw-diff-movedpara-left" title="' . - $this->msg( 'diff-paragraph-moved-tonew' )->escaped() . '"', - ]; - // For inline diffs, add tooltips to `` and ``. - if ( isset( $this->slotDiffOptions['diff-type'] ) && $this->slotDiffOptions['diff-type'] == 'inline' ) { - $replacements[''] = Html::openElement( 'ins', - [ 'title' => $this->msg( 'diff-inline-tooltip-ins' )->plain() ] ); - $replacements[''] = Html::openElement( 'del', - [ 'title' => $this->msg( 'diff-inline-tooltip-del' )->plain() ] ); - } - return strtr( $text, $replacements ); + return $this->getTextDiffer()->localize( $this->getTextDiffFormat(), $text ); } /** @@ -2267,4 +2189,41 @@ class DifferenceEngine extends ContextSource { return true; } + /** + * Get the TextDiffer which will be used for rendering text + * + * @return ManifoldTextDiffer + */ + protected function getTextDiffer() { + if ( $this->textDiffer === null ) { + $this->textDiffer = new ManifoldTextDiffer( + $this->getContext(), + $this->getDiffLang(), + $this->getConfig()->get( MainConfigNames::DiffEngine ), + $this->getConfig()->get( MainConfigNames::ExternalDiffEngine ) + ); + } + return $this->textDiffer; + } + + /** + * Get the list of supported text diff formats + * + * @since 1.41 + * @return array|string[] + */ + public function getSupportedFormats() { + return $this->getTextDiffer()->getFormats(); + } + + /** + * Get the selected text diff format + * + * @since 1.41 + * @return string + */ + public function getTextDiffFormat() { + return $this->slotDiffOptions['diff-type'] ?? 'table'; + } + } diff --git a/includes/diff/TextDiffer/BaseTextDiffer.php b/includes/diff/TextDiffer/BaseTextDiffer.php new file mode 100644 index 00000000000..76478988f0b --- /dev/null +++ b/includes/diff/TextDiffer/BaseTextDiffer.php @@ -0,0 +1,127 @@ +localizer = $localizer; + } + + /** + * Provide a MessageLocalizer, or throw if setLocalizer() has not been called. + * + * @return MessageLocalizer + */ + protected function getLocalizer(): MessageLocalizer { + return $this->localizer; + } + + public function hasFormat( string $format ): bool { + return in_array( $format, $this->getFormats(), true ); + } + + public function addRowWrapper( string $format, string $diffText ): string { + if ( $this->getFormatContext( $format ) === self::CONTEXT_PLAIN ) { + return "$diffText"; + } else { + return $diffText; + } + } + + /** + * Throw an exception if any of the formats in the array is not supported. + * + * @param string[] $formats + */ + protected function validateFormats( $formats ) { + $badFormats = array_diff( $formats, $this->getFormats() ); + if ( $badFormats ) { + throw new \InvalidArgumentException( 'The requested format is not supported by this engine' ); + } + } + + public function render( string $oldText, string $newText, string $format ): string { + $result = $this->renderBatch( $oldText, $newText, [ $format ] ); + return reset( $result ); + } + + public function renderBatch( string $oldText, string $newText, array $formats ): array { + $this->validateFormats( $formats ); + if ( !count( $formats ) ) { + return []; + } + return $this->doRenderBatch( $oldText, $newText, $formats ); + } + + /** + * Subclasses should override this to render diffs in the specified formats. + * The $formats array is guaranteed to not be empty and to contain only + * formats supported by the subclass. + * + * @param string $oldText + * @param string $newText + * @param array $formats + * @return array + */ + abstract protected function doRenderBatch( string $oldText, string $newText, array $formats ): array; + + public function addModules( OutputPage $out, string $format ): void { + } + + public function getCacheKeys( array $formats ): array { + return []; + } + + public function localize( string $format, string $diff, array $options = [] ): string { + return $diff; + } + + public function getTablePrefixes( string $format ): array { + return []; + } + + public function getPreferredFormatBatch( string $format ): array { + return [ $format ]; + } + + /** + * Replace a common convention for language-independent line numbers with + * the text in the language of the current localizer. + * + * @param string $text + * @param bool $reducedLineNumbers + * + * @return string + */ + protected function localizeLineNumbers( + $text, $reducedLineNumbers + ) { + return preg_replace_callback( '//', + function ( array $matches ) use ( $reducedLineNumbers ) { + if ( $matches[1] === '1' && $reducedLineNumbers ) { + return ''; + } + return $this->getLocalizer()->msg( 'lineno' )->numParams( $matches[1] )->escaped(); + }, $text ); + } + +} diff --git a/includes/diff/TextDiffer/ExternalTextDiffer.php b/includes/diff/TextDiffer/ExternalTextDiffer.php new file mode 100644 index 00000000000..0691d066436 --- /dev/null +++ b/includes/diff/TextDiffer/ExternalTextDiffer.php @@ -0,0 +1,75 @@ +externalPath = $externalPath; + } + + public function getName(): string { + return 'external'; + } + + public function getFormats(): array { + return [ 'external' ]; + } + + public function getFormatContext( string $format ) { + return self::CONTEXT_ROW; + } + + protected function doRenderBatch( string $oldText, string $newText, array $formats ): array { + return [ 'external' => $this->doRender( $oldText, $newText ) ]; + } + + /** + * @param string $oldText + * @param string $newText + * @return string + */ + private function doRender( $oldText, $newText ) { + $tmpDir = wfTempDir(); + $tempName1 = tempnam( $tmpDir, 'diff_' ); + $tempName2 = tempnam( $tmpDir, 'diff_' ); + + $tempFile1 = fopen( $tempName1, "w" ); + if ( !$tempFile1 ) { + throw new RuntimeException( "Could not create temporary file $tempName1 for external diffing" ); + } + $tempFile2 = fopen( $tempName2, "w" ); + if ( !$tempFile2 ) { + throw new RuntimeException( "Could not create temporary file $tempName2 for external diffing" ); + } + fwrite( $tempFile1, $oldText ); + fwrite( $tempFile2, $newText ); + fclose( $tempFile1 ); + fclose( $tempFile2 ); + $cmd = [ $this->externalPath, $tempName1, $tempName2 ]; + $result = Shell::command( $cmd ) + ->execute(); + $exitCode = $result->getExitCode(); + if ( $exitCode !== 0 ) { + throw new RuntimeException( "External diff command returned code {$exitCode}. Stderr: " + . wfEscapeWikiText( $result->getStderr() ) + ); + } + $diffText = $result->getStdout(); + unlink( $tempName1 ); + unlink( $tempName2 ); + + return $diffText; + } +} diff --git a/includes/diff/TextDiffer/ManifoldTextDiffer.php b/includes/diff/TextDiffer/ManifoldTextDiffer.php new file mode 100644 index 00000000000..60b21223afd --- /dev/null +++ b/includes/diff/TextDiffer/ManifoldTextDiffer.php @@ -0,0 +1,284 @@ +localizer = $localizer; + $this->contentLanguage = $contentLanguage; + $this->diffEngine = $diffEngine; + $this->externalPath = $externalPath; + } + + public function getName(): string { + return 'manifold'; + } + + public function getFormats(): array { + $differs = $this->getDiffersByFormat(); + return array_keys( $differs ); + } + + public function hasFormat( string $format ): bool { + $differs = $this->getDiffersByFormat(); + return isset( $differs[$format] ); + } + + public function render( string $oldText, string $newText, string $format ): string { + if ( !in_array( $format, $this->getFormats(), true ) ) { + throw new \InvalidArgumentException( + 'The requested format is not supported by this engine' ); + } + $results = $this->renderBatch( $oldText, $newText, [ $format ] ); + return reset( $results ); + } + + public function renderBatch( string $oldText, string $newText, array $formats ): array { + $result = []; + $differs = $this->splitBatchByDiffer( $formats ); + /** @var TextDiffer $differ */ + foreach ( $differs as [ $differ, $formatBatch ] ) { + $result += $differ->renderBatch( $oldText, $newText, $formatBatch ); + } + return $result; + } + + public function getFormatContext( string $format ) { + return $this->getDifferForFormat( $format )->getFormatContext( $format ); + } + + public function addRowWrapper( string $format, string $diffText ): string { + return $this->getDifferForFormat( $format )->addRowWrapper( $format, $diffText ); + } + + public function addModules( OutputPage $out, string $format ): void { + $this->getDifferForFormat( $format )->addModules( $out, $format ); + } + + public function getCacheKeys( array $formats ): array { + $keys = []; + $engines = []; + $differs = $this->splitBatchByDiffer( $formats ); + /** @var TextDiffer $differ */ + foreach ( $differs as [ $differ, $formatBatch ] ) { + $keys += $differ->getCacheKeys( $formatBatch ); + $engines[] = $differ->getName() . '=' . implode( ',', $formatBatch ); + } + $keys['10-formats-and-engines'] = implode( ';', $engines ); + return $keys; + } + + public function localize( string $format, string $diff, array $options = [] ): string { + return $this->getDifferForFormat( $format )->localize( $format, $diff, $options ); + } + + public function getTablePrefixes( string $format ): array { + return $this->getDifferForFormat( $format )->getTablePrefixes( $format ); + } + + public function getPreferredFormatBatch( string $format ): array { + return $this->getDifferForFormat( $format )->getPreferredFormatBatch( $format ); + } + + /** + * @return TextDiffer[] + */ + private function getDiffersByFormat() { + if ( $this->differsByFormat === null ) { + $differs = []; + foreach ( $this->getDiffers() as $differ ) { + foreach ( $differ->getFormats() as $format ) { + // getDiffers() is in order of priority -- don't overwrite + $differs[$format] ??= $differ; + } + } + $this->differsByFormat = $differs; + } + return $this->differsByFormat; + } + + /** + * @param string $format + * @return TextDiffer + */ + private function getDifferForFormat( $format ) { + $differs = $this->getDiffersByFormat(); + if ( !isset( $differs[$format] ) ) { + throw new \InvalidArgumentException( + "Unknown format \"$format\"" + ); + } + return $differs[$format]; + } + + /** + * Disable text differs apart from the one with the given name. + * + * @param string $name + */ + public function setEngine( string $name ) { + $this->diffEngine = $name; + $this->differs = null; + $this->differsByFormat = null; + } + + /** + * Get the text differ name which will be used for the specified format + * + * @param string $format + * @return string|null + */ + public function getEngineForFormat( string $format ) { + return $this->getDifferForFormat( $format )->getName(); + } + + /** + * Get differs in a numerically indexed array. When a format is requested, + * the first TextDiffer in this array which can handle the format will be + * used. + * + * @return TextDiffer[] + */ + private function getDiffers() { + if ( $this->differs === null ) { + $differs = []; + if ( $this->diffEngine === null ) { + $differNames = [ 'external', 'wikidiff2', 'php' ]; + } else { + $differNames = [ $this->diffEngine ]; + } + $failureReason = ''; + foreach ( $differNames as $name ) { + $differ = $this->maybeCreateDiffer( $name, $failureReason ); + if ( $differ ) { + $this->injectDeps( $differ ); + $differs[] = $differ; + } + } + if ( !$differs ) { + throw new UnexpectedValueException( + "Cannot use diff engine '{$this->diffEngine}': $failureReason" ); + } + // TODO: add a hook here, allowing extensions to add differs + $this->differs = $differs; + } + return $this->differs; + } + + /** + * Initialize an object which may be a subclass of BaseTextDiffer, passing + * down injected dependencies. + * + * @param TextDiffer $differ + */ + public function injectDeps( TextDiffer $differ ) { + if ( $differ instanceof BaseTextDiffer ) { + $differ->setLocalizer( $this->localizer ); + } + } + + /** + * Create a TextDiffer by engine name. If it can't be created due to a + * configuration or platform issue, return null and set $failureReason. + * + * @param string $engine + * @param string &$failureReason Out param which will be set to the failure reason + * @return TextDiffer|null + */ + private function maybeCreateDiffer( $engine, &$failureReason ) { + switch ( $engine ) { + case 'external': + if ( is_string( $this->externalPath ) ) { + if ( is_executable( $this->externalPath ) ) { + return new ExternalTextDiffer( + $this->externalPath + ); + } + $failureReason = 'ExternalDiffEngine config points to a non-executable'; + } elseif ( $this->externalPath ) { + $failureReason = 'ExternalDiffEngine config is set to a non-string value'; + } else { + return null; + } + wfWarn( "$failureReason, ignoring" ); + return null; + + case 'wikidiff2': + if ( Wikidiff2TextDiffer::isInstalled() ) { + return new Wikidiff2TextDiffer; + } + $failureReason = 'wikidiff2 is not available'; + return null; + + case 'php': + // Always available. + return new PhpTextDiffer( + $this->contentLanguage + ); + + default: + throw new DomainException( 'Invalid value for $wgDiffEngine: ' . $engine ); + } + } + + /** + * Given an array of formats, break it down by the TextDiffer object which + * will handle each format. Each element of the result array is a list in + * which the first element is the TextDiffer object, and the second element + * is the list of formats which the TextDiffer will handle. + * + * @param array $formats + * @return array|array{0:TextDiffer,1:string[]} + */ + private function splitBatchByDiffer( $formats ) { + $result = []; + foreach ( $formats as $format ) { + $differ = $this->getDifferForFormat( $format ); + $name = $differ->getName(); + if ( isset( $result[$name] ) ) { + $result[$name][1][] = $format; + } else { + $result[$name] = [ $differ, [ $format ] ]; + } + } + return array_values( $result ); + } +} diff --git a/includes/diff/TextDiffer/PhpTextDiffer.php b/includes/diff/TextDiffer/PhpTextDiffer.php new file mode 100644 index 00000000000..421b1cd8eaf --- /dev/null +++ b/includes/diff/TextDiffer/PhpTextDiffer.php @@ -0,0 +1,62 @@ +contentLanguage = $contentLanguage; + } + + public function getName(): string { + return 'php'; + } + + public function getFormats(): array { + return [ 'table' ]; + } + + public function getFormatContext( string $format ) { + return $format === 'table' ? self::CONTEXT_ROW : self::CONTEXT_PLAIN; + } + + protected function doRenderBatch( string $oldText, string $newText, array $formats ): array { + $language = $this->contentLanguage; + if ( $language ) { + $oldText = $language->segmentForDiff( $oldText ); + $newText = $language->segmentForDiff( $newText ); + } + $oldLines = explode( "\n", $oldText ); + $newLines = explode( "\n", $newText ); + $diff = new Diff( $oldLines, $newLines ); + $result = []; + foreach ( $formats as $format ) { + // @phan-suppress-next-line PhanNoopSwitchCases -- rectified in followup commit + switch ( $format ) { + default: // 'table': + $formatter = new TableDiffFormatter(); + $diffText = $formatter->format( $diff ); + break; + } + if ( $language ) { + $diffText = $language->unsegmentForDiff( $diffText ); + } + $result[$format] = $diffText; + } + + return $result; + } + + public function localize( string $format, string $diff, array $options = [] ): string { + return $this->localizeLineNumbers( $diff, $options['reducedLineNumbers'] ?? false ); + } +} diff --git a/includes/diff/TextDiffer/TextDiffer.php b/includes/diff/TextDiffer/TextDiffer.php new file mode 100644 index 00000000000..75111a48dca --- /dev/null +++ b/includes/diff/TextDiffer/TextDiffer.php @@ -0,0 +1,134 @@ + + */ + public function getCacheKeys( array $formats ): array; + + /** + * Expand messages in the diff text using the current MessageLocalizer. + * + * Perform any other necessary post-cache transformations. + * + * @param string $format + * @param string $diff + * @param array $options An associative array of options, may contain: + * - reducedLineNumbers: If true, remove "line 1" but allow other line numbers + * @return string + */ + public function localize( string $format, string $diff, array $options = [] ): string; + + /** + * Get table prefixes for the specified format. These are HTML fragments + * placed above all slot diffs. The key should be a string, used for sorting + * and deduplication. + * + * @param string $format + * @return array + */ + public function getTablePrefixes( string $format ): array; + + /** + * Given a format, get a list of formats which can be generated at the same + * time with minimal additional CPU cost. + * + * @param string $format + * @return string[] + */ + public function getPreferredFormatBatch( string $format ): array; +} diff --git a/includes/diff/TextDiffer/Wikidiff2TextDiffer.php b/includes/diff/TextDiffer/Wikidiff2TextDiffer.php new file mode 100644 index 00000000000..08933797b71 --- /dev/null +++ b/includes/diff/TextDiffer/Wikidiff2TextDiffer.php @@ -0,0 +1,165 @@ +version = self::$fakeVersionForTesting ?? phpversion( 'wikidiff2' ); + $this->haveMoveSupport = version_compare( $this->version, '1.5.0', '>=' ); + $this->haveCutoffParameter = $this->haveMoveSupport + && version_compare( $this->version, '1.8.0', '<' ); + } + + public function getName(): string { + return 'wikidiff2'; + } + + public function getFormatContext( string $format ) { + return $format === 'inline' ? self::CONTEXT_PLAIN : self::CONTEXT_ROW; + } + + public function getCacheKeys( array $formats ): array { + return [ '20-wikidiff2-version' => $this->version ]; + } + + public function doRenderBatch( string $oldText, string $newText, array $formats ): array { + $result = []; + foreach ( $formats as $format ) { + switch ( $format ) { + case 'table': + $result['table'] = $this->doTableFormat( $oldText, $newText ); + break; + + case 'inline': + $result['inline'] = $this->doInlineFormat( $oldText, $newText ); + break; + } + } + return $result; + } + + /** + * Do a table format diff + * + * @param string $old + * @param string $new + * @return string + */ + private function doTableFormat( $old, $new ) { + if ( $this->haveCutoffParameter ) { + return wikidiff2_do_diff( + $old, + $new, + 2, + 0 + ); + } else { + // Don't pass the 4th parameter introduced in version 1.5.0 and removed in version 1.8.0 + return wikidiff2_do_diff( + $old, + $new, + 2 + ); + } + } + + /** + * Do an inline format diff + * + * @param string $oldText + * @param string $newText + * @return string + */ + private function doInlineFormat( $oldText, $newText ) { + return wikidiff2_inline_diff( $oldText, $newText, 2 ); + } + + public function getFormats(): array { + return [ 'table', 'inline' ]; + } + + public function getTablePrefixes( string $format ): array { + $localizer = $this->getLocalizer(); + $ins = Html::element( 'span', + [ 'class' => 'mw-diff-inline-legend-ins' ], + $localizer->msg( 'diff-inline-tooltip-ins' )->plain() + ); + $del = Html::element( 'span', + [ 'class' => 'mw-diff-inline-legend-del' ], + $localizer->msg( 'diff-inline-tooltip-del' )->plain() + ); + $hideDiffClass = $format === 'inline' ? '' : 'oo-ui-element-hidden'; + $legend = Html::rawElement( 'div', + [ 'class' => 'mw-diff-inline-legend ' . $hideDiffClass ], "$ins $del" + ); + return [ TextSlotDiffRenderer::INLINE_LEGEND_KEY => $legend ]; + } + + public function localize( string $format, string $diff, array $options = [] ): string { + $diff = $this->localizeLineNumbers( $diff, + $options['reducedLineNumbers'] ?? false ); + // FIXME: in the inline case, comments like remain in the output + if ( $this->haveMoveSupport ) { + $diff = $this->addLocalizedTitleTooltips( $format, $diff ); + } + return $diff; + } + + /** + * Add title attributes for tooltips on various diff elements + * + * @param string $format + * @param string $text + * @return string + */ + private function addLocalizedTitleTooltips( $format, $text ) { + // Moved paragraph indicators. + $localizer = $this->getLocalizer(); + $replacements = [ + 'class="mw-diff-movedpara-right"' => + 'class="mw-diff-movedpara-right" title="' . + $localizer->msg( 'diff-paragraph-moved-toold' )->escaped() . '"', + 'class="mw-diff-movedpara-left"' => + 'class="mw-diff-movedpara-left" title="' . + $localizer->msg( 'diff-paragraph-moved-tonew' )->escaped() . '"', + ]; + // For inline diffs, add tooltips to `` and ``. + if ( $format == 'inline' ) { + $replacements[''] = Html::openElement( 'ins', + [ 'title' => $localizer->msg( 'diff-inline-tooltip-ins' )->plain() ] ); + $replacements[''] = Html::openElement( 'del', + [ 'title' => $localizer->msg( 'diff-inline-tooltip-del' )->plain() ] ); + } + return strtr( $text, $replacements ); + } + +} diff --git a/includes/diff/TextSlotDiffRenderer.php b/includes/diff/TextSlotDiffRenderer.php index 22ca94ce912..c7d5f4403cb 100644 --- a/includes/diff/TextSlotDiffRenderer.php +++ b/includes/diff/TextSlotDiffRenderer.php @@ -21,18 +21,16 @@ * @ingroup DifferenceEngine */ - use MediaWiki\HookContainer\HookContainer; +use MediaWiki\Diff\TextDiffer\ManifoldTextDiffer; +use MediaWiki\Diff\TextDiffer\TextDiffer; +use MediaWiki\HookContainer\HookContainer; use MediaWiki\HookContainer\HookRunner; use MediaWiki\Html\Html; use MediaWiki\MainConfigNames; use MediaWiki\MediaWikiServices; -use MediaWiki\Shell\Shell; use MediaWiki\Title\Title; use OOUI\ButtonGroupWidget; use OOUI\ButtonWidget; -use Wikimedia\Assert\Assert; -use Wikimedia\Diff\Diff; -use Wikimedia\Diff\TableDiffFormatter; /** * Renders a slot diff by doing a text diff on the native representation. @@ -65,27 +63,21 @@ class TextSlotDiffRenderer extends SlotDiffRenderer { /** @var IBufferingStatsdDataFactory|null */ private $statsdDataFactory; - /** @var Language|null The language this content is in. */ - private $language; - /** @var HookRunner|null */ private $hookRunner; - /** @var string One of the ENGINE_* constants. */ - private $engine = self::ENGINE_PHP; - - /** @var string|null Path to an executable to be used as the diff engine. */ - private $externalEngine; + /** @var string|null */ + private $format; /** @var string */ private $contentModel; + /** @var TextDiffer|null */ + private $textDiffer; + /** @inheritDoc */ public function getExtraCacheKeys() { - // Tell DifferenceEngine this is a different variant from the standard wikidiff2 variant - return $this->engine === self::ENGINE_WIKIDIFF2_INLINE ? [ - phpversion( 'wikidiff2' ), 'inline' - ] : []; + return $this->textDiffer->getCacheKeys( [ $this->format ] ); } /** @@ -113,10 +105,13 @@ class TextSlotDiffRenderer extends SlotDiffRenderer { } /** - * @param Language $language The language of the text being diffed, for word segmentation + * This has no effect since MW 1.41. The language is now injected via setTextDiffer(). + * + * @param Language $language + * @deprecated since 1.41 */ public function setLanguage( Language $language ) { - $this->language = $language; + wfDeprecated( __METHOD__, '1.41' ); } /** @@ -137,23 +132,72 @@ class TextSlotDiffRenderer extends SlotDiffRenderer { /** * Set which diff engine to use. + * * @param string $type One of the ENGINE_* constants. - * @param string|null $executable Path to an external executable, only when type is ENGINE_EXTERNAL. + * @param null $executable Must be null since 1.41. Previously a path to execute. */ public function setEngine( $type, $executable = null ) { - $engines = [ self::ENGINE_PHP, self::ENGINE_WIKIDIFF2, self::ENGINE_EXTERNAL, - self::ENGINE_WIKIDIFF2_INLINE ]; - Assert::parameter( in_array( $type, $engines, true ), '$type', - 'must be one of the TextSlotDiffRenderer::ENGINE_* constants' ); - if ( $type === self::ENGINE_EXTERNAL ) { - Assert::parameter( is_string( $executable ) && is_executable( $executable ), '$executable', - 'must be a path to a valid executable' ); - } else { - Assert::parameter( $executable === null, '$executable', - 'must not be set unless $type is ENGINE_EXTERNAL' ); + if ( $executable !== null ) { + throw new \InvalidArgumentException( + 'The $executable parameter is no longer supported and must be null' + ); } - $this->engine = $type; - $this->externalEngine = $executable; + switch ( $type ) { + case self::ENGINE_PHP: + $engine = 'php'; + $format = 'table'; + break; + + case self::ENGINE_WIKIDIFF2: + $engine = 'wikidiff2'; + $format = 'table'; + break; + + case self::ENGINE_EXTERNAL: + $engine = 'external'; + $format = 'external'; + break; + + case self::ENGINE_WIKIDIFF2_INLINE: + $engine = 'wikidiff2'; + $format = 'inline'; + break; + + default: + throw new \InvalidArgumentException( '$type ' . + 'must be one of the TextSlotDiffRenderer::ENGINE_* constants' ); + } + if ( $this->textDiffer instanceof ManifoldTextDiffer ) { + $this->textDiffer->setEngine( $engine ); + } + $this->setFormat( $format ); + } + + /** + * Set the TextDiffer format + * + * @since 1.41 + * @param string $format + */ + public function setFormat( $format ) { + $this->format = $format; + } + + /** + * @param TextDiffer $textDiffer + */ + public function setTextDiffer( TextDiffer $textDiffer ) { + $this->textDiffer = $textDiffer; + } + + /** + * Get the current TextDiffer, or throw an exception if setTextDiffer() has + * not been called. + * + * @return TextDiffer + */ + private function getTextDiffer(): TextDiffer { + return $this->textDiffer; } /** @@ -180,56 +224,39 @@ class TextSlotDiffRenderer extends SlotDiffRenderer { * @inheritDoc */ public function getTablePrefix( IContextSource $context, Title $newTitle ): array { - $legend = null; - $inlineSwitcher = null; + $parts = $this->getTextDiffer()->getTablePrefixes( $this->format ); $showDiffToggleSwitch = $context->getConfig()->get( MainConfigNames::ShowDiffToggleSwitch ); - - // wikidiff2 inline type gets a legend to explain the highlighting colours and show an inline toggle - if ( $this->engine === self::ENGINE_WIKIDIFF2 || - $this->engine === self::ENGINE_WIKIDIFF2_INLINE ) { - $ins = Html::element( 'span', - [ 'class' => 'mw-diff-inline-legend-ins' ], - $context->msg( 'diff-inline-tooltip-ins' )->plain() + // If we support the inline type, add a toggle switch + if ( $showDiffToggleSwitch && $this->getTextDiffer()->hasFormat( 'inline' ) ) { + $values = $context->getRequest()->getValues(); + $isInlineDiffType = $this->format === 'inline'; + unset( $values[ 'diff-type' ] ); + unset( $values[ 'title' ] ); + $parts[self::INLINE_SWITCHER_KEY] = Html::rawElement( 'div', + [ 'class' => 'mw-diffPage-inlineToggle-container' ], + // Will be replaced by a ButtonSelectWidget in JS + new ButtonGroupWidget( [ + 'items' => [ + new ButtonWidget( [ + 'active' => $isInlineDiffType, + 'label' => $context->msg( 'diff-inline-format-label' )->plain(), + 'href' => $newTitle->getLocalURL( $values ) . '&diff-type=inline' + ] ), + new ButtonWidget( [ + 'active' => !$isInlineDiffType, + 'label' => $context->msg( 'diff-table-format-label' )->plain(), + 'href' => $newTitle->getLocalURL( $values ) + ] ) + ] + ] ) ); - $del = Html::element( 'span', - [ 'class' => 'mw-diff-inline-legend-del' ], - $context->msg( 'diff-inline-tooltip-del' )->plain() - ); - $hideDiffClass = $this->engine === self::ENGINE_WIKIDIFF2 ? 'oo-ui-element-hidden' : ''; - $legend = Html::rawElement( 'div', - [ 'class' => 'mw-diff-inline-legend ' . $hideDiffClass ], "$ins $del" - ); - - if ( $showDiffToggleSwitch ) { - $values = $context->getRequest()->getValues(); - $isInlineDiffType = isset( $values['diff-type'] ) && $values['diff-type'] === 'inline'; - unset( $values[ 'diff-type' ] ); - unset( $values[ 'title' ] ); - $inlineSwitcher = Html::rawElement( 'div', - [ 'class' => 'mw-diffPage-inlineToggle-container' ], - // Will be replaced by a ButtonSelectWidget in JS - new ButtonGroupWidget( [ - 'items' => [ - new ButtonWidget( [ - 'active' => $isInlineDiffType, - 'label' => $context->msg( 'diff-inline-format-label' )->plain(), - 'href' => $newTitle->getLocalURL( $values ) . '&diff-type=inline' - ] ), - new ButtonWidget( [ - 'active' => !$isInlineDiffType, - 'label' => $context->msg( 'diff-table-format-label' )->plain(), - 'href' => $newTitle->getLocalURL( $values ) - ] ) - ] - ] ) - ); - } } - // Allow extensions to add other parts to this area (or modify the legend). - // An empty placeholder for the legend is added when it's not in use and other items have been added. - $parts = [ self::INLINE_LEGEND_KEY => $legend, self::INLINE_SWITCHER_KEY => $inlineSwitcher ]; + // Add an empty placeholder for the legend is added when it's not in + // use and other items have been added. + $parts += [ self::INLINE_LEGEND_KEY => null, self::INLINE_SWITCHER_KEY => null ]; + // Allow extensions to add other parts to this area (or modify the legend). $this->hookRunner->onTextSlotDiffRendererTablePrefix( $this, $context, $parts ); if ( count( $parts ) > 1 && $parts[self::INLINE_LEGEND_KEY] === null ) { $parts[self::INLINE_LEGEND_KEY] = Html::element( 'div' ); @@ -287,9 +314,6 @@ class TextSlotDiffRenderer extends SlotDiffRenderer { * @throws Exception */ protected function getTextDiffInternal( $oldText, $newText ) { - // TODO move most of this into three parallel implementations of a text diff generator - // class, choose which one to use via dependency injection - $oldText = str_replace( "\r\n", "\n", $oldText ); $newText = str_replace( "\r\n", "\n", $newText ); @@ -297,86 +321,9 @@ class TextSlotDiffRenderer extends SlotDiffRenderer { return ''; } - // Better external diff engine, the 2 may some day be dropped - // This one does the escaping and segmenting itself - if ( $this->engine === self::ENGINE_WIKIDIFF2 ) { - $wikidiff2Version = phpversion( 'wikidiff2' ); - if ( - $wikidiff2Version !== false && - version_compare( $wikidiff2Version, '1.5.0', '>=' ) && - version_compare( $wikidiff2Version, '1.8.0', '<' ) - ) { - $text = wikidiff2_do_diff( - $oldText, - $newText, - 2, - 0 - ); - } else { - // Don't pass the 4th parameter introduced in version 1.5.0 and removed in version 1.8.0 - $text = wikidiff2_do_diff( - $oldText, - $newText, - 2 - ); - } - - return $text; - } elseif ( $this->engine === self::ENGINE_EXTERNAL ) { - # Diff via the shell - $tmpDir = wfTempDir(); - $tempName1 = tempnam( $tmpDir, 'diff_' ); - $tempName2 = tempnam( $tmpDir, 'diff_' ); - - $tempFile1 = fopen( $tempName1, "w" ); - if ( !$tempFile1 ) { - throw new Exception( "Could not create temporary file $tempName1 for external diffing" ); - } - $tempFile2 = fopen( $tempName2, "w" ); - if ( !$tempFile2 ) { - throw new Exception( "Could not create temporary file $tempName2 for external diffing" ); - } - fwrite( $tempFile1, $oldText ); - fwrite( $tempFile2, $newText ); - fclose( $tempFile1 ); - fclose( $tempFile2 ); - $cmd = [ $this->externalEngine, $tempName1, $tempName2 ]; - $result = Shell::command( $cmd ) - ->execute(); - $exitCode = $result->getExitCode(); - if ( $exitCode !== 0 ) { - throw new Exception( "External diff command returned code {$exitCode}. Stderr: " - . wfEscapeWikiText( $result->getStderr() ) - ); - } - $difftext = $result->getStdout(); - unlink( $tempName1 ); - unlink( $tempName2 ); - - return $difftext; - } elseif ( $this->engine === self::ENGINE_PHP ) { - if ( $this->language ) { - $oldText = $this->language->segmentForDiff( $oldText ); - $newText = $this->language->segmentForDiff( $newText ); - } - $ota = explode( "\n", $oldText ); - $nta = explode( "\n", $newText ); - $diffs = new Diff( $ota, $nta ); - $formatter = new TableDiffFormatter(); - $difftext = $formatter->format( $diffs ); - if ( $this->language ) { - $difftext = $this->language->unsegmentForDiff( $difftext ); - } - - return $difftext; - } elseif ( $this->engine === self::ENGINE_WIKIDIFF2_INLINE ) { - // Note wikidiff2_inline_diff returns an element sans table. - // Due to the way other diffs work (return a table with before and after), we need to wrap - // the output in a row that spans the 4 columns that are expected, so that our diff appears in - // the correct place! - return '' . wikidiff2_inline_diff( $oldText, $newText, 2 ) . ''; - } - throw new LogicException( 'Invalid engine: ' . $this->engine ); + $textDiffer = $this->getTextDiffer(); + $diffText = $textDiffer->render( $oldText, $newText, $this->format ); + return $textDiffer->addRowWrapper( $this->format, $diffText ); } } diff --git a/tests/common/TestsAutoLoader.php b/tests/common/TestsAutoLoader.php index 08a85c807db..6efb0827be3 100644 --- a/tests/common/TestsAutoLoader.php +++ b/tests/common/TestsAutoLoader.php @@ -124,6 +124,7 @@ $wgAutoloadClasses += [ # tests/phpunit/includes/diff 'CustomDifferenceEngine' => "$testDir/phpunit/includes/diff/CustomDifferenceEngine.php", + 'MediaWiki\Tests\Diff\TextDiffer\TextDifferData' => "$testDir/phpunit/includes/diff/TextDiffer/TextDifferData.php", # tests/phpunit/includes/externalstore 'ExternalStoreForTesting' => "$testDir/phpunit/includes/externalstore/ExternalStoreForTesting.php", diff --git a/tests/phpunit/includes/diff/DifferenceEngineTest.php b/tests/phpunit/includes/diff/DifferenceEngineTest.php index c766dafcdb2..1716085a62f 100644 --- a/tests/phpunit/includes/diff/DifferenceEngineTest.php +++ b/tests/phpunit/includes/diff/DifferenceEngineTest.php @@ -7,7 +7,6 @@ use MediaWiki\Revision\RevisionRecord; use MediaWiki\Revision\SlotRecord; use MediaWiki\Title\Title; use MediaWiki\User\UserIdentityValue; -use Wikimedia\TestingAccessWrapper; /** * @covers DifferenceEngine @@ -199,33 +198,6 @@ class DifferenceEngineTest extends MediaWikiIntegrationTestCase { $this->assertEquals( $revs[2], $diffEngine->getNewid(), 'diff get new id' ); } - public static function provideLocaliseTitleTooltipsTestData() { - return [ - 'moved paragraph left shoud get new location title' => [ - '⚫', - '⚫', - ], - 'moved paragraph right shoud get old location title' => [ - '⚫', - '⚫', - ], - 'nothing changed when key not hit' => [ - '⚫', - '⚫', - ], - ]; - } - - /** - * @dataProvider provideLocaliseTitleTooltipsTestData - */ - public function testAddLocalisedTitleTooltips( $input, $expected ) { - $this->setContentLang( 'qqx' ); - /** @var DifferenceEngine $diffEngine */ - $diffEngine = TestingAccessWrapper::newFromObject( new DifferenceEngine() ); - $this->assertEquals( $expected, $diffEngine->addLocalisedTitleTooltips( $input ) ); - } - /** * @dataProvider provideGenerateContentDiffBody */ @@ -292,7 +264,7 @@ class DifferenceEngineTest extends MediaWikiIntegrationTestCase { * @dataProvider provideGetDiffBody */ public function testGetDiffBody( - ?array $oldSlots, ?array $newSlots, $expectedDiff + ?array $oldSlots, ?array $newSlots, $slotDiffOptions, $expectedDiff ) { $oldRevision = $this->getRevisionRecord( $oldSlots ); $newRevision = $this->getRevisionRecord( $newSlots ); @@ -302,6 +274,9 @@ class DifferenceEngineTest extends MediaWikiIntegrationTestCase { } $differenceEngine = new DifferenceEngine(); $differenceEngine->setRevisions( $oldRevision, $newRevision ); + if ( $slotDiffOptions !== null ) { + $differenceEngine->setSlotDiffOptions( $slotDiffOptions ); + } if ( $expectedDiff instanceof Exception ) { return; } @@ -323,38 +298,51 @@ class DifferenceEngineTest extends MediaWikiIntegrationTestCase { 'revision vs. null' => [ null, $main1 + $slot1, + null, '', ], 'revision vs. itself' => [ $main1 + $slot1, $main1 + $slot1, + null, '', ], 'different text in one slot' => [ $main1 + $slot1, $main1 + $slot2, + null, "slotLine 1:\nLine 1:\n-aaa+bbb", ], 'different text in two slots' => [ $main1 + $slot1, $main2 + $slot2, + null, "Line 1:\nLine 1:\n-xxx+yyy\nslotLine 1:\nLine 1:\n-aaa+bbb", ], 'new slot' => [ $main1, $main1 + $slot1, + null, "slotLine 1:\nLine 1:\n- +aaa", ], 'ignored difference in derived slot' => [ $main1 + $slot3, $main1 + $slot4, + null, '', ], 'incompatible slot' => [ $main1 + $slot5, $main2 + $slot1, + null, "Line 1:\nLine 1:\n-xxx+yyy\nslotCannot compare content models \"testing\" and \"plain text\"", ], + 'invalid diff-type' => [ + $main1, + $main2, + [ 'diff-type' => 'invalid' ], + "Line 1:\nLine 1:\n-xxx+yyy", + ] ]; } diff --git a/tests/phpunit/includes/diff/TextDiffer/ExternalTextDifferTest.php b/tests/phpunit/includes/diff/TextDiffer/ExternalTextDifferTest.php new file mode 100644 index 00000000000..d5d6bd36592 --- /dev/null +++ b/tests/phpunit/includes/diff/TextDiffer/ExternalTextDifferTest.php @@ -0,0 +1,23 @@ +markTestSkipped( 'ExternalTextDiffer can\'t pass extra ' . + 'arguments like $wgPhpCli, so it\'s hard to be platform-independent' ); + } + $oldText = 'foo'; + $newText = 'bar'; + $differ = new ExternalTextDiffer( __DIR__ . '/externalDiffTest.sh' ); + $result = $differ->render( $oldText, $newText, 'external' ); + $this->assertSame( "- foo\n+ bar\n", $result ); + } +} diff --git a/tests/phpunit/includes/diff/TextDiffer/ManifoldTextDifferTest.php b/tests/phpunit/includes/diff/TextDiffer/ManifoldTextDifferTest.php new file mode 100644 index 00000000000..2a3fc836afb --- /dev/null +++ b/tests/phpunit/includes/diff/TextDiffer/ManifoldTextDifferTest.php @@ -0,0 +1,122 @@ +getServiceContainer(); + return new ManifoldTextDiffer( + RequestContext::getMain(), + $services->getLanguageFactory()->getLanguage( 'en' ), + $configVars['DiffEngine'] ?? null, + $configVars['ExternalDiffEngine'] ?? null + ); + } + + public function testGetName() { + $this->assertSame( 'manifold', $this->createDiffer()->getName() ); + } + + public function testGetFormats() { + if ( extension_loaded( 'wikidiff2' ) ) { + $formats = [ 'table', 'inline' ]; + } else { + $formats = [ 'table' ]; + } + $this->assertSame( + $formats, + $this->createDiffer()->getFormats() + ); + } + + public function testHasFormat() { + $differ = $this->createDiffer(); + $this->assertTrue( $differ->hasFormat( 'table' ) ); + if ( extension_loaded( 'wikidiff2' ) ) { + $this->assertTrue( $differ->hasFormat( 'inline' ) ); + } + $this->assertFalse( $differ->hasFormat( 'external' ) ); + $this->assertFalse( $differ->hasFormat( 'nonexistent' ) ); + } + + public function testHasFormatExternal() { + $differ = $this->createDiffer( [ + 'ExternalDiffEngine' => __DIR__ . '/externalDiffTest.sh' + ] ); + $this->assertTrue( $differ->hasFormat( 'external' ) ); + } + + public function testRenderForcePhp() { + $differ = $this->createDiffer( [ + 'DiffEngine' => 'php' + ] ); + $result = $differ->render( 'foo', 'bar', 'table' ); + $this->assertSame( + TextDifferData::PHP_TABLE, + $result + ); + } + + /** + * @requires extension wikidiff2 + */ + public function testRenderUnforcedWikidiff2() { + $differ = $this->createDiffer(); + $result = $differ->render( 'foo', 'bar', 'table' ); + $this->assertSame( + TextDifferData::WIKIDIFF2_TABLE, + $result + ); + } + + /** + * @requires extension wikidiff2 + */ + public function testRenderBatchWikidiff2External() { + if ( !is_executable( '/bin/sh' ) ) { + $this->markTestSkipped( 'ExternalTextDiffer can\'t pass extra ' . + 'arguments like $wgPhpCli, so it\'s hard to be platform-independent' ); + } + $differ = $this->createDiffer( [ + 'ExternalDiffEngine' => __DIR__ . '/externalDiffTest.sh' + ] ); + $result = $differ->renderBatch( 'foo', 'bar', [ 'table', 'inline', 'external' ] ); + $this->assertSame( + [ + 'table' => TextDifferData::WIKIDIFF2_TABLE, + 'inline' => TextDifferData::WIKIDIFF2_INLINE, + 'external' => TextDifferData::EXTERNAL + ], + $result + ); + } + + public static function provideAddRowWrapper() { + return [ + [ 'table', false ], + [ 'external', false ], + ]; + } + + /** + * @dataProvider provideAddRowWrapper + * @param string $format + * @param bool $isWrap + */ + public function testAddRowWrapper( $format, $isWrap ) { + $differ = $this->createDiffer( [ + 'ExternalDiffEngine' => __DIR__ . '/externalDiffTest.sh' + ] ); + $result = $differ->addRowWrapper( $format, 'foo' ); + if ( $isWrap ) { + $this->assertSame( 'foo', $result ); + } else { + $this->assertSame( 'foo', $result ); + } + } +} diff --git a/tests/phpunit/includes/diff/TextDiffer/PhpTextDifferTest.php b/tests/phpunit/includes/diff/TextDiffer/PhpTextDifferTest.php new file mode 100644 index 00000000000..0573eab5ad4 --- /dev/null +++ b/tests/phpunit/includes/diff/TextDiffer/PhpTextDifferTest.php @@ -0,0 +1,111 @@ +getServiceContainer() + ->getLanguageFactory() + ->getLanguage( 'en' ); + $differ = new PhpTextDiffer( $lang ); + + $localizer = RequestContext::getMain(); + $localizer->setLanguage( $lang ); + + $differ->setLocalizer( $localizer ); + return $differ; + } + + public function testRender() { + $differ = $this->createDiffer(); + $result = $differ->render( 'foo', 'bar', 'table' ); + $this->assertSame( TextDifferData::PHP_TABLE, $result ); + } + + public static function provideRenderBatch() { + return [ + 'empty' => [ + [], + [] + ], + 'one format' => [ + [ 'table' ], + [ + 'table' => TextDifferData::PHP_TABLE, + ] + ], + ]; + } + + /** + * @dataProvider provideRenderBatch + * @param array $formats + * @param array $expected + */ + public function testRenderBatch( $formats, $expected ) { + $oldText = 'foo'; + $newText = 'bar'; + $differ = $this->createDiffer(); + $result = $differ->renderBatch( $oldText, $newText, $formats ); + $this->assertSame( $expected, $result ); + } + + public function testHasFormat() { + $differ = $this->createDiffer(); + $this->assertTrue( $differ->hasFormat( 'table' ) ); + $this->assertFalse( $differ->hasFormat( 'external' ) ); + } + + public function testAddModules() { + $out = RequestContext::getMain()->getOutput(); + $differ = $this->createDiffer(); + $differ->addModules( $out, 'table' ); + $this->assertSame( [], $out->getModules() ); + } + + public function testGetCacheKeys() { + $differ = $this->createDiffer(); + $result = $differ->getCacheKeys( [ 'table' ] ); + $this->assertSame( [], $result ); + } + + public static function provideLocalize() { + return [ + [ 1, [], 'Line 1:' ], + [ 2, [], 'Line 2:' ], + [ 1, [ 'reducedLineNumbers' => true ], '' ], + ]; + } + + /** + * @dataProvider provideLocalize + * @param string $line + * @param array $options + * @param string $expected + */ + public function testLocalize( $line, $options, $expected ) { + $differ = $this->createDiffer(); + $result = $differ->localize( + 'table', + "", + $options + ); + $this->assertSame( $expected, $result ); + } + + public function testGetTablePrefixes() { + $this->assertSame( [], $this->createDiffer()->getTablePrefixes( 'table' ) ); + } + + public function testGetPreferredFormatBatch() { + $this->assertSame( + [ 'table' ], + $this->createDiffer()->getPreferredFormatBatch( 'table' ) + ); + } +} diff --git a/tests/phpunit/includes/diff/TextDiffer/TextDifferData.php b/tests/phpunit/includes/diff/TextDiffer/TextDifferData.php new file mode 100644 index 00000000000..cbda305fd66 --- /dev/null +++ b/tests/phpunit/includes/diff/TextDiffer/TextDifferData.php @@ -0,0 +1,33 @@ + + +
foo
bar
+'; + + public const WIKIDIFF2_TABLE = ' + + + + + + +
bar
+ + + +
foo
+ + +'; + + public const WIKIDIFF2_INLINE = '
+
bar
+
foo
+'; +} diff --git a/tests/phpunit/includes/diff/TextDiffer/Wikidiff2TextDifferTest.php b/tests/phpunit/includes/diff/TextDiffer/Wikidiff2TextDifferTest.php new file mode 100644 index 00000000000..af389c4ec53 --- /dev/null +++ b/tests/phpunit/includes/diff/TextDiffer/Wikidiff2TextDifferTest.php @@ -0,0 +1,148 @@ +setLanguage( 'qqx' ); + $differ->setLocalizer( $localizer ); + TestingAccessWrapper::newFromObject( $differ )->haveMoveSupport = true; + return $differ; + } + + /** + * @requires extension wikidiff2 + */ + public function testRenderBatch() { + $oldText = 'foo'; + $newText = 'bar'; + $differ = new Wikidiff2TextDiffer(); + // Should not need a MessageLocalizer + $result = $differ->renderBatch( $oldText, $newText, [ 'table', 'inline' ] ); + $this->assertSame( + [ + 'table' => TextDifferData::WIKIDIFF2_TABLE, + 'inline' => TextDifferData::WIKIDIFF2_INLINE + ], + $result + ); + } + + public function testGetName() { + $differ = new Wikidiff2TextDiffer(); + $this->assertSame( 'wikidiff2', $differ->getName() ); + } + + public function testGetFormatContext() { + $differ = new Wikidiff2TextDiffer(); + $this->assertSame( TextDiffer::CONTEXT_ROW, $differ->getFormatContext( 'table' ) ); + } + + public static function provideGetTablePrefixes() { + return [ + [ + 'table', + 'class="mw-diff-inline-legend oo-ui-element-hidden".*\(diff-inline-tooltip-ins\)' + ], + [ + 'inline', + 'class="mw-diff-inline-legend".*\(diff-inline-tooltip-ins\)' + ], + ]; + } + + /** + * @dataProvider provideGetTablePrefixes + * @param string $format + * @param string $pattern + */ + public function testGetTablePrefixes( $format, $pattern ) { + $differ = $this->createDiffer(); + $result = $differ->getTablePrefixes( $format ); + $this->assertMatchesRegularExpression( + '{' . $pattern . '}s', + $result[TextSlotDiffRenderer::INLINE_LEGEND_KEY] + ); + } + + public static function provideLocalize() { + return [ + 'normal table' => [ + 'table', + TextDifferData::WIKIDIFF2_TABLE, + [], + '\(lineno: 1\)' + ], + 'table with move tooltip' => [ + 'table', + // From wikidiff2 001.phpt + '⚫', + [], + 'title="\(diff-paragraph-moved-tonew\)"' + ], + 'table with reduced line numbers' => [ + 'table', + TextDifferData::WIKIDIFF2_TABLE, + [ 'reducedLineNumbers' => true ], + '' + ], + 'inline tooltip' => [ + 'inline', + TextDifferData::WIKIDIFF2_INLINE, + [], + '' + ], + ]; + } + + /** + * @dataProvider provideLocalize + * @param string $format + * @param string $input + * @param array $options + * @param string $pattern + */ + public function testLocalize( $format, $input, $options, $pattern ) { + $differ = $this->createDiffer(); + $result = $differ->localize( $format, $input, $options ); + $this->assertMatchesRegularExpression( + '{' . $pattern . '}s', + $result + ); + } + + public static function provideAddLocalizedTitleTooltips() { + return [ + 'moved paragraph left shoud get new location title' => [ + '⚫', + '⚫', + ], + 'moved paragraph right shoud get old location title' => [ + '⚫', + '⚫', + ], + 'nothing changed when key not hit' => [ + '⚫', + '⚫', + ], + ]; + } + + /** + * @dataProvider provideAddLocalizedTitleTooltips + */ + public function testAddLocalizedTitleTooltips( $input, $expected ) { + $differ = TestingAccessWrapper::newFromObject( $this->createDiffer() ); + + $this->assertEquals( $expected, $differ->addLocalizedTitleTooltips( 'table', $input ) ); + } + +} diff --git a/tests/phpunit/includes/diff/TextDiffer/externalDiffTest.sh b/tests/phpunit/includes/diff/TextDiffer/externalDiffTest.sh new file mode 100755 index 00000000000..c0c3ca7efb7 --- /dev/null +++ b/tests/phpunit/includes/diff/TextDiffer/externalDiffTest.sh @@ -0,0 +1,8 @@ +#!/bin/sh + +echo -n "- " +cat $1 +echo +echo -n "+ " +cat $2 +echo diff --git a/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php b/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php index bca75dbf2e8..ac3d93de277 100644 --- a/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php +++ b/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php @@ -1,5 +1,7 @@ getTextSlotDiffRenderer(); $key = $slotDiffRenderer->getExtraCacheKeys(); $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_WIKIDIFF2_INLINE ); $inlineKey = $slotDiffRenderer->getExtraCacheKeys(); - $this->assertSame( [], $key ); - $this->assertSame( $inlineKey, [ phpversion( 'wikidiff2' ), 'inline' ] ); + + ksort( $key ); + ksort( $inlineKey ); + + $this->assertSame( [ '10-formats-and-engines' => 'php=table' ], $key ); + $this->assertSame( + [ + '10-formats-and-engines' => 'wikidiff2=inline', + '20-wikidiff2-version' => '1.14.1' + ], + $inlineKey + ); } /** @@ -91,14 +111,24 @@ class TextSlotDiffRendererTest extends MediaWikiIntegrationTestCase { // no separate test for getTextDiff() as getDiff() is just a thin wrapper around it /** + * @param string $langCode * @return TextSlotDiffRenderer */ - private function getTextSlotDiffRenderer() { + private function getTextSlotDiffRenderer( $langCode = 'en' ) { $slotDiffRenderer = new TextSlotDiffRenderer(); $slotDiffRenderer->setStatsdDataFactory( new NullStatsdDataFactory() ); - $slotDiffRenderer->setLanguage( - $this->getServiceContainer()->getLanguageFactory()->getLanguage( 'en' ) ); - $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_PHP ); + $lang = $this->getServiceContainer()->getLanguageFactory()->getLanguage( $langCode ); + $context = new RequestContext; + $context->setLanguage( $lang ); + $differ = new ManifoldTextDiffer( + $context, + $lang, + 'php', + null, + [] + ); + $slotDiffRenderer->setTextDiffer( $differ ); + $slotDiffRenderer->setFormat( 'table' ); return $slotDiffRenderer; } @@ -166,7 +196,7 @@ class TextSlotDiffRendererTest extends MediaWikiIntegrationTestCase { OOUI\Theme::setSingleton( new OOUI\BlankTheme() ); $this->overrideConfigValue( MainConfigNames::ShowDiffToggleSwitch, true ); - $slotDiffRenderer = $this->getTextSlotDiffRenderer(); + $slotDiffRenderer = $this->getTextSlotDiffRenderer( 'qqx' ); $slotDiffRenderer->setHookContainer( $this->createHookContainer() ); $slotDiffRenderer->setEngine( $engine );