Switch to the delegate pattern,

eliminating a nasty race condition and deleting a lot of code.
This commit is contained in:
Michael Bolin
2015-06-02 17:40:27 -04:00
parent fa3fd9c50c
commit 4eb30f3925
3 changed files with 38 additions and 72 deletions

View File

@@ -945,12 +945,8 @@ describe "Workspace", ->
numPathsSearchedInDir2 = 1
numPathsToPretendToSearchInCustomDirectorySearcher = 10
class CustomDirectorySearch
constructor: ->
constructor: (delegate) ->
@promise = Promise.resolve()
then: (args...) ->
@promise.then.apply(@promise, args)
onDidMatch: (callback) ->
# Invoke the callback with the only result we plan to return.
searchResult1 =
filePath: foreignFilePath,
matches: [
@@ -959,22 +955,18 @@ describe "Workspace", ->
lineTextOffset: 0,
matchText: 'Hello',
range: [[0, 0], [0, 5]],
}
},
]
callback(searchResult1)
new Disposable
onDidError: (callback) ->
new Disposable
onDidSearchPaths: (callback) ->
# Invoke the callback with the one notification we plan to send.
callback(numPathsToPretendToSearchInCustomDirectorySearcher)
new Disposable
delegate.onDidMatch(searchResult1)
delegate.onDidSearchPaths(numPathsToPretendToSearchInCustomDirectorySearcher)
then: (args...) ->
@promise.then.apply(@promise, args)
cancel: ->
class CustomDirectorySearcher
canSearchDirectory: (directory) -> directory.getPath() is dir1
search: (directory, options) ->
new CustomDirectorySearch
search: (directory, delegate, options) ->
new CustomDirectorySearch(delegate)
atom.packages.serviceHub.provide(
"atom.directory-searcher", "0.1.0", new CustomDirectorySearcher())
@@ -1004,12 +996,6 @@ describe "Workspace", ->
customDirectorySearcherPromiseInstance = this
then: (args...) ->
@promise.then.apply(@promise, args)
onDidMatch: (callback) ->
new Disposable
onDidError: (callback) ->
new Disposable
onDidSearchPaths: (callback) ->
new Disposable
cancel: ->
@hoistedReject()

View File

