From 388428b0740764fa55ae7909321adafc468b3efa Mon Sep 17 00:00:00 2001
From: Max Brunsfeld <maxbrunsfeld@gmail.com>
Date: Tue, 23 Dec 2014 16:20:37 -0800
Subject: [PATCH] Fix logic error when exception is thrown in config observer

---
 spec/config-spec.coffee | 12 +++++++++---
 src/config.coffee       | 21 ++++++++++-----------
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/spec/config-spec.coffee b/spec/config-spec.coffee
index 9640fae3e..4940678fe 100644
--- a/spec/config-spec.coffee
+++ b/spec/config-spec.coffee
@@ -493,13 +493,19 @@ describe "Config", ->
         expect(observeHandler).not.toHaveBeenCalled()
 
       it "fires the callback every time the observed value changes", ->
-        observeHandler.reset() # clear the initial call
         atom.config.set('foo.bar.baz', "value 2")
         expect(observeHandler).toHaveBeenCalledWith({newValue: 'value 2', oldValue: 'value 1'})
         observeHandler.reset()
 
-        atom.config.set('foo.bar.baz', "value 1")
+        observeHandler.andCallFake -> throw new Error("oops")
+        expect(-> atom.config.set('foo.bar.baz', "value 1")).toThrow("oops")
         expect(observeHandler).toHaveBeenCalledWith({newValue: 'value 1', oldValue: 'value 2'})
+        observeHandler.reset()
+
+        # Regression: exception in earlier handler shouldn't put observer
+        # into a bad state.
+        atom.config.set('something.else', "new value")
+        expect(observeHandler).not.toHaveBeenCalled()
 
     describe 'when a keyPath is not specified', ->
       beforeEach ->
@@ -567,7 +573,7 @@ describe "Config", ->
       atom.config.set("foo.bar.baz", "i'm back")
       expect(observeHandler).toHaveBeenCalledWith("i'm back")
 
-    it "does not fire the callback once the observe subscription is off'ed", ->
+    it "does not fire the callback once the subscription is disposed", ->
       observeHandler.reset() # clear the initial call
       observeSubscription.dispose()
       atom.config.set('foo.bar.baz', "value 2")
diff --git a/src/config.coffee b/src/config.coffee
index d0d8f7d2f..9db755f2f 100644
--- a/src/config.coffee
+++ b/src/config.coffee
@@ -882,13 +882,12 @@ class Config
 
   onDidChangeKeyPath: (keyPath, callback) ->
     oldValue = @get(keyPath)
-
-    didChange = =>
+    @emitter.on 'did-change', =>
       newValue = @get(keyPath)
-      callback({oldValue, newValue}) unless _.isEqual(oldValue, newValue)
-      oldValue = newValue
-
-    @emitter.on 'did-change', didChange
+      unless _.isEqual(oldValue, newValue)
+        event = {oldValue, newValue}
+        oldValue = newValue
+        callback(event)
 
   isSubKeyPath: (keyPath, subKeyPath) ->
     return false unless keyPath? and subKeyPath?
@@ -1003,12 +1002,12 @@ class Config
 
   onDidChangeScopedKeyPath: (scope, keyPath, callback) ->
     oldValue = @get(keyPath, {scope})
-    didChange = =>
+    @emitter.on 'did-change', =>
       newValue = @get(keyPath, {scope})
-      callback({oldValue, newValue}) unless _.isEqual(oldValue, newValue)
-      oldValue = newValue
-
-    @emitter.on 'did-change', didChange
+      unless _.isEqual(oldValue, newValue)
+        event = {oldValue, newValue}
+        oldValue = newValue
+        callback(event)
 
   # TODO: figure out how to change / remove this. The return value is awkward.
   # * language mode uses it for one thing.