From 307ac74098bd772b44689dfc6567f2e647091dfd Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 26 Oct 2017 13:24:08 -0700 Subject: [PATCH] Only use TextBuffer.onDidChangeText, not onDidChange --- lib/autocomplete-manager.js | 140 ++++-------------- spec/autocomplete-manager-integration-spec.js | 39 +++-- 2 files changed, 55 insertions(+), 124 deletions(-) diff --git a/lib/autocomplete-manager.js b/lib/autocomplete-manager.js index 6378f9ec..31b22435 100644 --- a/lib/autocomplete-manager.js +++ b/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) { diff --git a/spec/autocomplete-manager-integration-spec.js b/spec/autocomplete-manager-integration-spec.js index d41a92e9..1f043c4a 100644 --- a/spec/autocomplete-manager-integration-spec.js +++ b/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ã') }) })