From a279db5568707e5c17009735eea4d72295a0eed8 Mon Sep 17 00:00:00 2001 From: joshaber Date: Wed, 27 Apr 2016 12:50:13 -0400 Subject: [PATCH 1/7] Failing test. --- spec/git-spec.coffee | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/git-spec.coffee b/spec/git-spec.coffee index 7d9e9bbd4..82e371146 100644 --- a/spec/git-spec.coffee +++ b/spec/git-spec.coffee @@ -289,6 +289,16 @@ describe "GitRepository", -> expect(repo.isStatusModified(status)).toBe true expect(repo.isStatusNew(status)).toBe false + it 'caches statuses that were looked up synchronously', -> + originalContent = 'undefined' + fs.writeFileSync(modifiedPath, 'making this path modified') + repo.getPathStatus('file.txt') + + fs.writeFileSync(modifiedPath, originalContent) + waitsForPromise -> repo.refreshStatus() + runs -> + expect(repo.isStatusModified(repo.getCachedPathStatus(modifiedPath))).toBeFalsy() + describe "buffer events", -> [editor] = [] From 69e97204d51ec412f0b81acf1eb89369aed08a15 Mon Sep 17 00:00:00 2001 From: joshaber Date: Wed, 27 Apr 2016 12:50:27 -0400 Subject: [PATCH 2/7] Test for undefinedness instead of 0. --- src/git-repository.coffee | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/git-repository.coffee b/src/git-repository.coffee index a04124b78..cea000efc 100644 --- a/src/git-repository.coffee +++ b/src/git-repository.coffee @@ -369,7 +369,10 @@ class GitRepository @getCachedRelativePathStatus(relativePath) getCachedRelativePathStatus: (relativePath) -> - @statusesByPath[relativePath] ? @async.getCachedPathStatuses()[relativePath] + cachedStatus = @statusesByPath[relativePath] + return cachedStatus if cachedStatus? + + @async.getCachedPathStatuses()[relativePath] # Public: Returns true if the given status indicates modification. # From c0adf125ecaf2100a4e3bd584565d5c6c5435fad Mon Sep 17 00:00:00 2001 From: joshaber Date: Wed, 27 Apr 2016 16:05:00 -0400 Subject: [PATCH 3/7] Reset the cache before calling the callback. --- src/git-repository.coffee | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/git-repository.coffee b/src/git-repository.coffee index cea000efc..28455c983 100644 --- a/src/git-repository.coffee +++ b/src/git-repository.coffee @@ -166,12 +166,10 @@ class GitRepository # # Returns a {Disposable} on which `.dispose()` can be called to unsubscribe. onDidChangeStatuses: (callback) -> - @async.onDidChangeStatuses -> - # Defer the callback to the next tick so that we've reset - # `@statusesByPath` by the time it's called. Otherwise reads from within - # the callback could be inconsistent. - # See https://github.com/atom/atom/issues/11396 - process.nextTick callback + @async.onDidChangeStatuses => + @branch = @async?.branch + @statusesByPath = {} + callback() ### Section: Repository Details @@ -369,10 +367,7 @@ class GitRepository @getCachedRelativePathStatus(relativePath) getCachedRelativePathStatus: (relativePath) -> - cachedStatus = @statusesByPath[relativePath] - return cachedStatus if cachedStatus? - - @async.getCachedPathStatuses()[relativePath] + @statusesByPath[relativePath] ? @async.getCachedPathStatuses()[relativePath] # Public: Returns true if the given status indicates modification. # @@ -499,10 +494,7 @@ class GitRepository # # Returns a promise that resolves when the repository has been refreshed. refreshStatus: -> - asyncRefresh = @async.refreshStatus().then => - @statusesByPath = {} - @branch = @async?.branch - + asyncRefresh = @async.refreshStatus() syncRefresh = new Promise (resolve, reject) => @handlerPath ?= require.resolve('./repository-status-handler') From 630b8c69a603e69351864bb6fd851e1e9b4ef063 Mon Sep 17 00:00:00 2001 From: joshaber Date: Wed, 27 Apr 2016 16:24:29 -0400 Subject: [PATCH 4/7] Fix the test. --- spec/git-spec.coffee | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/spec/git-spec.coffee b/spec/git-spec.coffee index 82e371146..88c5c9a86 100644 --- a/spec/git-spec.coffee +++ b/spec/git-spec.coffee @@ -295,7 +295,12 @@ describe "GitRepository", -> repo.getPathStatus('file.txt') fs.writeFileSync(modifiedPath, originalContent) - waitsForPromise -> repo.refreshStatus() + + statusHandler = jasmine.createSpy('statusHandler') + repo.onDidChangeStatuses statusHandler + repo.refreshStatus() + + waitsFor -> statusHandler.callCount > 0 runs -> expect(repo.isStatusModified(repo.getCachedPathStatus(modifiedPath))).toBeFalsy() From 4727d6611e53cb8454a77ad607973a2c8efa8885 Mon Sep 17 00:00:00 2001 From: joshaber Date: Wed, 27 Apr 2016 17:06:37 -0400 Subject: [PATCH 5/7] Revert "Fix the test." This reverts commit 630b8c69a603e69351864bb6fd851e1e9b4ef063. --- spec/git-spec.coffee | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/spec/git-spec.coffee b/spec/git-spec.coffee index 88c5c9a86..82e371146 100644 --- a/spec/git-spec.coffee +++ b/spec/git-spec.coffee @@ -295,12 +295,7 @@ describe "GitRepository", -> repo.getPathStatus('file.txt') fs.writeFileSync(modifiedPath, originalContent) - - statusHandler = jasmine.createSpy('statusHandler') - repo.onDidChangeStatuses statusHandler - repo.refreshStatus() - - waitsFor -> statusHandler.callCount > 0 + waitsForPromise -> repo.refreshStatus() runs -> expect(repo.isStatusModified(repo.getCachedPathStatus(modifiedPath))).toBeFalsy() From d11e30579b3203bc179018dacd91441ce37a4fff Mon Sep 17 00:00:00 2001 From: joshaber Date: Wed, 27 Apr 2016 17:17:09 -0400 Subject: [PATCH 6/7] Manually emit the change event. --- src/git-repository.coffee | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/git-repository.coffee b/src/git-repository.coffee index 28455c983..a92a03a89 100644 --- a/src/git-repository.coffee +++ b/src/git-repository.coffee @@ -166,10 +166,7 @@ class GitRepository # # Returns a {Disposable} on which `.dispose()` can be called to unsubscribe. onDidChangeStatuses: (callback) -> - @async.onDidChangeStatuses => - @branch = @async?.branch - @statusesByPath = {} - callback() + @emitter.on 'did-change-statuses', callback ### Section: Repository Details @@ -494,7 +491,23 @@ class GitRepository # # Returns a promise that resolves when the repository has been refreshed. refreshStatus: -> - asyncRefresh = @async.refreshStatus() + statusesChanged = false + subscription = @async.onDidChangeStatuses -> + subscription?.dispose() + subscription = null + + statusesChanged = true + + asyncRefresh = @async.refreshStatus().then => + subscription?.dispose() + subscription = null + + @branch = @async?.branch + @statusesByPath = {} + + if statusesChanged + @emitter.emit 'did-change-statuses' + syncRefresh = new Promise (resolve, reject) => @handlerPath ?= require.resolve('./repository-status-handler') From cd8b28da4da588b4bf63d410da6a14f96adbc8d9 Mon Sep 17 00:00:00 2001 From: joshaber Date: Wed, 27 Apr 2016 22:40:34 -0400 Subject: [PATCH 7/7] Document why --- src/git-repository.coffee | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/git-repository.coffee b/src/git-repository.coffee index a92a03a89..fcbc40830 100644 --- a/src/git-repository.coffee +++ b/src/git-repository.coffee @@ -492,6 +492,11 @@ class GitRepository # Returns a promise that resolves when the repository has been refreshed. refreshStatus: -> statusesChanged = false + + # Listen for `did-change-statuses` so we know if something changed. But we + # need to wait to propagate it until after we've set the branch and cleared + # the `statusesByPath` cache. So just set a flag, and we'll emit the event + # after refresh is done. subscription = @async.onDidChangeStatuses -> subscription?.dispose() subscription = null