[MediaWiki-commits] [Gerrit] mediawiki...CodeMirror[master]: Rewrite VE<->CM synchronizer using transaction op walking

2017-07-07 Thread jenkins-bot (Code Review)
jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/363644 )

Change subject: Rewrite VE<->CM synchronizer using transaction op walking
..


Rewrite VE<->CM synchronizer using transaction op walking

Previous hacky method fell apart with multi-line transactions.

Depends-On: I465a3f6a8afcd6536293999eb40c01daeb9d905b
Depends-On: I800085eb3a5f7332eab356b62a34bfc603a29839
Change-Id: I66946d7e3f425d8dbbdae720756a7978fbe25582
---
M resources/modules/ve-cm/ve.ui.CodeMirrorAction.js
1 file changed, 50 insertions(+), 32 deletions(-)

Approvals:
  Catrope: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/resources/modules/ve-cm/ve.ui.CodeMirrorAction.js 
b/resources/modules/ve-cm/ve.ui.CodeMirrorAction.js
index effe1de..14d49d7 100644
--- a/resources/modules/ve-cm/ve.ui.CodeMirrorAction.js
+++ b/resources/modules/ve-cm/ve.ui.CodeMirrorAction.js
@@ -65,11 +65,11 @@
 
// As the action is regenerated each time, we need to store the 
bound listener
// in the mirror for later disconnection.
-   surface.mirror.veTransactionListener = 
this.onDocumentTransact.bind( this, surface );
+   surface.mirror.veTransactionListener = 
this.onDocumentPrecommit.bind( this );
 
