Only use TextBuffer.onDidChangeText, not onDidChange

lib/autocomplete-manager.js

@@ -1,4 +1,4 @@
-const {Range, CompositeDisposable, Disposable} = require('atom')
+const {CompositeDisposable, Disposable} = require('atom')
 const path = require('path')
 const semver = require('semver')
 const fuzzaldrin = require('fuzzaldrin')
@@ -42,9 +42,7 @@ class AutocompleteManager {
     this.displaySuggestions = this.displaySuggestions.bind(this)
     this.hideSuggestionList = this.hideSuggestionList.bind(this)
 
-    this.toggleActivationForBufferChange = this.toggleActivationForBufferChange.bind(this)
     this.showOrHideSuggestionListForBufferChanges = this.showOrHideSuggestionListForBufferChanges.bind(this)
-    this.showOrHideSuggestionListForBufferChange = this.showOrHideSuggestionListForBufferChange.bind(this)
     this.providerManager = new ProviderManager()
     this.suggestionList = new SuggestionList()
     this.watchedEditors = new WeakSet()
@@ -103,13 +101,7 @@ class AutocompleteManager {
 
     // Subscribe to buffer events:
     this.editorSubscriptions.add(this.buffer.onDidSave((e) => { this.bufferSaved(e) }))
-    if (typeof this.buffer.onDidChangeText === 'function') {
-      this.editorSubscriptions.add(this.buffer.onDidChange(this.toggleActivationForBufferChange))
-      this.editorSubscriptions.add(this.buffer.onDidChangeText(this.showOrHideSuggestionListForBufferChanges))
-    } else {
-      // TODO: Remove this after `TextBuffer.prototype.onDidChangeText` lands on Atom stable.
-      this.editorSubscriptions.add(this.buffer.onDidChange(this.showOrHideSuggestionListForBufferChange))
-    }
+    this.editorSubscriptions.add(this.buffer.onDidChangeText(this.showOrHideSuggestionListForBufferChanges))
 
     // Watch IME Events To Allow IME To Function Without The Suggestion List Showing
     const compositionStart = () => {
@@ -541,12 +533,18 @@ See https://github.com/atom/autocomplete-plus/wiki/Provider-API`
   }
 
   requestHideSuggestionList (command) {
-    this.hideTimeout = setTimeout(this.hideSuggestionList, 0)
+    if (this.hideTimeout == null) {
+      this.hideTimeout = setTimeout(() => {
+        this.hideSuggestionList()
+        this.hideTimeout = null
+      }, 0)
+    }
     this.shouldDisplaySuggestions = false
   }
 
   cancelHideSuggestionListRequest () {
-    return clearTimeout(this.hideTimeout)
+    clearTimeout(this.hideTimeout)
+    this.hideTimeout = null
   }
 
   // Private: Replaces the current prefix with the given match.
@@ -650,7 +648,7 @@ See https://github.com/atom/autocomplete-plus/wiki/Provider-API`
     // bufferChanged handler decides to show suggestions, it will cancel the
     // hideSuggestionList request. If there is no bufferChanged event,
     // suggestionList will be hidden.
-    if (!textChanged && !this.shouldActivate) { return this.requestHideSuggestionList() }
+    if (!textChanged) this.requestHideSuggestionList()
   }
 
   // Private: Gets called when the user saves the document. Cancels the
@@ -659,111 +657,35 @@ See https://github.com/atom/autocomplete-plus/wiki/Provider-API`
     if (!this.autosaveEnabled) { return this.hideSuggestionList() }
   }
 
