Closes #136 by preventing infinite scroll loop when sync scroll active

Author: mupchrch

CHANGELOG.md

@@ -1,3 +1,6 @@
+## 1.5.2 - 2017-12-29
+* Fixed infinite scroll loop when performing a next/prev diff command #136
+
 ## 1.5.1 - 2017-08-29
 * Fixed selected line losing its background color with some themes - thanks skylerlee!
 

lib/diff-view.js

@@ -93,8 +93,9 @@ module.exports = class DiffView {
 
   /**
    * Called to move the current selection highlight to the next diff chunk.
+   * @param isSyncScrollEnabled Only autoscroll one editor if sync scroll is enabled or we will get in an infinite loop
    */
-  nextDiff() {
+  nextDiff(isSyncScrollEnabled) {
     if(this._isSelectionActive) {
       this._selectedChunkIndex++;
       if(this._selectedChunkIndex >= this.getNumDifferences()) {
@@ -104,7 +105,7 @@ module.exports = class DiffView {
       this._isSelectionActive = true;
     }
 
-    var success = this._selectChunk(this._selectedChunkIndex, true);
+    var success = this._selectChunk(this._selectedChunkIndex, true, isSyncScrollEnabled);
     if(!success) {
         return -1;
     }
@@ -114,8 +115,9 @@ module.exports = class DiffView {
 
   /**
    * Called to move the current selection highlight to the previous diff chunk.
+   * @param isSyncScrollEnabled Only autoscroll one editor if sync scroll is enabled or we will get in an infinite loop
    */
-  prevDiff() {
+  prevDiff(isSyncScrollEnabled) {
     if(this._isSelectionActive) {
       this._selectedChunkIndex--;
       if(this._selectedChunkIndex < 0) {
@@ -125,7 +127,7 @@ module.exports = class DiffView {
       this._isSelectionActive = true;
     }
 
-    var success = this._selectChunk(this._selectedChunkIndex, true);
+    var success = this._selectChunk(this._selectedChunkIndex, true, isSyncScrollEnabled);
     if(!success) {
         return -1;
     }
@@ -218,6 +220,10 @@ module.exports = class DiffView {
     }
   }
 
+  /**
+   * Restores soft wrap to the appropriate editor.
+   * @param editorIndex The index of the editor to restore soft wrap to.
+   */
   restoreEditorSoftWrap(editorIndex) {
     if(editorIndex === 1) {
       this._editorDiffExtender1.getEditor().setSoftWrapped(true);
@@ -243,10 +249,20 @@ module.exports = class DiffView {
     return Array.isArray(this._chunks) ? this._chunks.length : 0;
   }
 
+  /**
+   * Gets the marker layers in use by the editors.
+   * @return An object containing the marker layers and approriate information.
+   */
   getMarkerLayers() {
     return this._markerLayers;
   }
 
+  /**
+   * Handles when the cursor moves in the editor. Will highlight chunks that have a cursor in them.
+   * @param cursor The cursor object from the event.
+   * @param oldBufferPosition The old position of the cursor in the buffer.
+   * @param newBufferPosition The new position of the cursor in the buffer.
+   */
   handleCursorChange(cursor, oldBufferPosition, newBufferPosition) {
     var editorIndex = (cursor.editor === this._editorDiffExtender1.getEditor()) ? 1 : 2;
     var oldPositionChunkIndex = this._getChunkIndexByLineNumber(editorIndex, oldBufferPosition.row);
@@ -272,8 +288,10 @@ module.exports = class DiffView {
    * given index.
    *
    * @param index The index of the diff chunk to highlight in both editors.
+   * @param isNextOrPrev Whether we are moving to a direct sibling (if not, this is a click)
+   * @param isSyncScrollEnabled Only autoscroll one editor if sync scroll is enabled or we will get in an infinite loop
    */
-  _selectChunk(index, isNextOrPrev) {
+  _selectChunk(index, isNextOrPrev, isSyncScrollEnabled) {
     var diffChunk = this._chunks[index];
     if(diffChunk != null) {
       diffChunk.isSelected = true;
@@ -284,7 +302,7 @@ module.exports = class DiffView {
         this._editorDiffExtender2.deselectAllLines();
         // scroll the editors
         this._editorDiffExtender1.getEditor().setCursorBufferPosition([diffChunk.oldLineStart, 0], {autoscroll: true});
-        this._editorDiffExtender2.getEditor().setCursorBufferPosition([diffChunk.newLineStart, 0], {autoscroll: true});
+        this._editorDiffExtender2.getEditor().setCursorBufferPosition([diffChunk.newLineStart, 0], {autoscroll: !isSyncScrollEnabled});
       }
 
       // highlight selection in both editors
@@ -297,6 +315,12 @@ module.exports = class DiffView {
     return false;
   }
 
+  /**
+   * Gets the index of a chunk by the line number.
+   * @param editorIndex The index of the editor to check.
+   * @param lineNumber  The line number to use to check if it is in a chunk.
+   * @return The index of the chunk.
+   */
   _getChunkIndexByLineNumber(editorIndex, lineNumber) {
     for(var i=0; i<this._chunks.length; i++) {
       var diffChunk = this._chunks[i];

lib/split-diff.coffee

@@ -154,13 +154,21 @@ module.exports = SplitDiff =
   # called by "Move to next diff" command
   nextDiff: ->
     if @diffView?
-      selectedIndex = @diffView.nextDiff()
+      isSyncScrollEnabled = false
+      scrollSyncType = @options.scrollSyncType ? @_getConfig('scrollSyncType')
+      if scrollSyncType == 'Vertical + Horizontal' || scrollSyncType == 'Vertical'
+        isSyncScrollEnabled = true
+      selectedIndex = @diffView.nextDiff(isSyncScrollEnabled)
       @footerView?.showSelectionCount( selectedIndex + 1 )
 
   # called by "Move to previous diff" command
   prevDiff: ->
     if @diffView?
-      selectedIndex = @diffView.prevDiff()
+      isSyncScrollEnabled = false
+      scrollSyncType = @options.scrollSyncType ? @_getConfig('scrollSyncType')
+      if scrollSyncType == 'Vertical + Horizontal' || scrollSyncType == 'Vertical'
+        isSyncScrollEnabled = true
+      selectedIndex = @diffView.prevDiff(isSyncScrollEnabled)
       @footerView?.showSelectionCount( selectedIndex + 1 )
 
   # called by "Copy to right" command

package.json

@@ -25,7 +25,7 @@
   "repository": "https://github.com/mupchrch/split-diff",
   "license": "MIT",
   "engines": {
-    "atom": ">= 1.17 < 2.0.0"
+    "atom": ">= 1.19 < 2.0.0"
   },
   "dependencies": {
     "diff": "latest"