From c728ad6d57337ee858800a29d7b94f2035fb0295 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 12 Feb 2015 15:47:09 -0800 Subject: [PATCH 1/6] Introduce atom.directory-provider service. A `Project` will always have a `DefaultDirectoryProvider` that will be used if there are no other `DirectoryProvider` objects that can produce a `Directory` for a path. --- spec/default-directory-provider-spec.coffee | 21 +++++++++++++ src/default-directory-provider.coffee | 33 ++++++++++++++++++++ src/project.coffee | 34 +++++++++++++-------- 3 files changed, 75 insertions(+), 13 deletions(-) create mode 100644 spec/default-directory-provider-spec.coffee create mode 100644 src/default-directory-provider.coffee diff --git a/spec/default-directory-provider-spec.coffee b/spec/default-directory-provider-spec.coffee new file mode 100644 index 000000000..63da18523 --- /dev/null +++ b/spec/default-directory-provider-spec.coffee @@ -0,0 +1,21 @@ +DefaultDirectoryProvider = require "../src/default-directory-provider" +path = require "path" +temp = require "temp" + +describe "DefaultDirectoryProvider", -> + describe ".directoryForURISync(uri)", -> + it "returns a Directory with a path that matches the uri", -> + provider = new DefaultDirectoryProvider() + tmp = temp.mkdirSync() + + directory = provider.directoryForURISync(tmp) + expect(directory.getPath()).toEqual tmp + + describe ".directoryForURI(uri)", -> + it "returns a Promise that resolves to a Directory with a path that matches the uri", -> + provider = new DefaultDirectoryProvider() + tmp = temp.mkdirSync() + + waitsForPromise -> + provider.directoryForURI(tmp).then (directory) -> + expect(directory.getPath()).toEqual tmp diff --git a/src/default-directory-provider.coffee b/src/default-directory-provider.coffee new file mode 100644 index 000000000..a05e006f8 --- /dev/null +++ b/src/default-directory-provider.coffee @@ -0,0 +1,33 @@ +{Directory} = require 'pathwatcher' +fs = require 'fs-plus' +path = require 'path' + +module.exports = +class DefaultDirectoryProvider + + # Public: Create a Directory that corresponds to the specified URI. + # + # * `uri` {String} The path to the directory to add. This is guaranteed not to + # be contained by a {Directory} in `atom.project`. + # Returns: + # * {Directory} if the given URI is compatible with this provider. + # * `null` if the given URI is not compatibile with this provider. + directoryForURISync: (uri) -> + projectPath = path.normalize(uri) + + directoryPath = if fs.isDirectorySync(projectPath) + projectPath + else + path.dirname(projectPath) + + new Directory(directoryPath) + + # Public: Create a Directory that corresponds to the specified URI. + # + # * `uri` {String} The path to the directory to add. This is guaranteed not to + # be contained by a {Directory} in `atom.project`. + # Returns a Promise that resolves to: + # * {Directory} if the given URI is compatible with this provider. + # * `null` if the given URI is not compatibile with this provider. + directoryForURI: (uri) -> + Promise.resolve(@directoryForURISync(uri)) diff --git a/src/project.coffee b/src/project.coffee index 50bda3ed8..1f17e6610 100644 --- a/src/project.coffee +++ b/src/project.coffee @@ -8,9 +8,9 @@ Q = require 'q' {Model} = require 'theorist' {Subscriber} = require 'emissary' {Emitter} = require 'event-kit' +DefaultDirectoryProvider = require './default-directory-provider' Serializable = require 'serializable' TextBuffer = require 'text-buffer' -{Directory} = require 'pathwatcher' Grim = require 'grim' TextEditor = require './text-editor' @@ -41,6 +41,14 @@ class Project extends Model @rootDirectories = [] @repositories = [] + @directoryProviders = [new DefaultDirectoryProvider()] + atom.packages.serviceHub.consume( + 'atom.directory-provider', + '^0.1.0', + # New providers are added to the front of @directoryProviders because + # DefaultDirectoryProvider is a catch-all that will always provide a Directory. + (provider) => @directoryProviders.unshift(provider)) + # Mapping from the real path of a {Directory} to a {Promise} that resolves # to either a {Repository} or null. Ideally, the {Directory} would be used # as the key; however, there can be multiple {Directory} objects created for @@ -182,22 +190,22 @@ class Project extends Model Grim.deprecate("Use ::setPaths instead") @setPaths([path]) - # Public: Add a path the project's list of root paths + # Public: Add a path to the project's list of root paths # # * `projectPath` {String} The path to the directory to add. addPath: (projectPath, options) -> - projectPath = path.normalize(projectPath) + for directory in @getDirectories() + # Apparently a Directory does not believe it can contain itself, so we + # must also check whether the paths match. + return if directory.contains(projectPath) or directory.getPath() is projectPath - directoryPath = if fs.isDirectorySync(projectPath) - projectPath - else - path.dirname(projectPath) - - return if @getPaths().some (existingPath) -> - (directoryPath is existingPath) or - (directoryPath.indexOf(path.join(existingPath, path.sep)) is 0) - - directory = new Directory(directoryPath) + directory = null + for provider in @directoryProviders + break if directory = provider.directoryForURISync?(projectPath) + if directory is null + # This should never happen because DefaultDirectoryProvider should always + # return a Directory. + throw new Error(projectPath + ' could not be resolved to a directory') @rootDirectories.push(directory) repo = null From 461cd8c5fe887ca42a215fade7cfa7dc855c2e16 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 12 Feb 2015 15:47:09 -0800 Subject: [PATCH 2/6] Introduce atom.directory-provider service. A `Project` will always have a `DefaultDirectoryProvider` that will be used if there are no other `DirectoryProvider` objects that can produce a `Directory` for a path. --- spec/default-directory-provider-spec.coffee | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/spec/default-directory-provider-spec.coffee b/spec/default-directory-provider-spec.coffee index 63da18523..e0726a282 100644 --- a/spec/default-directory-provider-spec.coffee +++ b/spec/default-directory-provider-spec.coffee @@ -11,6 +11,25 @@ describe "DefaultDirectoryProvider", -> directory = provider.directoryForURISync(tmp) expect(directory.getPath()).toEqual tmp + it "normalizes its input before creating a Directory for it", -> + provider = new DefaultDirectoryProvider() + tmp = temp.mkdirSync() + nonNormalizedPath = tmp + path.sep + ".." + path.sep + path.basename(tmp) + expect(tmp.contains("..")).toBe false + expect(nonNormalizedPath.contains("..")).toBe true + + directory = provider.directoryForURISync(nonNormalizedPath) + expect(directory.getPath()).toEqual tmp + + it "creates a Directory for its parent dir when passed a file", -> + provider = new DefaultDirectoryProvider() + tmp = temp.mkdirSync() + file = path.join(tmp, 'example.txt') + fs.writeFileSync(file, 'data') + + directory = provider.directoryForURISync(file) + expect(directory.getPath()).toEqual tmp + describe ".directoryForURI(uri)", -> it "returns a Promise that resolves to a Directory with a path that matches the uri", -> provider = new DefaultDirectoryProvider() From ede049554c04693188432051c4aa85b217d70a66 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 19 Feb 2015 20:28:36 -0800 Subject: [PATCH 3/6] formatting fixes --- src/default-directory-provider.coffee | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/default-directory-provider.coffee b/src/default-directory-provider.coffee index a05e006f8..7ecdbd354 100644 --- a/src/default-directory-provider.coffee +++ b/src/default-directory-provider.coffee @@ -8,7 +8,8 @@ class DefaultDirectoryProvider # Public: Create a Directory that corresponds to the specified URI. # # * `uri` {String} The path to the directory to add. This is guaranteed not to - # be contained by a {Directory} in `atom.project`. + # be contained by a {Directory} in `atom.project`. + # # Returns: # * {Directory} if the given URI is compatible with this provider. # * `null` if the given URI is not compatibile with this provider. @@ -25,7 +26,8 @@ class DefaultDirectoryProvider # Public: Create a Directory that corresponds to the specified URI. # # * `uri` {String} The path to the directory to add. This is guaranteed not to - # be contained by a {Directory} in `atom.project`. + # be contained by a {Directory} in `atom.project`. + # # Returns a Promise that resolves to: # * {Directory} if the given URI is compatible with this provider. # * `null` if the given URI is not compatibile with this provider. From bf9c4132b217d75a7d7c6275d88a14ff04444c48 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 19 Feb 2015 21:02:31 -0800 Subject: [PATCH 4/6] Create a comprehensive test for the new behavior in Project. --- spec/default-directory-provider-spec.coffee | 4 +- spec/project-spec.coffee | 70 +++++++++++++++++++++ src/project.coffee | 2 +- 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/spec/default-directory-provider-spec.coffee b/spec/default-directory-provider-spec.coffee index e0726a282..780f2afd5 100644 --- a/spec/default-directory-provider-spec.coffee +++ b/spec/default-directory-provider-spec.coffee @@ -24,8 +24,8 @@ describe "DefaultDirectoryProvider", -> it "creates a Directory for its parent dir when passed a file", -> provider = new DefaultDirectoryProvider() tmp = temp.mkdirSync() - file = path.join(tmp, 'example.txt') - fs.writeFileSync(file, 'data') + file = path.join(tmp, "example.txt") + fs.writeFileSync(file, "data") directory = provider.directoryForURISync(file) expect(directory.getPath()).toEqual tmp diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 956895952..6944ac168 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -14,6 +14,76 @@ describe "Project", -> atom.project.setPaths([atom.project.getDirectories()[0]?.resolve('dir')]) describe "constructor", -> + it "enables a custom DirectoryProvider to supersede the DefaultDirectoryProvider", -> + remotePath = "ssh://foreign-directory:8080/" + class DummyDirectory + constructor: (@path) -> + getPath: -> @path + getFile: -> existsSync: -> false + getSubdirectory: -> existsSync: -> false + isRoot: -> true + off: -> + contains: (filePath) -> filePath.startsWith(remotePath) + + directoryProvider = + directoryForURISync: (uri) -> + if uri.startsWith("ssh://") + new DummyDirectory(uri) + else + null + directoryForURI: (uri) -> throw new Error("This should not be called.") + atom.packages.serviceHub.provide( + "atom.directory-provider", "0.1.0", directoryProvider) + + expect(atom.project.directoryProviders.length).toBe 2 + expect(atom.project.directoryProviders[0]).toBe directoryProvider + + tmp = temp.mkdirSync() + atom.project.setPaths([tmp, remotePath]) + directories = atom.project.getDirectories() + expect(directories.length).toBe 2 + + localDirectory = directories[0] + expect(localDirectory.getPath()).toBe tmp + expect(localDirectory instanceof Directory).toBe true + + dummyDirectory = directories[1] + expect(dummyDirectory.getPath()).toBe remotePath + expect(dummyDirectory instanceof DummyDirectory).toBe true + + expect(atom.project.getPaths()).toEqual([tmp, remotePath]) + + # Make sure that DummyDirectory.contains() is honored. + remotePathSubdirectory = remotePath + "a/subdirectory" + atom.project.addPath(remotePathSubdirectory) + expect(atom.project.getDirectories().length).toBe 2 + + # Make sure that a new DummyDirectory that is not contained by the first + # DummyDirectory can be added. + otherRemotePath = "ssh://other-foreign-directory:8080/" + atom.project.addPath(otherRemotePath) + newDirectories = atom.project.getDirectories() + expect(newDirectories.length).toBe 3 + otherDummyDirectory = newDirectories[2] + expect(otherDummyDirectory.getPath()).toBe otherRemotePath + expect(otherDummyDirectory instanceof DummyDirectory).toBe true + + it "a custom DirectoryProvider that returns null defaults to the DefaultDirectoryProvider", -> + directoryProvider = + directoryForURISync: (uri) -> null + directoryForURI: (uri) -> throw new Error("This should not be called.") + atom.packages.serviceHub.provide( + "atom.directory-provider", "0.1.0", directoryProvider) + + expect(atom.project.directoryProviders.length).toBe 2 + expect(atom.project.directoryProviders[0]).toBe directoryProvider + + tmp = temp.mkdirSync() + atom.project.setPaths([tmp]) + directories = atom.project.getDirectories() + expect(directories.length).toBe 1 + expect(directories[0].getPath()).toBe tmp + it "tries to update repositories when a new RepositoryProvider is registered", -> tmp = temp.mkdirSync('atom-project') atom.project.setPaths([tmp]) diff --git a/src/project.coffee b/src/project.coffee index 1f17e6610..d3fbd88eb 100644 --- a/src/project.coffee +++ b/src/project.coffee @@ -167,7 +167,7 @@ class Project extends Model # Public: Get an {Array} of {String}s containing the paths of the project's # directories. - getPaths: -> rootDirectory.path for rootDirectory in @rootDirectories + getPaths: -> rootDirectory.getPath() for rootDirectory in @rootDirectories getPath: -> Grim.deprecate("Use ::getPaths instead") @getPaths()[0] From 54c70706480a2f67a3a7ddb2c16fd0f9506cc362 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 19 Feb 2015 21:16:30 -0800 Subject: [PATCH 5/6] kill assertions about directoryProviders --- spec/project-spec.coffee | 6 ------ 1 file changed, 6 deletions(-) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 6944ac168..29329dfee 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -35,9 +35,6 @@ describe "Project", -> atom.packages.serviceHub.provide( "atom.directory-provider", "0.1.0", directoryProvider) - expect(atom.project.directoryProviders.length).toBe 2 - expect(atom.project.directoryProviders[0]).toBe directoryProvider - tmp = temp.mkdirSync() atom.project.setPaths([tmp, remotePath]) directories = atom.project.getDirectories() @@ -75,9 +72,6 @@ describe "Project", -> atom.packages.serviceHub.provide( "atom.directory-provider", "0.1.0", directoryProvider) - expect(atom.project.directoryProviders.length).toBe 2 - expect(atom.project.directoryProviders[0]).toBe directoryProvider - tmp = temp.mkdirSync() atom.project.setPaths([tmp]) directories = atom.project.getDirectories() From d5abd8764352f1ad62c40003f05638d6bc06e5c8 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 19 Feb 2015 21:35:07 -0800 Subject: [PATCH 6/6] reword it() message --- spec/project-spec.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 29329dfee..466431faf 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -65,7 +65,7 @@ describe "Project", -> expect(otherDummyDirectory.getPath()).toBe otherRemotePath expect(otherDummyDirectory instanceof DummyDirectory).toBe true - it "a custom DirectoryProvider that returns null defaults to the DefaultDirectoryProvider", -> + it "uses the default directory provider if no custom provider can handle the URI", -> directoryProvider = directoryForURISync: (uri) -> null directoryForURI: (uri) -> throw new Error("This should not be called.")