From 318498464a766ad8f946bf126d083fefb5773003 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 3 Jun 2015 21:35:30 -0400 Subject: [PATCH] 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. --- spec/workspace-spec.coffee | 105 ++++++++++---------------- src/default-directory-searcher.coffee | 31 +++++--- src/workspace.coffee | 46 ++++------- 3 files changed, 75 insertions(+), 107 deletions(-) diff --git a/spec/workspace-spec.coffee b/spec/workspace-spec.coffee index 07c543968..26e348f72 100644 --- a/spec/workspace-spec.coffee +++ b/spec/workspace-spec.coffee @@ -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 -> diff --git a/src/default-directory-searcher.coffee b/src/default-directory-searcher.coffee index b209cca70..912ad818f 100644 --- a/src/default-directory-searcher.coffee +++ b/src/default-directory-searcher.coffee @@ -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 diff --git a/src/workspace.coffee b/src/workspace.coffee index 77706fd69..e9ac46bd2 100644 --- a/src/workspace.coffee +++ b/src/workspace.coffee @@ -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)