-   doc.on( 'transact', surface.mirror.veTransactionListener );
+   doc.on( 'precommit', surface.mirror.veTransactionListener );
} else if ( surface.mirror && enable !== true ) {
-   doc.off( 'transact', surface.mirror.veTransactionListener );
+   doc.off( 'precommit', surface.mirror.veTransactionListener );
 
surfaceView.$documentNode.removeClass(
've-ce-documentNode-codeEditor-webkit-hide 
ve-ce-documentNode-codeEditor-webkit'
@@ -83,37 +83,55 @@
return true;
 };
 
-ve.ui.CodeMirrorAction.prototype.onDocumentTransact = function ( surface, tx ) 
{
-   var node, textRange, line,
-   doc = surface.getModel().getDocument(),
-   mirror = surface.mirror,
-   modifiedRange = tx.getModifiedRange( doc ),
-   nodes = doc.selectNodes( modifiedRange, 'leaves' );
+/**
+ * Handle precommit events from the document.
+ *
+ * The document is still in it's 'old' state before the transaction
+ * has been applied at this point.
+ *
+ * @param {ve.dm.Transaction} tx [description]
+ */
+ve.ui.CodeMirrorAction.prototype.onDocumentPrecommit = function ( tx ) {
+   var i,
+   offset = 0,
+   replacements = [],
+   linearData = this.surface.getModel().getDocument().data,
+   store = linearData.getStore(),
+   mirror = this.surface.mirror;
 
-   // TODO: Iterate over operations and perform a replaceRange for each 
replace operation
-   if ( nodes.length === 1 && nodes[ 0 ].node instanceof ve.dm.TextNode ) {
-   node = nodes[ 0 ].node.parent;
-   textRange = nodes[ 0 ].nodeRange;
-   line = node.parent.children.indexOf( node );
-   if ( tx.operations.every( function ( op ) {
-   return op.type === 'retain' || ( op.type === 'replace' 
&& op.remove.length === 0 );
-   } ) ) {
-   // Single line insert
-   mirror.replaceRange(
-   doc.data.getText( true, modifiedRange ),
-   { line: line, ch: modifiedRange.start - 
textRange.start }
-   );
-   } else {
-   // Single line replace
-   mirror.replaceRange(
-   doc.data.getText( true, textRange ),
-   { line: line, ch: 0 },
-   { line: line, ch: mirror.getLine( line ).length 
}
-   );
+   /**
+* Convert a VE offset to a 2D CodeMirror position
+*
+* @private
+* @param {Number} veOffset VE linear model offset
+* @return {Object} Code mirror position, containing 'line' and 'ch'
+*/
+   function convertOffset( veOffset ) {
+   var cmOffset = linearData.getSourceText( new ve.Range( 0, 
veOffset ) ).length;
+   return mirror.posFromIndex( cmOffset );
+   }
+
+   tx.operations.forEach( function ( op ) {
+   if ( op.type === 'retain' ) {
+   offset += op.length;
+   } else if ( op.type === 'replace' ) {
+   replacements.push( {
+   start: convertOffset( offset ),
+   // Don't bother recalculating end offset if not 
a removal, replaceRange works with just one arg
+   end: op.remove.length ? convertOffset( offset + 
op.remove.length ) : undefined,
+   

[MediaWiki-commits] [Gerrit] mediawiki...CodeMirror[master]: Rewrite VE<->CM synchronizer using transaction op walking

2017-07-06 Thread Esanders (Code Review)
Esanders has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/363644 )

Change subject: Rewrite VE<->CM synchronizer using transaction op walking
..

Rewrite VE<->CM synchronizer using transaction op walking

Previous hacky method fell apart with multi-line transactions.

Depends-On: I465a3f6a8afcd6536293999eb40c01daeb9d905b
Change-Id: I66946d7e3f425d8dbbdae720756a7978fbe25582
---
M resources/modules/ve-cm/ve.ui.CodeMirrorAction.js
1 file changed, 50 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CodeMirror 
refs/changes/44/363644/1

diff --git a/resources/modules/ve-cm/ve.ui.CodeMirrorAction.js 
b/resources/modules/ve-cm/ve.ui.CodeMirrorAction.js
index effe1de..14d49d7 100644
--- a/resources/modules/ve-cm/ve.ui.CodeMirrorAction.js
+++ b/resources/modules/ve-cm/ve.ui.CodeMirrorAction.js
@@ -65,11 +65,11 @@
 
// As the action is regenerated each time, we need to store the 
bound listener
// in the mirror for later disconnection.
-   surface.mirror.veTransactionListener = 
this.onDocumentTransact.bind( this, surface );
+   surface.mirror.veTransactionListener = 
this.onDocumentPrecommit.bind( this );
 
-   doc.on( 'transact', surface.mirror.veTransactionListener );
+   doc.on( 'precommit', surface.mirror.veTransactionListener );
} else if ( surface.mirror && enable !== true ) {
-   doc.off( 'transact', surface.mirror.veTransactionListener );
+   doc.off( 'precommit', surface.mirror.veTransactionListener );
 
surfaceView.$documentNode.removeClass(
've-ce-documentNode-codeEditor-webkit-hide 
ve-ce-documentNode-codeEditor-webkit'
@@ -83,37 +83,55 @@
return true;
 };
 
-ve.ui.CodeMirrorAction.prototype.onDocumentTransact = function ( surface, tx ) 
{
-   var node, textRange, line,
-   doc = surface.getModel().getDocument(),
-   mirror = surface.mirror,
-   modifiedRange = tx.getModifiedRange( doc ),
-   nodes = doc.selectNodes( modifiedRange, 'leaves' );
+/**
+ * Handle precommit events from the document.
+ *
+ * The document is still in it's 'old' state before the transaction
+ * has been applied at this point.
+ *
+ * @param {ve.dm.Transaction} tx [description]
+ */
+ve.ui.CodeMirrorAction.prototype.onDocumentPrecommit = function ( tx ) {
+   var i,
+   offset = 0,
+   replacements = [],
+   linearData = this.surface.getModel().getDocument().data,
+   store = linearData.getStore(),
+   mirror = this.surface.mirror;
 
-   // TODO: Iterate over operations and perform a replaceRange for each 
replace operation
-   if ( nodes.length === 1 && nodes[ 0 ].node instanceof ve.dm.TextNode ) {
-   node = nodes[ 0 ].node.parent;
-   textRange = nodes[ 0 ].nodeRange;
-   line = node.parent.children.indexOf( node );
-   if ( tx.operations.every( function ( op ) {
-   return op.type === 'retain' || ( op.type === 'replace' 
&& op.remove.length === 0 );
-   } ) ) {
-   // Single line insert
-   mirror.replaceRange(
-   doc.data.getText( true, modifiedRange ),
-   { line: line, ch: modifiedRange.start - 
textRange.start }
-   );
-   } else {
-   // Single line replace
-   mirror.replaceRange(
-   doc.data.getText( true, textRange ),
-   { line: line, ch: 0 },
-   { line: line, ch: mirror.getLine( line ).length 
}
-   );
+   /**
+* Convert a VE offset to a 2D CodeMirror position
+*
+* @private
+* @param {Number} veOffset VE linear model offset
+* @return {Object} Code mirror position, containing 'line' and 'ch'
+*/
+   function convertOffset( veOffset ) {
+   var cmOffset = linearData.getSourceText( new ve.Range( 0, 
veOffset ) ).length;
+   return mirror.posFromIndex( cmOffset );
+   }
+
+   tx.operations.forEach( function ( op ) {
+   if ( op.type === 'retain' ) {
+   offset += op.length;
+   } else if ( op.type === 'replace' ) {
+   replacements.push( {
+   start: convertOffset( offset ),
+   // Don't bother recalculating end offset if not 
a removal, replaceRange works with just one arg
+   end: op.remove.length ? convertOffset( offset + 
op.remove.length ) : undefined,
+   data: new