WMDE-Fisch has uploaded a new change for review.
https://gerrit.wikimedia.org/r/322128
Change subject: [WIP] Add highlighting of changes.
......................................................................
[WIP] Add highlighting of changes.
This patch introduces a new DiffFormater that builds similar to
the ArrayDiffFormater an array that aligns changes to the lines
from the original text. The formatted array will then be used
to build the highlighted and annotated single column diff view
for conflict solving.
TODO:
- design refinements
- documentation
- use of i18n
Bug: T149720
Change-Id: Ib0c9911fe9f1e1e633df55183f1dd1c9765fd6c0
---
M extension.json
M i18n/en.json
A includes/LineBasedUnifiedDiffFormatter.php
M includes/TwoColConflictPage.php
M modules/ext.TwoColConflict.css
A tests/phpunit/includes/LineBasedUnifiedDiffFormatterTest.php
6 files changed, 626 insertions(+), 12 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/TwoColConflict
refs/changes/28/322128/1
diff --git a/extension.json b/extension.json
index 4d8d187..4af7921 100644
--- a/extension.json
+++ b/extension.json
@@ -11,7 +11,8 @@
"manifest_version": 1,
"AutoloadClasses": {
"TwoColConflictHooks": "TwoColConflict.hooks.php",
- "TwoColConflictPage": "includes/TwoColConflictPage.php"
+ "TwoColConflictPage": "includes/TwoColConflictPage.php",
+ "LineBasedUnifiedDiffFormatter":
"includes/LineBasedUnifiedDiffFormatter.php"
},
"Hooks": {
"AlternateEdit": [
diff --git a/i18n/en.json b/i18n/en.json
index 77dac1b..b94c155 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -8,7 +8,7 @@
"twoColConflict-desc": "Showing a side-by-side edit merge screen for
edit conflict resolution",
"twoColConflict-explainconflict": "Another user just edited and saved
this page. There is a conflict between your version and the current version.
You will have to merge your changes into the current text version. Only the
text in the editor field will be saved when you click on \"$1\".",
"twoColConflict-changes-col-title": "Conflicting changes",
- "twoColConflict-changes-col-desc": "Differences between the currently
published version (by $1 on $2) and your text ($3)",
+ "twoColConflict-changes-col-desc": "Differences between the currently
published version (by $1 on $2) and <span
class=\"mw-twocolconflict-user\">your</span> text ($3)",
"twoColConflict-editor-col-title": "Editor with version to be
published",
"twoColConflict-editor-col-desc": "The version in the editor will be
published. Initially, its content is the currently published version (by $1 on
$2)"
}
\ No newline at end of file
diff --git a/includes/LineBasedUnifiedDiffFormatter.php
b/includes/LineBasedUnifiedDiffFormatter.php
new file mode 100644
index 0000000..70f6a85
--- /dev/null
+++ b/includes/LineBasedUnifiedDiffFormatter.php
@@ -0,0 +1,148 @@
+<?php
+use MediaWiki\Diff\WordAccumulator;
+
+class LineBasedUnifiedDiffFormatter extends DiffFormatter {
+ public $insClass = ' class="diffchange diffchange-inline"';
+ public $delClass = ' class="diffchange diffchange-inline"';
+
+ private $oldline = 1;
+ private $newline = 1;
+ private $retval = [];
+
+ /**
+ * @param Diff $diff A Diff object.
+ *
+ * @return array[] List of associative arrays, each describing a
difference.
+ */
+ public function format( $diff ) {
+ $this->oldline = 1;
+ $this->newline = 1;
+ $this->retval = [];
+
+ foreach ( $diff->getEdits() as $edit ) {
+ switch ( $edit->getType() ) {
+ case 'add':
+ $this->addLines( $edit->getClosing() );
+ $this->newline += count(
$edit->getClosing() );
+ break;
+ case 'delete':
+ $this->deleteLines( $edit->getOrig() );
+ $this->oldline += count(
$edit->getOrig() );
+ break;
+ case 'change':
+ $wordLevelDiff =
$this->getWordLevelDiff( $edit->getOrig(), $edit->getClosing() );
+ if ( $wordLevelDiff ) {
+ $this->retval[ $this->oldline
][] = [
+ 'action' => 'delete',
+ 'old' =>
$this->getOriginalInlineDiff( $wordLevelDiff ),
+ 'oldline' =>
$this->oldline,
+ ];
+ $this->retval[ $this->oldline
][] = [
+ 'action' => 'add',
+ 'new' =>
$this->getClosingInlineDiff( $wordLevelDiff ),
+ 'newline' =>
$this->newline
+ ];
+ } else {
+ $this->deleteLines(
$edit->getOrig() );
+ $this->addLines(
$edit->getClosing() );
+ }
+ $this->newline += count(
$edit->getClosing() );
+ $this->oldline += count(
$edit->getOrig() );
+ break;
+ case 'copy':
+ $this->copyLines( $edit->getOrig() );
+ $this->oldline += count(
$edit->getOrig() );
+ $this->newline += count(
$edit->getOrig() );
+ break;
+ }
+ }
+
+ return $this->retval;
+ }
+
+ private function deleteLines( array $lines ) {
+ $this->retval[ $this->oldline ][] = [
+ 'action' => 'delete',
+ 'old' => "<del{$this->delClass}>" .
$this->composeLines( $lines ) . '</del>',
+ 'oldline' => $this->oldline,
+ ];
+ }
+
+ private function addLines( array $lines ) {
+ $this->retval[ $this->oldline ][] = [
+ 'action' => 'add',
+ 'new' => "<ins{$this->insClass}>" .
$this->composeLines( $lines ) . '</ins>',
+ 'newline' => $this->newline
+ ];
+ }
+
+ private function copyLines( array $lines ) {
+ $this->retval[ $this->oldline ][] = [
+ 'action' => 'copy',
+ 'copy' => $this->composeLines( $lines, false ),
+ 'oldline' => $this->oldline,
+ 'newline' => $this->newline
+ ];
+ $this->newline += count( $lines );
+ }
+
+ private function getWordLevelDiff( array $orig, array $closing ) {
+ $diff = new WordLevelDiff( $orig, $closing );
+
+ // when comparing long multi-word lines getting an inline
results might lead to
+ // a lot inline edits that confuse more then help
+ if ( count( $diff->getEdits() ) > 5 ) {
+ return false;
+ }
+
+ return $diff;
+ }
+
+ private function getOriginalInlineDiff( WordLevelDiff $diff ) {
+ $wordAccumulator = $this->getWordAccumulator();
+
+ foreach ( $diff->getEdits() as $edit ) {
+ if ( $edit->type == 'copy' ) {
+ $wordAccumulator->addWords( $edit->orig );
+ } elseif ( $edit->orig ) {
+ $wordAccumulator->addWords( $edit->orig, 'del'
);
+ }
+ }
+ return implode( "\n", $wordAccumulator->getLines() );
+ }
+
+ private function getClosingInlineDiff( WordLevelDiff $diff ) {
+ $wordAccumulator = $this->getWordAccumulator();
+
+ foreach ( $diff->getEdits() as $edit ) {
+ if ( $edit->type == 'copy' ) {
+ $wordAccumulator->addWords( $edit->closing );
+ } elseif ( $edit->closing ) {
+ $wordAccumulator->addWords( $edit->closing,
'ins' );
+ }
+ }
+ return implode( "\n", $wordAccumulator->getLines() );
+ }
+
+ private function getWordAccumulator() {
+ $wordAccumulator = new WordAccumulator;
+ $wordAccumulator->insClass = $this->insClass;
+ $wordAccumulator->delClass = $this->delClass;
+ return $wordAccumulator;
+ }
+
+ private function composeLines( $lines, $replaceEmptyLine = true ) {
+ $result = [];
+ foreach ( $lines as $line ) {
+ $result[] = $this->replaceEmptyLine( $line,
$replaceEmptyLine );
+ }
+ return implode( "\n", $result );
+ }
+
+ private function replaceEmptyLine( $line, $replaceEmptyLine = true ) {
+ if ( $line === '' && $replaceEmptyLine ) {
+ $line = ' ';
+ }
+ return $line;
+ }
+}
diff --git a/includes/TwoColConflictPage.php b/includes/TwoColConflictPage.php
index 101c834..82721ac 100644
--- a/includes/TwoColConflictPage.php
+++ b/includes/TwoColConflictPage.php
@@ -4,18 +4,22 @@
class TwoColConflictPage extends EditPage {
+ public function __construct( Article $article ) {
+ parent::__construct( $article );
+ }
+
protected function addExplainConflictHeader( OutputPage $out ) {
$labelAsPublish =
$this->mArticle->getContext()->getConfig()->get(
'EditSubmitButtonLabelPublish'
);
- $buttonLabel = $this->context->msg(
+ $buttonLabel = $this->getContext()->msg(
$labelAsPublish ? 'publishchanges' : 'savechanges'
)->text();
$out->wrapWikiMsg(
"<div
class='mw-twocolconflict-explainconflict'>\n$1\n</div>",
- $this->context->msg( 'twoColConflict-explainconflict',
$buttonLabel )
+ $this->getContext()->msg(
'twoColConflict-explainconflict', $buttonLabel )
);
}
@@ -43,8 +47,8 @@
$de = $handler->createDifferenceEngine(
$this->mArticle->getContext() );
$de->setContent( $content2, $content1 );
$de->showDiff(
- $this->context->msg( 'yourtext' )->parse(),
- $this->context->msg( 'storedversion' )->text()
+ $this->getContext()->msg( 'yourtext' )->parse(),
+ $this->getContext()->msg( 'storedversion' )->text()
);
}
@@ -59,9 +63,12 @@
private function buildConflictPageChangesCol() {
global $wgUser;
- $lastUser = $this->mArticle->getPage()->getUserText();
+ $lastUser =
+ '<span class="mw-twocolconflict-lastuser">' .
+ $this->mArticle->getPage()->getUserText() .
+ '</span>';
$lastChangeTime =
$this->getContext()->getLanguage()->userTimeAndDate(
- $this->mArticle->getPage()->getTimestamp(),
+ $this->getArticle()->getPage()->getTimestamp(),
$wgUser
);
$yourChangeTime =
$this->getContext()->getLanguage()->userTimeAndDate(
@@ -84,7 +91,7 @@
global $wgUser;
$name = 'mw-twocolconflict-changes-editor';
- $wikitext = $this->safeUnicodeOutput( $this->textbox1 );
+ $wikitext = $this->safeUnicodeOutput(
$this->getComposedChangedText() );
$wikitext = $this->addNewLineAtEnd( $wikitext );
$customAttribs = [];
@@ -100,8 +107,8 @@
private function buildConflictPageEditorCol() {
global $wgUser;
- $lastUser = $this->mArticle->getPage()->getUserText();
- $lastChangeTime = $this->mArticle->getPage()->getTimestamp();
+ $lastUser = $this->getArticle()->getPage()->getUserText();
+ $lastChangeTime =
$this->getArticle()->getPage()->getTimestamp();
$lastChangeTime =
$this->getContext()->getLanguage()->userTimeAndDate( $lastChangeTime, $wgUser );
$out = '<div class="mw-twocolconflict-editor-col">';
@@ -128,4 +135,52 @@
private function wikiEditorIsEnabled() {
return class_exists( WikiEditorHooks::class ) &&
WikiEditorHooks::isEnabled( 'toolbar' );
}
+
+ private function getArrayDiff( $fromTextLines, $toTextLines ) {
+ $formatter = new LineBasedUnifiedDiffFormatter();
+ $formatter->insClass = ' class="mw-twocolconflict-diffchange"';
+ $formatter->delClass = ' class="mw-twocolconflict-diffchange"';
+ return $formatter->format(
+ new Diff( $fromTextLines, $toTextLines )
+ );
+ }
+
+ private function getComposedChangedText() {
+ $lastUser = $this->getArticle()->getPage()->getUserText();
+ $currentText = $this->toEditText( $this->getCurrentContent() );
+ $yourText = $this->textbox1;
+
+ $currentLines = explode( "\n", $currentText );
+ $yourLines = explode( "\n", $yourText );
+
+ $combinedChanges = $this->getArrayDiff( $currentLines,
$yourLines );
+
+ $output = [];
+ foreach ( $currentLines as $key => $currentLine ) {
+ ++$key;
+ if ( isset( $combinedChanges[ $key ] ) ) {
+ foreach ( $combinedChanges[ $key ] as
$changeSet ) {
+ switch ( $changeSet[ 'action' ] ) {
+ case 'add':
+ $output[] = '<div
class="mw-twocolconflict-diffchange-own">' .
+ '<div
class="mw-twocolconflict-diffchange-title">in your version</div>';
+ $output[] =
$changeSet['new'] . '</div>';
+ break;
+ case 'delete':
+ $output[] = '<div
class="mw-twocolconflict-diffchange-foreign">' .
+ '<div
class="mw-twocolconflict-diffchange-title">in ' .
+ $lastUser .
'\'s version</div>';
+ $output[] =
$changeSet['old'] . '</div>';
+ break;
+ case 'copy':
+ $output[] = '<div
class="mw-twocolconflict-diffchange-same">' .
+
$changeSet['copy'] . '</div>';
+ break;
+ }
+ }
+ }
+ }
+
+ return implode( "\n", $output );
+ }
}
diff --git a/modules/ext.TwoColConflict.css b/modules/ext.TwoColConflict.css
index f2bfa06..986e1c3 100644
--- a/modules/ext.TwoColConflict.css
+++ b/modules/ext.TwoColConflict.css
@@ -42,10 +42,14 @@
text-shadow: none;
text-align: start;
overflow: auto;
- padding: 0.1em;
+ padding: 2px;
width: 100%;
height: 400px;
background-color: #f2f2f2;
+}
+
+#mw-twocolconflict-changes-editor div {
+ display: inline-block;
}
#mw-twocolconflict-changes-editor.mw-twocolconflict-wikieditor {
@@ -56,3 +60,41 @@
.mw-twocolconflict-editor-col #wpTextbox1 {
height: 400px;
}
+
+.mw-twocolconflict-lastuser, del.mw-twocolconflict-diffchange {
+ background-color: #add8e6;
+}
+
+.mw-twocolconflict-user, ins.mw-twocolconflict-diffchange {
+ background-color: #ffffe0;
+}
+
+ins.mw-twocolconflict-diffchange, del.mw-twocolconflict-diffchange {
+ text-decoration: none;
+}
+
+.mw-twocolconflict-diffchange-own,
+.mw-twocolconflict-diffchange-foreign,
+.mw-twocolconflict-diffchange-same {
+ margin: 5px 0 3px 0;
+}
+
+.mw-twocolconflict-diffchange-same {
+ background-color: #e2e2e2;
+}
+
+.mw-twocolconflict-diffchange-own .mw-twocolconflict-diffchange-title,
+.mw-twocolconflict-diffchange-foreign .mw-twocolconflict-diffchange-title {
+ color: white;
+ padding: 1px 2px;
+ font-weight: bold;
+ border-radius: 2px;
+}
+
+.mw-twocolconflict-diffchange-own .mw-twocolconflict-diffchange-title {
+ background-color: #ff8c00;
+}
+
+.mw-twocolconflict-diffchange-foreign .mw-twocolconflict-diffchange-title {
+ background-color: #3879ec;
+}
diff --git a/tests/phpunit/includes/LineBasedUnifiedDiffFormatterTest.php
b/tests/phpunit/includes/LineBasedUnifiedDiffFormatterTest.php
new file mode 100644
index 0000000..97b5b73
--- /dev/null
+++ b/tests/phpunit/includes/LineBasedUnifiedDiffFormatterTest.php
@@ -0,0 +1,368 @@
+<?php
+
+class LineBasedUnifiedDiffFormatterTest extends MediaWikiTestCase {
+
+ /**
+ * @param string $before
+ * @param string $after
+ * @param array $expectedOutput
+ * @dataProvider testFormatProvider
+ * @covers LineBasedUnifiedDiffFormatter::format
+ */
+ public function testFormat( $before, $after, $result ) {
+ $diff = new Diff( explode( "\n", $before ), explode( "\n",
$after ) );
+ $instance = new LineBasedUnifiedDiffFormatter();
+ $output = $instance->format( $diff );
+ $this->assertEquals( $result, $output );
+ }
+
+ public function testFormatProvider() {
+ return [
+ [
+ 'before' => 'Just text.',
+ 'after' => 'Just text.',
+ 'result' => [
+ 1 => [
+ [
+ 'action' => 'copy',
+ 'copy' => 'Just text.',
+ 'oldline' => 1,
+ 'newline' => 1
+ ]
+ ]
+ ],
+ ],
+ [
+ 'before' => 'Just text.',
+ 'after' => 'Just text. And more.',
+ 'result' =>
+ [
+ 1 =>
+ [
+ [
+
'action' => 'delete',
+ 'old'
=> 'Just text.',
+
'oldline' => 1
+ ],
+ [
+
'action' => 'add',
+ 'new'
=> 'Just text<ins class="diffchange diffchange-inline">. And more</ins>.',
+
'newline' => 1
+ ]
+ ],
+ ],
+ ],
+ [
+ 'before' => 'Just less text.',
+ 'after' => 'Just less.',
+ 'result' =>
+ [
+ 1 =>
+ [
+ [
+
'action' => 'delete',
+ 'old'
=> 'Just less <del class="diffchange diffchange-inline">text</del>.',
+
'oldline' => 1
+ ],
+ [
+
'action' => 'add',
+ 'new'
=> 'Just less.',
+
'newline' => 1
+ ]
+ ]
+ ],
+ ],
+ [
+ 'before' =>
+<<<TEXT
+Just multi-line text.
+Line number 2.
+TEXT
+ ,
+ 'after' =>
+<<<TEXT
+Just multi-line text.
+Line number 1.5.
+Line number 2.
+TEXT
+ ,
+ 'result' =>
+ [
+ 1 => [
+ [
+ 'action' =>
'copy',
+ 'copy' => 'Just
multi-line text.',
+ 'oldline' => 1,
+ 'newline' => 1
+ ]
+ ],
+ 2 =>
+ [
+ [
+
'action' => 'add',
+ 'new'
=> '<ins class="diffchange diffchange-inline">Line number 1.5.</ins>',
+
'newline' => 3
+ ],
+ [
+
'action' => 'copy',
+ 'copy'
=> 'Line number 2.',
+
'oldline' => 2,
+
'newline' => 3
+ ]
+ ]
+ ],
+ ],
+ [
+ 'before' =>
+<<<TEXT
+Just multi-line text.
+Line number 1.5.
+Line number 2.
+TEXT
+ ,
+ 'after' =>
+<<<TEXT
+Just multi-line text.
+Line number 1.5.
+TEXT
+ ,
+ 'result' =>
+ [
+ 1 => [
+ [
+ 'action' =>
'copy',
+ 'copy' => "Just
multi-line text.\nLine number 1.5.",
+ 'oldline' => 1,
+ 'newline' => 1
+ ]
+ ],
+ 3 =>
+ [
+ [
+
'action' => 'delete',
+ 'old'
=> '<del class="diffchange diffchange-inline">Line number 2.</del>',
+
'oldline' => 3
+ ]
+ ]
+ ],
+ ],
+ [
+ 'before' =>
+<<<TEXT
+Just multi-line text.
+Line number 1.5.
+Line number 2.
+TEXT
+ ,
+ 'after' =>
+<<<TEXT
+Just multi-line test.
+Line number 2.
+Line number 3.
+TEXT
+ ,
+ 'result' =>
+ [
+ 1 =>
+ [
+ [
+
'action' => 'delete',
+ 'old' =>
+<<<TEXT
+Just multi-line <del class="diffchange diffchange-inline">text.</del>
+<del class="diffchange diffchange-inline">Line number 1.5</del>.
+TEXT
+ ,
+
'oldline' => 1
+ ],
+ [
+
'action' => 'add',
+ 'new'
=> 'Just multi-line <ins class="diffchange diffchange-inline">test</ins>.',
+
'newline' => 1
+ ]
+ ],
+ 3 => [
+ [
+ 'action' =>
'copy',
+ 'copy' => 'Line
number 2.',
+ 'oldline' => 3,
+ 'newline' => 2
+ ]
+ ],
+ 4 =>
+ [
+ [
+
'action' => 'add',
+ 'new'
=> '<ins class="diffchange diffchange-inline">Line number 3.</ins>',
+
'newline' => 3
+
+ ]
+ ],
+ ],
+ ],
+ [
+ 'before' =>
+<<<TEXT
+Just multi-line text.
+To change number 2.
+To change number 3.
+TEXT
+ ,
+ 'after' =>
+<<<TEXT
+Just multi-line test.
+Line number 2 changed.
+Line number 3 also changed.
+TEXT
+ ,
+ 'result' =>
+ [
+ 1 =>
+ [
+ [
+
'action' => 'delete',
+ 'old' =>
+<<<TEXT
+<del class="diffchange diffchange-inline">Just multi-line text.
+To change number 2.
+To change number 3.</del>
+TEXT
+ ,
+
'oldline' => 1
+ ], ],
+ 4 =>
+ [
+ [
+
'action' => 'add',
+ 'new' =>
+<<<TEXT
+<ins class="diffchange diffchange-inline">Just multi-line test.
+Line number 2 changed.
+Line number 3 also changed.</ins>
+TEXT
+ ,
+
'newline' => 1
+ ]
+ ],
+ ],
+ ],
+ [
+ 'before' =>
+<<<TEXT
+Just a multi-line text.
+Line number two. This line is quite long!
+And that's line number three - even longer than the line before.
+
+Just another line with an empty line above.
+TEXT
+ ,
+ 'after' =>
+<<<TEXT
+Just a multi-line text.
+Add something new.
+Line number two. Now line number three and quite long!
+Add more new stuff.
+TEXT
+ ,
+ 'result' =>
+ [
+ 1 => [
+ [
+ 'action' =>
'copy',
+ 'copy' => 'Just
a multi-line text.',
+ 'oldline' => 1,
+ 'newline' => 1
+ ]
+ ],
+ 2 =>
+ [
+ [
+
'action' => 'delete',
+ 'old' =>
+<<<TEXT
+<del class="diffchange diffchange-inline">Line number two. This line is quite
long!
+And that's line number three - even longer than the line before.
+ 
+Just another line with an empty line above.</del>
+TEXT
+ ,
+
'oldline' => 2
+ ], ],
+ 6 =>
+ [
+ [
+
'action' => 'add',
+ 'new' =>
+<<<TEXT
+<ins class="diffchange diffchange-inline">Add something new.
+Line number two. Now line number three and quite long!
+Add more new stuff.</ins>
+TEXT
+ ,
+
'newline' => 2
+ ]
+ ],
+ ],
+ ],
+ [
+ 'before' =>
+<<<TEXT
+Just a multi-line text.
+Line number two. This line is quite long!
+Line number three.
+TEXT
+ ,
+ 'after' =>
+<<<TEXT
+Just a multi-line text.
+Line number two. This line is now a bit longer!
+
+And it gets even longer.
+
+Line number three.
+TEXT
+ ,
+ 'result' =>
+ [
+ 1 => [
+ [
+ 'action' =>
'copy',
+ 'copy' => 'Just
a multi-line text.',
+ 'oldline' => 1,
+ 'newline' => 1
+ ]
+ ],
+ 2 =>
+ [
+ [
+
'action' => 'delete',
+ 'old'
=> 'Line number two. This line is
+<del class="diffchange diffchange-inline">quite long</del>!',
+
'oldline' => 2
+ ],
+ [
+
'action' => 'add',
+ 'new' =>
+<<<TEXT
+Line number two. This line is <ins class="diffchange diffchange-inline">now a
bit longer</ins>!
+ 
+<ins class="diffchange diffchange-inline">And it gets even longer.</ins>
+ 
+TEXT
+ ,
+
'newline' => 2
+ ],
+ ],
+ 3 => [
+ [
+ 'action' =>
'copy',
+ 'copy' => 'Line
number three.',
+ 'oldline' => 3,
+ 'newline' => 6
+ ]
+ ],
+ ],
+ ],
+ ];
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/322128
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib0c9911fe9f1e1e633df55183f1dd1c9765fd6c0
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/TwoColConflict
Gerrit-Branch: master
Gerrit-Owner: WMDE-Fisch <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
