Fix smart mode bugs

package.json

@@ -75,13 +75,14 @@
     ]
   },
   "scripts": {
-    "test": "standard src/*.js"
+    "test": "standard && mocha"
   },
   "dependencies": {
+    "diff": "3.5.0",
     "parinfer": "3.12.0"
   },
   "devDependencies": {
+    "mocha": "5.2.0",
     "standard": "11.0.1"
   }
-
 }

src/extension.js

@@ -2,100 +2,97 @@
 // Requires
 // -----------------------------------------------------------------------------
 
+const diff = require('diff')
+const fs = require('fs')
 const vscode = require('vscode')
 const window = vscode.window
-
 const statusBar = require('./statusbar')
-
 const editorModule = require('./editor')
 const editorStates = editorModule.editorStates
-
 const parenTrailsModule = require('./parentrails')
 const clearParenTrailDecorators = parenTrailsModule.clearParenTrailDecorators
-
 const parinfer2 = require('./parinfer')
-const util = require('./util')
-
 const config = require('./config')
-
 const path = require('path')
-const fs = require('fs')
+const state = require('./state')
+const util = require('./util')
+const isString = util.isString
+const debounce = util.debounce
 
 // -----------------------------------------------------------------------------
 // Constants
 // -----------------------------------------------------------------------------
 
 const documentChangeEvent = 'DOCUMENT_CHANGE'
 const selectionChangeEvent = 'SELECTION_CHANGE'
-const fiveSecondsMs = 5 * 1000
 
 // -----------------------------------------------------------------------------
 // Events Queue
 // -----------------------------------------------------------------------------
 
 // TODO: need documentation for how the eventsQueue works
 let eventsQueue = []
-
-const eventsQueueMaxLength = 10
-
-function cleanUpEventsQueue () {
-  if (eventsQueue.length > eventsQueueMaxLength) {
-    eventsQueue.length = eventsQueueMaxLength
-  }
-}
-
-setInterval(cleanUpEventsQueue, fiveSecondsMs)
+let prevCursorLine = null
+let prevCursorX = null
 
 const logEventsQueue = false
