[MediaWiki-commits] [Gerrit] Alpha: Rewrite makePrettyDiff function - change (mediawiki...MobileFrontend)

[JGonera (Code Review)]

JGonera has submitted this change and it was merged.

Change subject: Alpha: Rewrite makePrettyDiff function
..


Alpha: Rewrite makePrettyDiff function

Add test case current function doesn't satisfy
Rewrite and simplify function

Change-Id: Iedf7643307247b6672ee63351dbd58a7431e98cc
---
M javascripts/specials/mobilediff.js
M tests/javascripts/specials/test_mobilediff.js
2 files changed, 31 insertions(+), 39 deletions(-)

Approvals:
  JGonera: Verified; Looks good to me, approved
  jenkins-bot: Checked



diff --git a/javascripts/specials/mobilediff.js 
b/javascripts/specials/mobilediff.js
index 1c70966..2e21156 100644
--- a/javascripts/specials/mobilediff.js
+++ b/javascripts/specials/mobilediff.js
@@ -1,51 +1,39 @@
 ( function( $, M ) {
 
function makePrettyDiff( $diff ) {
-   var $diffclone = $diff.clone();
+   var $diffclone = $diff.clone(), before = '', after = '',
+   diff;
 
-   // simple case where just an insert or just a delete
-   if ( $diff.children( 'ins' ).length === 0 || $diff.children( 
'del' ).length === 0 ) {
-   $diff.empty().addClass( 'prettyDiff' );
-   $diffclone.find( 'del,ins' ).each( function() {
-   var $el = $( this ).clone();
-   if ( $el.text() ) { // don't add empty elements
-   $el.appendTo( $diff );
-   $( 'br' ).appendTo( $diff );
-   }
-   } );
-   return $diff;
-   } else if ( $diff.children().length  1 ) { // if there is only 
one line it is not a complicated diff
-   $diff.empty().addClass( 'prettyDiff' );
-   $diffclone.find( 'ins' ).each( function() {
-   var $del = $( this ).prev( 'del' ), $add = $( 
this ),
-   diffChars;
+   $diffclone.find( 'del,ins' ).each( function() {
+   if ( this.tagName === 'DEL' ) {
+   before += $( this ).text() + 'br';
+   } else {
+   after += $( this ).text() + 'br';
+   }
+   } );
+   $diff.empty().addClass( 'prettyDiff' );
 
-   if ( $del.length  0 ) {
-   while ( $add.next()[ 0 ]  
$add.next()[ 0 ].tagName === 'INS' ) {
-   $add.clone().appendTo( $diff );
+   diff = JsDiff.diffWords( before, after );
+   diff.forEach( function( change ) {
+   var tag, vals;
+   if ( change.added ) {
+   tag = 'ins';
+   } else if ( change.removed ) {
+   tag = 'del';
+   } else {
+   tag = 'span';
+   }
+   vals = change.value.split( 'br' );
+   vals.forEach( function( val ) {
+   if ( val ) {
+   $( tag ).text( val ).appendTo( $diff );
+   if ( vals.length  1 ) {
$( 'br' ).appendTo( $diff );
-   $add = $add.next();
}
-
-   diffChars = JsDiff.diffWords( 
$del.text(), $add.text() );
-   diffChars.forEach( function( change ) {
-   var tag;
-   if ( change.added ) {
-   tag = 'ins';
-   } else if ( change.removed ) {
-   tag = 'del';
-   } else {
-   tag = 'span';
-   }
-   $( tag ).text( change.value 
).appendTo( $diff );
-   } );
-   } else if ( $add.prev()[ 0 ].tagName !== 'INS' 
){
-   $add.clone().css( 'display', 'inline' 
).appendTo( $diff );
}
-
-   $( 'br' ).appendTo( $diff );
} );
-   }
+   } );
+
return $diff;
}
 
diff --git a/tests/javascripts/specials/test_mobilediff.js 
b/tests/javascripts/specials/test_mobilediff.js

[MediaWiki-commits] [Gerrit] Alpha: Rewrite makePrettyDiff function - change (mediawiki...MobileFrontend)

[Jdlrobson (Code Review)]

Jdlrobson has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/60816


Change subject: Alpha: Rewrite makePrettyDiff function
..

Alpha: Rewrite makePrettyDiff function

Add test case current function doesn't satisfy
Rewrite and simplify function

Change-Id: Iedf7643307247b6672ee63351dbd58a7431e98cc
---
M javascripts/specials/mobilediff.js
M tests/javascripts/specials/test_mobilediff.js
2 files changed, 31 insertions(+), 39 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend 
refs/changes/16/60816/1

diff --git a/javascripts/specials/mobilediff.js 
b/javascripts/specials/mobilediff.js
index 1c70966..2e21156 100644
--- a/javascripts/specials/mobilediff.js
+++ b/javascripts/specials/mobilediff.js
@@ -1,51 +1,39 @@
 ( function( $, M ) {
 
function makePrettyDiff( $diff ) {
-   var $diffclone = $diff.clone();
+   var $diffclone = $diff.clone(), before = '', after = '',
+   diff;
 
-   // simple case where just an insert or just a delete
-   if ( $diff.children( 'ins' ).length === 0 || $diff.children( 
'del' ).length === 0 ) {
-   $diff.empty().addClass( 'prettyDiff' );
-   $diffclone.find( 'del,ins' ).each( function() {
-   var $el = $( this ).clone();
-   if ( $el.text() ) { // don't add empty elements
-   $el.appendTo( $diff );
-   $( 'br' ).appendTo( $diff );
-   }
-   } );
-   return $diff;
-   } else if ( $diff.children().length  1 ) { // if there is only 
one line it is not a complicated diff
-   $diff.empty().addClass( 'prettyDiff' );
-   $diffclone.find( 'ins' ).each( function() {
-   var $del = $( this ).prev( 'del' ), $add = $( 
this ),
-   diffChars;
+   $diffclone.find( 'del,ins' ).each( function() {
+   if ( this.tagName === 'DEL' ) {
+   before += $( this ).text() + 'br';
+   } else {
+   after += $( this ).text() + 'br';
+   }
+   } );
+   $diff.empty().addClass( 'prettyDiff' );
 
-   if ( $del.length  0 ) {
-   while ( $add.next()[ 0 ]  
$add.next()[ 0 ].tagName === 'INS' ) {
-   $add.clone().appendTo( $diff );
+   diff = JsDiff.diffWords( before, after );
+   diff.forEach( function( change ) {
+   var tag, vals;
+   if ( change.added ) {
+   tag = 'ins';
+   } else if ( change.removed ) {
+   tag = 'del';
+   } else {
+   tag = 'span';
+   }
+   vals = change.value.split( 'br' );
+   vals.forEach( function( val ) {
+   if ( val ) {
+   $( tag ).text( val ).appendTo( $diff );
+   if ( vals.length  1 ) {
$( 'br' ).appendTo( $diff );
-   $add = $add.next();
}
-
-   diffChars = JsDiff.diffWords( 
$del.text(), $add.text() );
-   diffChars.forEach( function( change ) {
-   var tag;
-   if ( change.added ) {
-   tag = 'ins';
-   } else if ( change.removed ) {
-   tag = 'del';
-   } else {
-   tag = 'span';
-   }
-   $( tag ).text( change.value 
).appendTo( $diff );
-   } );
-   } else if ( $add.prev()[ 0 ].tagName !== 'INS' 
){
-   $add.clone().css( 'display', 'inline' 
).appendTo( $diff );
}
-
-   $( 'br' ).appendTo( $diff );
} );
-   }
+   } );
+
return $diff;
}
 
diff --git