Merge pull request #9524 from atom/as-transact-async

Introduce Config::transactAsync

spec/config-spec.coffee

@@ -589,6 +589,59 @@ describe "Config", ->
       atom.config.transact ->
       expect(changeSpy).not.toHaveBeenCalled()
 
+  describe ".transactAsync(callback)", ->
+    changeSpy = null
+
+    beforeEach ->
+      changeSpy = jasmine.createSpy('onDidChange callback')
+      atom.config.onDidChange("foo.bar.baz", changeSpy)
+
+    it "allows only one change event for the duration of the given promise if it gets resolved", ->
+      promiseResult = null
+      transactionPromise = atom.config.transactAsync ->
+        atom.config.set("foo.bar.baz", 1)
+        atom.config.set("foo.bar.baz", 2)
+        atom.config.set("foo.bar.baz", 3)
+        Promise.resolve("a result")
+
+      waitsForPromise -> transactionPromise.then (r) -> promiseResult = r
+
+      runs ->
+        expect(promiseResult).toBe("a result")
+        expect(changeSpy.callCount).toBe(1)
+        expect(changeSpy.argsForCall[0][0]).toEqual(newValue: 3, oldValue: undefined)
+
+    it "allows only one change event for the duration of the given promise if it gets rejected", ->
+      promiseError = null
+      transactionPromise = atom.config.transactAsync ->
+        atom.config.set("foo.bar.baz", 1)
+        atom.config.set("foo.bar.baz", 2)
+        atom.config.set("foo.bar.baz", 3)
+        Promise.reject("an error")
+
+      waitsForPromise -> transactionPromise.catch (e) -> promiseError = e
+
+      runs ->
+        expect(promiseError).toBe("an error")
+        expect(changeSpy.callCount).toBe(1)
+        expect(changeSpy.argsForCall[0][0]).toEqual(newValue: 3, oldValue: undefined)
+
+    it "allows only one change event even when the given callback throws", ->
+      error = new Error("Oops!")
+      promiseError = null
+      transactionPromise = atom.config.transactAsync ->
+        atom.config.set("foo.bar.baz", 1)
+        atom.config.set("foo.bar.baz", 2)
+        atom.config.set("foo.bar.baz", 3)
+        throw error
+
+      waitsForPromise -> transactionPromise.catch (e) -> promiseError = e
+
+      runs ->
+        expect(promiseError).toBe(error)
+        expect(changeSpy.callCount).toBe(1)
+        expect(changeSpy.argsForCall[0][0]).toEqual(newValue: 3, oldValue: undefined)
+
   describe ".getSources()", ->
     it "returns an array of all of the config's source names", ->
       expect(atom.config.getSources()).toEqual([])

src/config.coffee

@@ -677,6 +677,34 @@ class Config
     finally
       @endTransaction()
 
+  ###
+  Section: Internal methods used by core
+  ###
+
+  # Private: Suppress calls to handler functions registered with {::onDidChange}
+  # and {::observe} for the duration of the {Promise} returned by `callback`.
+  # After the {Promise} is either resolved or rejected, handlers will be called
+  # once if the value for their key-path has changed.
+  #
+  # * `callback` {Function} that returns a {Promise}, which will be executed
+  #   while suppressing calls to handlers.
+  #
+  # Returns a {Promise} that is either resolved or rejected according to the
+  # `{Promise}` returned by `callback`. If `callback` throws an error, a
+  # rejected {Promise} will be returned instead.
+  transactAsync: (callback) ->
+    @beginTransaction()
+    try
+      endTransaction = (fn) => (args...) =>
+        @endTransaction()
+        fn(args...)
+      result = callback()
+      new Promise (resolve, reject) =>
+        result.then(endTransaction(resolve)).catch(endTransaction(reject))
+    catch error
+      @endTransaction()
+      Promise.reject(error)
+
   beginTransaction: ->
     @transactDepth++
 
@@ -684,10 +712,6 @@ class Config
     @transactDepth--
     @emitChangeEvent()
 
-  ###
-  Section: Internal methods used by core
-  ###
-
   pushAtKeyPath: (keyPath, value) ->
     arrayValue = @get(keyPath) ? []
     result = arrayValue.push(value)

src/package-manager.coffee

@@ -418,13 +418,11 @@ class PackageManager
 
   activatePackages: (packages) ->
     promises = []
-    @config.beginTransaction()
-    for pack in packages
-      promise = @activatePackage(pack.name)
-      promises.push(promise) unless pack.activationShouldBeDeferred()
-    Promise.all(promises)
-      .then(=> @config.endTransaction())
-      .catch(=> @config.endTransaction())
+    @config.transactAsync =>
+      for pack in packages
+        promise = @activatePackage(pack.name)
+        promises.push(promise) unless pack.activationShouldBeDeferred()
+      Promise.all(promises)
     @observeDisabledPackages()
     @observePackagesWithKeymapsDisabled()
     promises