-  toggleActivationForBufferChange ({newText, newRange, oldText, oldRange}) {
-    if (this.disposed) { return }
-    if (this.shouldActivate) { return }
-    if (this.compositionInProgress) { return this.hideSuggestionList() }
-
-    if (this.autoActivationEnabled || this.suggestionList.isActive()) {
-      if (newText.length > 0) {
-        // Activate on space, a non-whitespace character, or a bracket-matcher pair.
-        if (newText === ' ' || newText.trim().length === 1) {
-          this.shouldActivate = true
-        }
-
-        if (newText.length === 2) {
-          for (const pair of this.bracketMatcherPairs) {
-            if (newText === pair) {
-              this.shouldActivate = true
-            }
-          }
-        }
-      } else if (oldText.length > 0) {
-        // Suggestion list must be either active or backspaceTriggersAutocomplete must be true for activation to occur.
-        // Activate on removal of a space, a non-whitespace character, or a bracket-matcher pair.
-        if (this.backspaceTriggersAutocomplete || this.suggestionList.isActive()) {
-          if (oldText.length > 0 && (this.backspaceTriggersAutocomplete || this.suggestionList.isActive())) {
-            if (oldText === ' ' || oldText.trim().length === 1) {
-              this.shouldActivate = true
-            }
-
-            if (oldText.length === 2) {
-              for (const pair of this.bracketMatcherPairs) {
-                if (oldText === pair) {
-                  this.shouldActivate = true
-                }
-              }
-            }
-          }
-        }
-      }
-
-      if (this.shouldActivate && this.shouldSuppressActivationForEditorClasses()) {
-        this.shouldActivate = false
-      }
-    }
-  }
-
   showOrHideSuggestionListForBufferChanges ({changes}) {
+    if (this.disposed) return
+
     const lastCursorPosition = this.editor.getLastCursor().getBufferPosition()
-    const changeOccurredNearLastCursor = changes.some(({start, newExtent}) => {
-      const newRange = new Range(start, start.traverse(newExtent))
+    const changeOccurredNearLastCursor = changes.some(({newRange}) => {
       return newRange.containsPoint(lastCursorPosition)
     })
+    if (!changeOccurredNearLastCursor) return
 
-    if (changeOccurredNearLastCursor) {
-      if (this.shouldActivate) {
-        this.cancelHideSuggestionListRequest()
-        this.requestNewSuggestions()
-      } else {
-        this.cancelNewSuggestionsRequest()
-        this.hideSuggestionList()
-      }
-    }
-
-    this.shouldActivate = false
-  }
-
-  showOrHideSuggestionListForBufferChange ({newText, newRange, oldText, oldRange}) {
-    if (this.disposed) { return }
-    if (this.compositionInProgress) { return this.hideSuggestionList() }
     let shouldActivate = false
-    const cursorPositions = this.editor.getCursorBufferPositions()
-
-    if (this.autoActivationEnabled || this.suggestionList.isActive()) {
-      // Activate on space, a non-whitespace character, or a bracket-matcher pair.
-      if (newText.length > 0) {
-        if (cursorPositions.some((position) => { return newRange.containsPoint(position) })) {
-          if (newText === ' ' || newText.trim().length === 1) {
-            shouldActivate = true
-          }
-          if (newText.length === 2) {
-            for (const pair of this.bracketMatcherPairs) {
-              if (newText === pair) {
-                shouldActivate = true
-              }
-            }
+    if (this.autoActivationEnabled || this.suggestionList.isActive() && !this.compositionInProgress) {
+      shouldActivate = changes.some(({oldText, newText}) => {
+        if (this.autoActivationEnabled || this.suggestionList.isActive()) {
+          if (newText.length > 0) {
+            // Activate on space, a non-whitespace character, or a bracket-matcher pair.
+            if (newText === ' ' ||
+                newText.trim().length === 1 ||
+                (newText.length === 2 && this.bracketMatcherPairs.includes(newText))) return true
+          } else if (oldText.length > 0 && (this.backspaceTriggersAutocomplete || this.suggestionList.isActive())) {
+            // Suggestion list must be either active or backspaceTriggersAutocomplete must be true for activation to occur.
+            // Activate on removal of a space, a non-whitespace character, or a bracket-matcher pair.
+            if (oldText === ' ' ||
+                oldText.trim().length === 1 ||
+                (oldText.length === 2 && this.bracketMatcherPairs.includes(oldText))) return true
           }
         }
-      // Suggestion list must be either active or backspaceTriggersAutocomplete must be true for activation to occur.
-      // Activate on removal of a space, a non-whitespace character, or a bracket-matcher pair.
-      } else if (oldText.length > 0) {
-        if ((this.backspaceTriggersAutocomplete || this.suggestionList.isActive()) &&
-        (cursorPositions.some((position) => { return newRange.containsPoint(position) }))) {
-          if (oldText === ' ' || oldText.trim().length === 1) {
-            shouldActivate = true
-          }
-          if (oldText.length === 2) {
-            for (const pair of this.bracketMatcherPairs) {
-              if (oldText === pair) {
-                shouldActivate = true
-              }
-            }
-          }
-        }
-      }
+      })
 
-      if (shouldActivate && this.shouldSuppressActivationForEditorClasses()) { shouldActivate = false }
+      if (shouldActivate && this.shouldSuppressActivationForEditorClasses()) shouldActivate = false
     }
 
     if (shouldActivate) {

spec/autocomplete-manager-integration-spec.js

@@ -6,8 +6,7 @@ const {
   triggerAutocompletion,
   waitForAutocomplete,
   waitForDeferredSuggestions,
-  buildIMECompositionEvent,
-  buildTextInputEvent
+  buildIMECompositionEvent
 } = require('./spec-helper')
 let temp = require('temp').track()
 const path = require('path')
@@ -2268,30 +2267,40 @@ defm`
         runs(() => expect(editorView.querySelector('.autocomplete-plus')).not.toExist())
       })
 
-      // TODO: Pretty Sure This Test Will Not Catch A Regression In Behavior Due To The Way It Is Written
-      it('should not update the suggestion list while composition is in progress', () => {
-        triggerAutocompletion(editor)
+      it('does not update the suggestion list while composition is in progress', () => {
+        const activeElement = editorView.querySelector('input')
+        spyOn(autocompleteManager.suggestionList, 'changeItems').andCallThrough()
 
-        // unfortunately, we need to fire IME events from the editor's input node so the editor picks them up
-        let activeElement = editorView.querySelector('input')
+        editor.moveToBottom()
+        editor.insertText('q')
+        editor.insertText('u')
 
         runs(() => {
-          spyOn(autocompleteManager.suggestionList, 'changeItems').andCallThrough()
           expect(autocompleteManager.suggestionList.changeItems).not.toHaveBeenCalled()
 
-          activeElement.dispatchEvent(buildIMECompositionEvent('compositionstart', {target: activeElement}))
-          activeElement.dispatchEvent(buildIMECompositionEvent('compositionupdate', {data: '~', target: activeElement}))
+          activeElement.dispatchEvent(buildIMECompositionEvent('compositionstart', {data: 'i', target: activeElement}))
+          triggerAutocompletion(editor, false, 'i')
+        })
 
-          waitForAutocomplete()
+        waitForAutocomplete()
+
+        runs(() => {
+          expect(autocompleteManager.suggestionList.changeItems).toHaveBeenCalled()
+          expect(autocompleteManager.suggestionList.changeItems.mostRecentCall.args[0][0].text).toBe('quicksort')
+          autocompleteManager.suggestionList.changeItems.reset()
+
+          activeElement.dispatchEvent(buildIMECompositionEvent('compositionupdate', {data: 'ï', target: activeElement}))
+          editor.selectLeft()
+
+          editor.insertText('ï')
+          advanceClock(autocompleteManager.suggestionDelay)
         })
 
+        waits(autocompleteManager.suggestionDelay)
+
         runs(() => {
           expect(autocompleteManager.suggestionList.changeItems).toHaveBeenCalledWith(null)
-
           activeElement.dispatchEvent(buildIMECompositionEvent('compositionend', {target: activeElement}))
-          activeElement.dispatchEvent(buildTextInputEvent({data: 'ã', target: activeElement}))
-
-          expect(editor.lineTextForBufferRow(13)).toBe('fã')
         })
       })