Responded to all of @maxbrunsfeld's comments except for:

* The effect of a failed search.
* Letting `DirectorySearcher::search` take multiple directories.

I'm working on those now.
This commit is contained in:
Michael Bolin
2015-06-03 21:35:30 -04:00
parent 5fc9d9e01a
commit 318498464a
3 changed files with 75 additions and 107 deletions

View File

@@ -939,36 +939,47 @@ describe "Workspace", ->
expect(resultPaths).toEqual([file2])
describe "when a custom directory searcher is registered", ->
fakeSearch = null
# Function that is invoked once all of the fields on fakeSearch are set.
onFakeSearchCreated = null
class FakeSearch
constructor: (@options) ->
# Note that hoisting resolve and reject in this way is generally frowned upon.
@promise = new Promise (resolve, reject) =>
@hoistedResolve = resolve
@hoistedReject = reject
onFakeSearchCreated?(this)
then: (args...) ->
@promise.then.apply(@promise, args)
cancel: -> @cancelled = true
beforeEach ->
fakeSearch = null
onFakeSearchCreated = null
atom.packages.serviceHub.provide('atom.directory-searcher', '0.1.0', {
canSearchDirectory: (directory) -> directory.getPath() is dir1
search: (directory, regex, options) -> fakeSearch = new FakeSearch(options)
})
it "can override the DefaultDirectorySearcher on a per-directory basis", ->
foreignFilePath = 'ssh://foreign-directory:8080/hello.txt'
numPathsSearchedInDir2 = 1
numPathsToPretendToSearchInCustomDirectorySearcher = 10
class CustomDirectorySearch
constructor: (delegate) ->
@promise = Promise.resolve()
searchResult1 =
filePath: foreignFilePath,
matches: [
{
lineText: 'Hello world',
lineTextOffset: 0,
matchText: 'Hello',
range: [[0, 0], [0, 5]],
},
]
delegate.didMatch(searchResult1)
delegate.didSearchPaths(numPathsToPretendToSearchInCustomDirectorySearcher)
then: (args...) ->
@promise.then.apply(@promise, args)
cancel: ->
class CustomDirectorySearcher
canSearchDirectory: (directory) -> directory.getPath() is dir1
search: (directory, delegate, options) ->
new CustomDirectorySearch(delegate)
atom.packages.serviceHub.provide(
"atom.directory-searcher", "0.1.0", new CustomDirectorySearcher())
searchResult =
filePath: foreignFilePath,
matches: [
{
lineText: 'Hello world',
lineTextOffset: 0,
matchText: 'Hello',
range: [[0, 0], [0, 5]],
},
]
onFakeSearchCreated = (fakeSearch) ->
fakeSearch.options.didMatch(searchResult)
fakeSearch.options.didSearchPaths(numPathsToPretendToSearchInCustomDirectorySearcher)
fakeSearch.hoistedResolve()
resultPaths = []
onPathsSearched = jasmine.createSpy('onPathsSearched')
@@ -986,28 +997,10 @@ describe "Workspace", ->
numPathsToPretendToSearchInCustomDirectorySearcher + numPathsSearchedInDir2)
it "can be cancelled when the object returned by scan() has its cancel() method invoked", ->
lastCustomDirectorySearchCreated = null
class CustomDirectorySearch
constructor: ->
lastCustomDirectorySearchCreated = this
@promise = Promise.resolve()
then: (args...) ->
@promise.then.apply(@promise, args)
cancel: ->
class CustomDirectorySearcher
canSearchDirectory: (directory) -> directory.getPath() is dir1
search: (directory, delegate, options) ->
new CustomDirectorySearch
atom.packages.serviceHub.provide(
"atom.directory-searcher", "0.1.0", new CustomDirectorySearcher())
thenable = atom.workspace.scan /aaaa/, ->
cancelSpy = spyOn(lastCustomDirectorySearchCreated, 'cancel').andCallThrough()
expect(cancelSpy).not.toHaveBeenCalled()
expect(fakeSearch.cancelled).toBe(undefined)
thenable.cancel()
expect(cancelSpy).toHaveBeenCalled()
expect(fakeSearch.cancelled).toBe(true)
resultOfPromiseSearch = null
waitsForPromise ->
@@ -1017,26 +1010,8 @@ describe "Workspace", ->
expect(resultOfPromiseSearch).toBe('cancelled')
it "will have the side-effect of failing the overall search if it fails", ->
# Note that hoisting reject in this way is generally frowned upon.
hoistedReject = null
class CustomDirectorySearchThatWillFail
constructor: ->
@promise = new Promise (resolve, reject) ->
hoistedReject = reject
then: (args...) ->
@promise.then.apply(@promise, args)
cancel: ->
class CustomDirectorySearcherToCancel
canSearchDirectory: (directory) -> directory.getPath() is dir1
search: (directory, options) ->
new CustomDirectorySearchThatWillFail
atom.packages.serviceHub.provide(
"atom.directory-searcher", "0.1.0", new CustomDirectorySearcherToCancel())
cancelableSearch = atom.workspace.scan /aaaa/, ->
hoistedReject()
fakeSearch.hoistedReject()
resultOfPromiseSearch = null
waitsForPromise ->

