From 64b6a2015a6b53e71d3418f1664079f9aff3396d Mon Sep 17 00:00:00 2001 From: Wliu Date: Tue, 7 Feb 2017 11:58:37 -0500 Subject: [PATCH 1/8] Don't try to update markers if there's no new markers --- lib/buffer-search.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/buffer-search.js b/lib/buffer-search.js index c00671b2..64c87ed6 100644 --- a/lib/buffer-search.js +++ b/lib/buffer-search.js @@ -224,6 +224,8 @@ class BufferSearch { } const newMarkers = this.createMarkers(scanStart, scanEnd); + if(newMarkers === false || !newMarkers.length) return; // Invalid/missing find pattern or no new markers + const oldMarkers = this.markers.splice(spliceStart, (spliceEnd - spliceStart) + 1, ...newMarkers); for (let oldMarker of oldMarkers) { oldMarker.destroy(); From f1aa436769c4dd96972351fcd3411a2cedbe5e9b Mon Sep 17 00:00:00 2001 From: Wliu Date: Tue, 7 Feb 2017 12:01:12 -0500 Subject: [PATCH 2/8] Fix deceptive class when an error occurs Consider the following scenario: 1. Turn on regex 2. Search for something simple in the file, eg "this" 3. has-results class appears as expected 4. Add an opening parenthesis to the find pattern 5. Find box is still green?!?! Step 5 introduces contradicting information: a red error message yet a green find box with the has-results class. So now when we set an error message we make sure to remove the has-results class and add the has-no-results class. I'm also fairly certain that nulling a function is not a good idea, so I removed that line. --- lib/find-view.coffee | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/find-view.coffee b/lib/find-view.coffee index 53aaaf51..be877412 100644 --- a/lib/find-view.coffee +++ b/lib/find-view.coffee @@ -307,7 +307,6 @@ class FindView extends View atom.beep() markersUpdated: (@markers) => - @findError = null @updateResultCounter() @updateReplaceEnablement() @@ -342,6 +341,8 @@ class FindView extends View @descriptionLabel.text(infoMessage).removeClass('text-error') setErrorMessage: (errorMessage) -> + this.removeClass('has-results') + this.addClass('has-no-results') @descriptionLabel.text(errorMessage).addClass('text-error') clearMessage: -> From 53db29a3c66e686482d4f7aeedb997f624020e7f Mon Sep 17 00:00:00 2001 From: Wliu Date: Wed, 8 Feb 2017 21:52:50 -0500 Subject: [PATCH 3/8] Add a spec --- lib/buffer-search.js | 2 +- spec/find-view-spec.coffee | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/buffer-search.js b/lib/buffer-search.js index 64c87ed6..300115e8 100644 --- a/lib/buffer-search.js +++ b/lib/buffer-search.js @@ -239,7 +239,7 @@ class BufferSearch { } } - this.emitter.emit('did-update', this.markers.slice()); + if(changes.length) this.emitter.emit('did-update', this.markers.slice()); this.currentResultMarker = null; this.setCurrentMarkerFromSelection(); } diff --git a/spec/find-view-spec.coffee b/spec/find-view-spec.coffee index 722c9ae9..69c04a92 100644 --- a/spec/find-view-spec.coffee +++ b/spec/find-view-spec.coffee @@ -506,6 +506,18 @@ describe 'FindView', -> expect(findView.descriptionLabel).toHaveClass 'text-error' expect(findView.descriptionLabel.text()).toContain 'Invalid regular expression' + it "does not reset when the buffer is updated", -> + # This is a rather specific test: the error message only disappeared + # when there were no changes. Backspacing nothing can reproduce this, + # as well as saving or undoing and then redoing. + editor.setCursorBufferPosition([0, 0]) + # Since we're at the beginning of the file, backspacing will do nothing but still send a change event + editor.backspace() + advanceClock(editor.buffer.stoppedChangingDelay) + + expect(findView.descriptionLabel).toHaveClass 'text-error' + expect(findView.descriptionLabel.text()).toContain 'Invalid regular expression' + it "will be reset when there is no longer an error", -> expect(findView.descriptionLabel).toHaveClass 'text-error' From 5fa4b3ba5ca38d210c3e8f81761dcf509ec5819a Mon Sep 17 00:00:00 2001 From: Wliu Date: Wed, 8 Feb 2017 22:00:35 -0500 Subject: [PATCH 4/8] Add spec for has-no-results --- spec/find-view-spec.coffee | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/find-view-spec.coffee b/spec/find-view-spec.coffee index 69c04a92..1f5fc9ad 100644 --- a/spec/find-view-spec.coffee +++ b/spec/find-view-spec.coffee @@ -518,6 +518,17 @@ describe 'FindView', -> expect(findView.descriptionLabel).toHaveClass 'text-error' expect(findView.descriptionLabel.text()).toContain 'Invalid regular expression' + it "adds the has-no-results class", -> + findView.findEditor.setText 'it' + atom.commands.dispatch(findView.findEditor.element, 'core:confirm') + expect(findView).toHaveClass 'has-results' + expect(findView).not.toHaveClass 'has-no-results' + + findView.findEditor.setText 'i[t' + atom.commands.dispatch(findView.findEditor.element, 'core:confirm') + expect(findView).toHaveClass 'has-no-results' + expect(findView).not.toHaveClass 'has-results' + it "will be reset when there is no longer an error", -> expect(findView.descriptionLabel).toHaveClass 'text-error' From e81f8df64f73245fea3a1cdef6c89fd261d0a77e Mon Sep 17 00:00:00 2001 From: Wliu Date: Wed, 8 Feb 2017 22:14:38 -0500 Subject: [PATCH 5/8] Add tests to prevent superfluous update events --- spec/buffer-search-spec.coffee | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/spec/buffer-search-spec.coffee b/spec/buffer-search-spec.coffee index 97b71924..4bf3fc63 100644 --- a/spec/buffer-search-spec.coffee +++ b/spec/buffer-search-spec.coffee @@ -376,6 +376,25 @@ describe "BufferSearch", -> expect(scannedRanges()).toEqual [] + describe "when no markers are affected in a change event", -> + it "does not emit an update event", -> + editor.setCursorBufferPosition([0, 0]) + editor.backspace() + advanceClock(editor.buffer.stoppedChangingDelay) + expectNoUpdateEvent() + + describe "when the find pattern is invalid and a change occurs", -> + it "does not attempt to update markers", -> + model.search "a(", + caseSensitive: false + useRegex: true + wholeWord: false + + editor.insertText(".") + advanceClock(editor.buffer.stoppedChangingDelay) + + expectNoUpdateEvent() + describe "replacing a search result", -> beforeEach -> editor.scanInBufferRange.reset() From e4c3c4b927b8ace382bf45b68e448b4f1ba6bc4d Mon Sep 17 00:00:00 2001 From: Wliu Date: Tue, 14 Feb 2017 23:09:09 -0500 Subject: [PATCH 6/8] Revert "Don't try to update markers if there's no new markers" This reverts commit 64b6a2015a6b53e71d3418f1664079f9aff3396d. --- lib/buffer-search.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/buffer-search.js b/lib/buffer-search.js index 300115e8..dd219236 100644 --- a/lib/buffer-search.js +++ b/lib/buffer-search.js @@ -224,8 +224,6 @@ class BufferSearch { } const newMarkers = this.createMarkers(scanStart, scanEnd); - if(newMarkers === false || !newMarkers.length) return; // Invalid/missing find pattern or no new markers - const oldMarkers = this.markers.splice(spliceStart, (spliceEnd - spliceStart) + 1, ...newMarkers); for (let oldMarker of oldMarkers) { oldMarker.destroy(); From 0d07d569a4b2a7619f32dd9a6ba6d108f59707f3 Mon Sep 17 00:00:00 2001 From: Wliu Date: Tue, 14 Feb 2017 23:09:27 -0500 Subject: [PATCH 7/8] Revert "Add tests to prevent superfluous update events" This reverts commit e81f8df64f73245fea3a1cdef6c89fd261d0a77e. --- spec/buffer-search-spec.coffee | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/spec/buffer-search-spec.coffee b/spec/buffer-search-spec.coffee index 4bf3fc63..97b71924 100644 --- a/spec/buffer-search-spec.coffee +++ b/spec/buffer-search-spec.coffee @@ -376,25 +376,6 @@ describe "BufferSearch", -> expect(scannedRanges()).toEqual [] - describe "when no markers are affected in a change event", -> - it "does not emit an update event", -> - editor.setCursorBufferPosition([0, 0]) - editor.backspace() - advanceClock(editor.buffer.stoppedChangingDelay) - expectNoUpdateEvent() - - describe "when the find pattern is invalid and a change occurs", -> - it "does not attempt to update markers", -> - model.search "a(", - caseSensitive: false - useRegex: true - wholeWord: false - - editor.insertText(".") - advanceClock(editor.buffer.stoppedChangingDelay) - - expectNoUpdateEvent() - describe "replacing a search result", -> beforeEach -> editor.scanInBufferRange.reset() From 9e8e75b6b2c84aab2f812363c0cb935287b59c29 Mon Sep 17 00:00:00 2001 From: Wliu Date: Tue, 14 Feb 2017 23:10:02 -0500 Subject: [PATCH 8/8] :art: --- lib/buffer-search.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/buffer-search.js b/lib/buffer-search.js index dd219236..e505c184 100644 --- a/lib/buffer-search.js +++ b/lib/buffer-search.js @@ -237,7 +237,7 @@ class BufferSearch { } } - if(changes.length) this.emitter.emit('did-update', this.markers.slice()); + if (changes.length) this.emitter.emit('did-update', this.markers.slice()); this.currentResultMarker = null; this.setCurrentMarkerFromSelection(); }