+const logTextChange = false
 
 function processEventsQueue () {
-  // defensive: these should never happen
+  // do nothing if the queue is empty (this should never happen)
   if (eventsQueue.length === 0) return
-  if (eventsQueue[0].type !== selectionChangeEvent) return
 
   const activeEditor = window.activeTextEditor
   const editorMode = editorStates.deref().get(activeEditor)
 
   // exit if we are not in Smart, Indent, or Paren mode
   if (!util.isRunState(editorMode)) return
 
-  if (logEventsQueue) {
-    if (eventsQueue[2]) console.log('2: ', eventsQueue[2])
-    if (eventsQueue[1]) console.log('1: ', eventsQueue[1])
-    console.log('0: ', eventsQueue[0])
-    console.log('~~~~~~~~~~~~~~~~ eventsQueue ~~~~~~~~~~~~~~~~')
-  }
-
-  const txt = eventsQueue[0].txt
-
-  // cursor options
+  // create the Parinfer options object
   let options = {
     cursorLine: eventsQueue[0].cursorLine,
     cursorX: eventsQueue[0].cursorX
   }
 
-  // TODO: this is not working correctly
-  // // add selectionStartLine if applicable
-  // if (eventsQueue[0].selectionStartLine) {
-  //   options.selectionStartLine = eventsQueue[0].selectionStartLine
-  // }
-
-  // check the last two events for previous cursor information
-  if (eventsQueue[1] && eventsQueue[1].cursorLine) {
-    options.prevCursorLine = eventsQueue[1].cursorLine
-    options.prevCursorX = eventsQueue[1].cursorX
-  } else if (eventsQueue[2] && eventsQueue[2].cursorLine) {
-    options.prevCursorLine = eventsQueue[2].cursorLine
-    options.prevCursorX = eventsQueue[2].cursorX
+  // text + changes
+  const currentTxt = eventsQueue[0].txt
+  const didTextChange = currentTxt !== state.prevTxt
+
+  if (logTextChange && didTextChange) {
+    console.log(state.prevTxt)
+    console.log('+++')
+    console.log(currentTxt)
+    console.log('~~~~~~~~~~~~~~~~ prev / current text ~~~~~~~~~~~~~~~~')
+  }
+
+  options.changes = null
+  if (isString(state.prevTxt) && didTextChange) {
+    options.changes = calculateChanges(state.prevTxt, currentTxt)
   }
 
-  // "document change" events always fire first followed immediately by a "selection change" event
-  // try to grab the changes from the most recent document change event
-  if (eventsQueue[1] && eventsQueue[1].type === documentChangeEvent && eventsQueue[1].changes) {
-    options.changes = eventsQueue[1].changes
+  // previous cursor information
+  options.prevCursorLine = prevCursorLine
+  options.prevCursorX = prevCursorX
+  prevCursorLine = options.cursorLine
+  prevCursorX = options.cursorX
+
+  if (logEventsQueue) {
+    console.log(JSON.stringify(eventsQueue, null, 2))
+    console.log('Parinfer options: ' + JSON.stringify(options))
+    console.log('~~~~~~~~~~~~~~~~ eventsQueue ~~~~~~~~~~~~~~~~')
   }
 
-  parinfer2.applyParinfer(activeEditor, txt, options)
+  // clear out the event queue
+  eventsQueue.length = 0
+  parinfer2.applyParinfer(activeEditor, currentTxt, options)
+}
+
+const processQueueDebounceIntervalMs = 20
+const debouncedProcessEventsQueue = debounce(processEventsQueue, processQueueDebounceIntervalMs)
+
+function calculateChanges (oldTxt, newTxt) {
+  const theDiff = diff.diffChars(oldTxt, newTxt)
+  return util.diffChangesToParinferChanges(theDiff)
 }
 
 // -----------------------------------------------------------------------------
@@ -114,7 +111,11 @@ function onChangeEditorStates (states) {
     statusBar.updateStatusBar(currentEditorState)
     if (util.isRunState(currentEditorState)) {
       const txt = activeEditor.document.getText()
-      parinfer2.applyParinfer(activeEditor, txt, {})
+      const options = {
+        cursorLine: activeEditor.selection.active.line,
+        cursorX: activeEditor.selection.active.character
+      }
+      parinfer2.applyParinfer(activeEditor, txt, options)
     } else {
       clearParenTrailDecorators(activeEditor)
     }
@@ -131,68 +132,61 @@ editorStates.addWatch(onChangeEditorStates)
 // -----------------------------------------------------------------------------
 
 function onChangeActiveEditor (editor) {
-  // clear out the eventsQueue when we switch editor tabs
-  eventsQueue = []
+  // clear out the state when we switch the active editor
+  eventsQueue.length = 0
+  state.ignoreDocumentVersion = null
+  prevCursorLine = null
+  prevCursorX = null
+  state.prevTxt = editor.document.getText()
 
   if (editor) {
     parinfer2.helloEditor(editor)
   }
 }
 
-// convert VS Code change object to the format Parinfer expects
-function convertChangeObjects (oldTxt, changeEvt) {
-  return {
-    lineNo: changeEvt.range.start.line,
-    newText: changeEvt.text,
-    oldText: util.getTextFromRange(oldTxt, changeEvt.range, changeEvt.rangeLength),
-    x: changeEvt.range.start.character
-  }
-}
-
+// this function fires any time a document's content is changed
 function onChangeTextDocument (evt) {
   // drop any events that do not contain document changes
-  // (usually the first event to a document)
+  // NOTE: this is usually the first change to a document and anytime the user presses "save"
   if (evt.contentChanges && evt.contentChanges.length === 0) {
     return
   }
 
+  // ignore edits that were made by Parinfer
+  if (state.ignoreDocumentVersion === evt.document.version) {
+    return
+  }
+
+  const activeEditor = window.activeTextEditor
   let parinferEvent = {
+    cursorLine: activeEditor.selection.active.line,
+    cursorX: activeEditor.selection.active.character,
+    documentVersion: evt.document.version,
     txt: evt.document.getText(),
     type: documentChangeEvent
   }
 
-  // only create a "changes" property if we have a prior event
-  if (eventsQueue[0] && eventsQueue[0].txt) {
-    const prevTxt = eventsQueue[0].txt
-    const convertFn = convertChangeObjects.bind(null, prevTxt)
-    parinferEvent.changes = evt.contentChanges.map(convertFn)
-  }
-
-  // put this event on the queue
+  // put this event on the queue and schedule a processing
   eventsQueue.unshift(parinferEvent)
+  debouncedProcessEventsQueue()
 }
 
+// this function fires any time a cursor's position changes (ie: often)
 function onChangeSelection (evt) {
+  // TODO: ignore events that were not made by a mouse or keyboard?
+
   const editor = evt.textEditor
-  const selection = evt.selections[0]
-  let parinferEvent = {
-    cursorLine: selection.active.line,
-    cursorX: selection.active.character,
+  const parinferEvent = {
+    cursorLine: editor.selection.active.line,
+    cursorX: editor.selection.active.character,
+    documentVersion: editor.document.version,
     txt: editor.document.getText(),
     type: selectionChangeEvent
   }
 
-  // TODO: this is not working correctly
-  // // add selectionStartLine if applicable
-  // if (selection && !selection.isEmpty) {
-  //   parinferEvent.selectionStartLine = selection.start.line
-  // }
-
-  // put this event on the queue
+  // put this event on the queue and schedule a processing
   eventsQueue.unshift(parinferEvent)
-
-  // process the queue after every "selection change" event
-  processEventsQueue()
+  debouncedProcessEventsQueue()
 }
 
 // -----------------------------------------------------------------------------

src/parinfer.js

@@ -1,38 +1,33 @@
 const vscode = require('vscode')
 const Position = vscode.Position
 const Selection = vscode.Selection
-
 const window = vscode.window
 const workspace = vscode.workspace
-
 const parinfer = require('parinfer')
-
 const parenTrailsModule = require('./parentrails')
 const updateParenTrails = parenTrailsModule.updateParenTrails
-
 const editorModule = require('./editor')
 const editorStates = editorModule.editorStates
-
 const util = require('./util')
-
 const messages = require('./messages')
 const parenModeFailedMsg = messages.parenModeFailedMsg
 const parenModeChangedFileMsg = messages.parenModeChangedFileMsg
-
 const config = require('./config')
+const state = require('./state')
 
 // -----------------------------------------------------------------------------
 // Parinfer Application
 // -----------------------------------------------------------------------------
 
+const undoOptions = {
+  undoStopAfter: false,
+  undoStopBefore: false
+}
+
 const logParinferInput = false
 const logParinferOutput = false
 
 function applyParinfer2 (editor, inputText, opts, mode) {
-  if (!opts) {
-    opts = {}
-  }
-
   if (logParinferInput) {
     console.log(inputText)
     console.log(opts)
@@ -45,6 +40,9 @@ function applyParinfer2 (editor, inputText, opts, mode) {
   else if (mode === 'SMART_MODE') result = parinfer.smartMode(inputText, opts)
   else if (mode === 'PAREN_MODE') result = parinfer.parenMode(inputText, opts)
 
+  console.assert(Number.isInteger(result.cursorLine), 'Parinfer result.cursorLine is not an integer')
+  console.assert(Number.isInteger(result.cursorX), 'Parinfer result.cursorX is not an integer')
+
   if (logParinferOutput) {
     console.log(result)
     console.log('~~~~~~~~~~~~~~~~ parinfer output ~~~~~~~~~~~~~~~~')
@@ -54,33 +52,42 @@ function applyParinfer2 (editor, inputText, opts, mode) {
   // FIXME: I think there are some cases where we can show an error here?
   if (!result.success) return
 
-  // if the text was unchanged, update the paren trails and exit
-  if (result.text === inputText) {
-    updateParenTrails(mode, editor, result.parenTrails)
-    return
-  }
+  const didTextChange = result.text !== inputText
 
-  const undoOptions = {
-    undoStopAfter: false,
-    undoStopBefore: false
+  const isSelectionEmpty = editor.selection.isEmpty
+  const anchorPosition = editor.selection.anchor
+  const newCursorPosition = new Position(result.cursorLine, result.cursorX)
+  let newSelection = null
+  if (isSelectionEmpty) {
+    newSelection = new Selection(newCursorPosition, newCursorPosition)
+  } else {
+    newSelection = new Selection(anchorPosition, newCursorPosition)
   }
 
-  const editPromise = editor.edit(function (editBuilder) {
-    editBuilder.replace(editorModule.getEditorRange(editor), result.text)
-  }, undoOptions)
-
-  editPromise.then(function (editWasApplied) {
-    if (editWasApplied) {
-      // set the new cursor position
-      const newCursorPosition = new Position(result.cursorLine, result.cursorX)
-      const nextCursor = new Selection(newCursorPosition, newCursorPosition)
-      editor.selection = nextCursor
-
-      updateParenTrails(mode, editor, result.parenTrails)
-    } else {
-      // TODO: should we do something here if the edit fails?
-    }
-  })
+  // text unchanged: just update the paren trails
+  if (!didTextChange) {
+    state.prevTxt = result.text
+    updateParenTrails(mode, editor, result.parenTrails)
+  // text changed
+  } else {
+    state.ignoreDocumentVersion = editor.document.version + 1
+    const editPromise = editor.edit(function (editBuilder) {
+      // NOTE: should this be delete + insert instead?
+      // https://github.com/Microsoft/vscode/issues/32058
+      editBuilder.replace(editorModule.getEditorRange(editor), result.text)
+    }, undoOptions)
+
+    // FYI - https://github.com/Microsoft/vscode/issues/16389
+    editPromise.then(function (editWasApplied) {
+      if (editWasApplied) {
+        editor.selection = newSelection
+        state.prevTxt = result.text
+        updateParenTrails(mode, editor, result.parenTrails)
+      } else {
+        // TODO: should we do something here if the edit fails?
+      }
+    })
+  }
 }
 
 function applyParinfer (editor, text, opts) {

src/state.js

@@ -0,0 +1,3 @@
+// state that is shared between modules
+exports.ignoreDocumentVersion = null
+exports.prevTxt = null