View File

@@ -29,16 +29,24 @@ 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, delegate, options, id) ->
constructor: (directory, regex, options, id) ->
scanHandlerOptions =
ignoreCase: regex.ignoreCase
inclusions: options.inclusions
includeHidden: options.includeHidden
excludeVcsIgnores: options.excludeVcsIgnores
exclusions: options.exclusions
follow: options.follow
@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.didMatch
@task.on 'scan:file-error', delegate.didError
@task.on 'scan:paths-searched', delegate.didSearchPaths
@task.start(rootPaths, regex.source, scanHandlerOptions, resolve)
@task.on 'scan:result-found', options.didMatch
@task.on 'scan:file-error', options.didError
@task.on 'scan:paths-searched', options.didSearchPaths
# Public: Implementation of `then()` to satisfy the *thenable* contract.
# This makes it possible to use a `DirectorySearch` with `Promise.all()`.
@@ -67,11 +75,13 @@ 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 invoking methods on the specified `delegate`.
# Results are streamed back to the caller by invoking methods on the specified `options`,
# such as `didMatch` and `didError`.
#
# * `directory` {Directory} that has been accepted by this provider's `canSearchDirectory()`
# predicate.
# * `delegate` {Object} with the following properties:
# * `regex` {RegExp} to search with.
# * `options` {Object} with the following properties:
# * `didMatch` {Function} call with a search result structured as follows:
# * `searchResult` {Object} with the following keys:
# * `filePath` {String} absolute path to the matching file.
@@ -82,9 +92,6 @@ class DefaultDirectorySearcher
# * `range` {Range} Identifies the matching region in the file. (Likely as an array of numeric arrays.)
# * `didError` {Function} call with an Error if there is a problem during the search.
# * `didSearchPaths` {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.
# * `inclusions` {Array} of glob patterns (as strings) to search within. Note that this
# array may be empty, indicating that all files should be searched.
#
@@ -98,10 +105,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, delegate, options) ->
search: (directory, regex, options) ->
id = nextId
nextId += 1
directorySearch = new DirectorySearch(directory, delegate, options, id)
directorySearch = new DirectorySearch(directory, regex, options, id)
directorySearch.then(onSearchFinished, onSearchFinished)
enqueue(id)
directorySearch

View File

@@ -47,7 +47,8 @@ class Workspace extends Model
@paneContainer ?= new PaneContainer()
@paneContainer.onDidDestroyPaneItem(@didDestroyPaneItem)
@directorySearchers = [new DefaultDirectorySearcher()]
@directorySearchers = []
@defaultDirectorySearcher = new DefaultDirectorySearcher()
atom.packages.serviceHub.consume(
'atom.directory-searcher',
'^0.1.0',
@@ -810,35 +811,15 @@ class Workspace extends Model
iterator = options
options = {}
searchOptions =
regexSource: regex.source
ignoreCase: regex.ignoreCase
inclusions: options.paths or []
includeHidden: true
excludeVcsIgnores: atom.config.get('core.excludeVcsIgnoredPaths')
exclusions: atom.config.get('core.ignoredNames')
follow: atom.config.get('core.followSymlinks')
# Find a searcher for every Directory in the project.
searchersAndDirectories = []
for directory in atom.project.getDirectories()
searcher = null
searcher = @defaultDirectorySearcher
for directorySearcher in @directorySearchers
if directorySearcher.canSearchDirectory(directory)
searcher = directorySearcher
break
if searcher
searchersAndDirectories.push({searcher, directory})
else
throw Error("Could not find directory searcher for #{directory.getPath()}")
# Now that we are sure every Directory has a searcher, construct the search options.
delegateProto = {
didMatch: (result) ->
iterator(result) unless atom.project.isPathModified(result.filePath)
didError: (error) ->
iterator(null, error)
}
searchersAndDirectories.push({searcher, directory})
# Define the onPathsSearched callback.
if _.isFunction(options.onPathsSearched)
@@ -861,13 +842,18 @@ class Workspace extends Model
allSearches = []
for entry in searchersAndDirectories
{searcher, directory} = entry
recordNumberOfPathsSearched = onPathsSearched.bind(undefined, directory)
delegate = Object.create(delegateProto, {
didSearchPaths: {
value: recordNumberOfPathsSearched,
}
})
directorySearcher = searcher.search(directory, delegate, searchOptions)
searchOptions =
inclusions: options.paths or []
includeHidden: true
excludeVcsIgnores: atom.config.get('core.excludeVcsIgnoredPaths')
exclusions: atom.config.get('core.ignoredNames')
follow: atom.config.get('core.followSymlinks')
didMatch: (result) ->
iterator(result) unless atom.project.isPathModified(result.filePath)
didError: (error) ->
iterator(null, error)
didSearchPaths: onPathsSearched.bind(undefined, directory)
directorySearcher = searcher.search(directory, regex, searchOptions)
allSearches.push(directorySearcher)
searchPromise = Promise.all(allSearches)