@@ -29,13 +29,16 @@ enqueue = (id) ->
class DirectorySearch
# Public: Creates a new DirectorySearch that will not start running until the
# `emitter` that is private to this file emits an event with the specified `id`.
constructor: (directory, options, id) ->
constructor: (directory, delegate, options, id) ->
@task = new Task(require.resolve('./scan-handler'))
rootPaths = [directory.getPath()]
@promise = new Promise (resolve, reject) =>
@task.on('task:cancelled', reject)
emitter.once id, =>
@task.start(rootPaths, options.regexSource, options, resolve)
@task.on 'scan:result-found', delegate.onDidMatch
@task.on 'scan:file-error', delegate.onDidError
@task.on 'scan:paths-searched', delegate.onDidSearchPaths
# Public: Implementation of `then()` to satisfy the *thenable* contract.
# This makes it possible to use a `DirectorySearch` with `Promise.all()`.
@@ -44,37 +47,6 @@ class DirectorySearch
then: (args...) ->
@promise.then.apply(@promise, args)
# Public: Get notified when a search result is found.
#
# * `callback` {Function} called with a search result structured as follows:
# * `searchResult` {Object} with the following keys:
# * `filePath` {String} absolute path to the matching file.
# * `matches` {Array} with object elements with the following keys:
# * `lineText` {String} The full text of the matching line (without a line terminator character).
# * `lineTextOffset` {Number} (This always seems to be 0?)
# * `matchText` {String} The text that matched the `regex` used for the search.
# * `range` {Range} Identifies the matching region in the file. (Likely as an array of numeric arrays.)
#
# Returns `Disposable`.
onDidMatch: (callback) ->
@task.on 'scan:result-found', callback
# Public: Get notified about any search errors.
#
# * `callback` {Function} called with an Error if there is a problem during the search.
#
# Returns `Disposable`.
onDidError: (callback) ->
@task.on 'scan:file-error', callback
# Public: Get notified with the number of paths searched thus far.
#
# * `callback` {Function} called with the number of paths searched thus far.
#
# Returns `Disposable`.
onDidSearchPaths: (callback) ->
@task.on 'scan:paths-searched', callback
# Public: Cancels the search.
cancel: ->
# This will cause @promise to reject.
@@ -95,11 +67,21 @@ class DefaultDirectorySearcher
# Public: Performs a text search for files in the specified `Directory`, subject to the
# specified parameters.
#
# Results are streamed back to the caller by adding callbacks to the `DirectorySearch` returned by
# this method.
# Results are streamed back to the caller by invoking methods on the specified `delegate`.
#
# * `directory` {Directory} that has been accepted by this provider's `canSearchDirectory()`
# predicate.
# * `delegate` {Object} with the following properties:
# * `onDidMatch` {Function} call with a search result structured as follows:
# * `searchResult` {Object} with the following keys:
# * `filePath` {String} absolute path to the matching file.
# * `matches` {Array} with object elements with the following keys:
# * `lineText` {String} The full text of the matching line (without a line terminator character).
# * `lineTextOffset` {Number} (This always seems to be 0?)
# * `matchText` {String} The text that matched the `regex` used for the search.
# * `range` {Range} Identifies the matching region in the file. (Likely as an array of numeric arrays.)
# * `onDidError` {Function} call with an Error if there is a problem during the search.
# * `onDidSearchPaths` {Function} periodically call with the number of paths searched thus far.
# * `options` {Object} with the following properties:
# * `regexSource` {String} regex to search with. Produced via `RegExp::source`.
# * `ignoreCase` {boolean} reflects whether the regex should be run with the `i` option.
@@ -116,10 +98,10 @@ class DefaultDirectorySearcher
#
# Returns a *thenable* `DirectorySearch` that includes a `cancel()` method. If `cancel()` is
# invoked before the `DirectorySearch` is determined, it will reject the `DirectorySearch`.
search: (directory, options) ->
search: (directory, delegate, options) ->
id = nextId
nextId += 1
directorySearch = new DirectorySearch(directory, options, id)
directorySearch = new DirectorySearch(directory, delegate, options, id)
directorySearch.then(onSearchFinished, onSearchFinished)
enqueue(id)
directorySearch

View File

@@ -832,10 +832,12 @@ class Workspace extends Model
throw Error("Could not find directory searcher for #{directory.getPath()}")
# Now that we are sure every Directory has a searcher, construct the search options.
onSearchResult = (result) ->
iterator(result) unless atom.project.isPathModified(result.filePath)
onSearchError = (error) ->
iterator(null, error)
delegateProto = {
onDidMatch: (result) ->
iterator(result) unless atom.project.isPathModified(result.filePath)
onDidError: (error) ->
iterator(null, error)
}
# Define the onPathsSearched callback.
if _.isFunction(options.onPathsSearched)
@@ -856,22 +858,18 @@ class Workspace extends Model
# Kick off all of the searches and unify them into one Promise.
allSearches = []
disposables = new CompositeDisposable
for entry in searchersAndDirectories
{searcher, directory} = entry
directorySearcher = searcher.search(directory, searchOptions)
disposables.add(directorySearcher.onDidMatch(onSearchResult))
disposables.add(directorySearcher.onDidError(onSearchError))
recordNumberOfPathsSearched = onPathsSearched.bind(undefined, directory)
disposables.add(directorySearcher.onDidSearchPaths(recordNumberOfPathsSearched))
delegate = Object.create(delegateProto, {
onDidSearchPaths: {
value: recordNumberOfPathsSearched,
}
})
directorySearcher = searcher.search(directory, delegate, searchOptions)
allSearches.push(directorySearcher)
searchPromise = Promise.all(allSearches)
# Make sure to clean up the disposables once the searchPromise is determined.
disposeAll = (args...) ->
disposables.dispose()
searchPromise.then(disposeAll, disposeAll)
for buffer in atom.project.getBuffers() when buffer.isModified()
filePath = buffer.getPath()
continue unless atom.project.contains(filePath)