From 50f8f8e7e9770a857ec534a7cfb9d6837ffb56ff Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 14 Mar 2016 16:28:36 -0400 Subject: [PATCH 1/5] Match GitRepository's responses to null paths. Fixes https://github.com/atom/git-diff/issues/93. --- src/git-repository-async.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/git-repository-async.js b/src/git-repository-async.js index f80f46a13..3b33a5003 100644 --- a/src/git-repository-async.js +++ b/src/git-repository-async.js @@ -442,6 +442,9 @@ export default class GitRepositoryAsync { // Returns a {Promise} which resolves to a {Boolean} that's true if the `path` // is ignored. isPathIgnored (_path) { + // NB: We're matching the behavior of `GitRepository` here. + if (!_path) return Promise.resolve(false) + return this.getRepo() .then(repo => { const relativePath = this.relativize(_path, repo.workdir()) @@ -518,6 +521,10 @@ export default class GitRepositoryAsync { // Returns a {Promise} which resolves to a status {Number} or null if the // path is not in the cache. getCachedPathStatus (_path) { + // NB: I don't love this, but we're matching the behavior of + // `GitRepository` here for API compatibility. + if (!_path) return null + return this.relativizeToWorkingDirectory(_path) .then(relativePath => this.pathStatusCache[relativePath]) } From a2a6ed05c5dcafa37668884f4b00bba9873b644e Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 14 Mar 2016 16:40:55 -0400 Subject: [PATCH 2/5] And again. --- src/git-repository-async.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/git-repository-async.js b/src/git-repository-async.js index 3b33a5003..d0903acd8 100644 --- a/src/git-repository-async.js +++ b/src/git-repository-async.js @@ -660,6 +660,11 @@ export default class GitRepositoryAsync { } return this._diffBlobToBuffer(blob, text, options) }) + .catch(e => { + // NB: I don't love this, but we're matching the behavior of + // `GitRepository` here for API compatibility. + return {} + }) } // Checking Out From 34698d57687a78ffe76136c3e69f95026c8fe400 Mon Sep 17 00:00:00 2001 From: joshaber Date: Tue, 15 Mar 2016 10:43:57 -0400 Subject: [PATCH 3/5] Revert "And again." This reverts commit a2a6ed05c5dcafa37668884f4b00bba9873b644e. --- src/git-repository-async.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/git-repository-async.js b/src/git-repository-async.js index d0903acd8..3b33a5003 100644 --- a/src/git-repository-async.js +++ b/src/git-repository-async.js @@ -660,11 +660,6 @@ export default class GitRepositoryAsync { } return this._diffBlobToBuffer(blob, text, options) }) - .catch(e => { - // NB: I don't love this, but we're matching the behavior of - // `GitRepository` here for API compatibility. - return {} - }) } // Checking Out From 8f9ab771a79d7b9f26604ed5333943de03e0b690 Mon Sep 17 00:00:00 2001 From: joshaber Date: Tue, 15 Mar 2016 10:44:00 -0400 Subject: [PATCH 4/5] Revert "Match GitRepository's responses to null paths." This reverts commit 50f8f8e7e9770a857ec534a7cfb9d6837ffb56ff. --- src/git-repository-async.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/git-repository-async.js b/src/git-repository-async.js index 3b33a5003..f80f46a13 100644 --- a/src/git-repository-async.js +++ b/src/git-repository-async.js @@ -442,9 +442,6 @@ export default class GitRepositoryAsync { // Returns a {Promise} which resolves to a {Boolean} that's true if the `path` // is ignored. isPathIgnored (_path) { - // NB: We're matching the behavior of `GitRepository` here. - if (!_path) return Promise.resolve(false) - return this.getRepo() .then(repo => { const relativePath = this.relativize(_path, repo.workdir()) @@ -521,10 +518,6 @@ export default class GitRepositoryAsync { // Returns a {Promise} which resolves to a status {Number} or null if the // path is not in the cache. getCachedPathStatus (_path) { - // NB: I don't love this, but we're matching the behavior of - // `GitRepository` here for API compatibility. - if (!_path) return null - return this.relativizeToWorkingDirectory(_path) .then(relativePath => this.pathStatusCache[relativePath]) } From 15b13e3ddc15d608d5c839a9ee160a3d1a6b56a8 Mon Sep 17 00:00:00 2001 From: joshaber Date: Tue, 15 Mar 2016 10:49:36 -0400 Subject: [PATCH 5/5] Note the changes to GitRepository. --- src/git-repository-async.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/git-repository-async.js b/src/git-repository-async.js index f80f46a13..c5984eed4 100644 --- a/src/git-repository-async.js +++ b/src/git-repository-async.js @@ -15,6 +15,12 @@ const submoduleMode = 57344 // TODO: compose this from libgit2 constants // Just using this for _.isEqual and _.object, we should impl our own here import _ from 'underscore-plus' +// For the most part, this class behaves the same as `GitRepository`, with a few +// notable differences: +// * Errors are generally propagated out to the caller instead of being +// swallowed within `GitRepositoryAsync`. +// * Methods accepting a path shouldn't be given a null path, unless it is +// specifically allowed as noted in the method's documentation. export default class GitRepositoryAsync { static open (path, options = {}) { // QUESTION: Should this wrap Git.Repository and reject with a nicer message?