From b0731afd4ce32c6e23d518000866f2c65aad1207 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 17 Dec 2014 12:03:43 -0800 Subject: [PATCH 1/2] Don't pass keyPath to Config::onDidChange callback The keyPath field was never used by core or any package, and for scoped settings, its value was always equal to the keyPath specified by the caller. --- spec/config-spec.coffee | 32 +++++++++++++++++++++----------- src/config.coffee | 25 +++++++++++++------------ 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/spec/config-spec.coffee b/spec/config-spec.coffee index 324db9d8a..db5146507 100644 --- a/spec/config-spec.coffee +++ b/spec/config-spec.coffee @@ -406,11 +406,11 @@ describe "Config", -> 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', keyPath: 'foo.bar.baz'}) + expect(observeHandler).toHaveBeenCalledWith({newValue: 'value 2', oldValue: 'value 1'}) observeHandler.reset() atom.config.set('foo.bar.baz', "value 1") - expect(observeHandler).toHaveBeenCalledWith({newValue: 'value 1', oldValue: 'value 2', keyPath: 'foo.bar.baz'}) + expect(observeHandler).toHaveBeenCalledWith({newValue: 'value 1', oldValue: 'value 2'}) describe 'when a keyPath is not specified', -> beforeEach -> @@ -424,15 +424,21 @@ describe "Config", -> it "fires the callback every time any 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', keyPath: 'foo.bar.baz'}) + expect(observeHandler).toHaveBeenCalled() + expect(observeHandler.mostRecentCall.args[0].newValue.foo.bar.baz).toBe("value 2") + expect(observeHandler.mostRecentCall.args[0].oldValue.foo.bar.baz).toBe("value 1") observeHandler.reset() atom.config.set('foo.bar.baz', "value 1") - expect(observeHandler).toHaveBeenCalledWith({newValue: 'value 1', oldValue: 'value 2', keyPath: 'foo.bar.baz'}) + expect(observeHandler).toHaveBeenCalled() + expect(observeHandler.mostRecentCall.args[0].newValue.foo.bar.baz).toBe("value 1") + expect(observeHandler.mostRecentCall.args[0].oldValue.foo.bar.baz).toBe("value 2") observeHandler.reset() atom.config.set('foo.bar.int', 1) - expect(observeHandler).toHaveBeenCalledWith({newValue: 1, oldValue: undefined, keyPath: 'foo.bar.int'}) + expect(observeHandler).toHaveBeenCalled() + expect(observeHandler.mostRecentCall.args[0].newValue.foo.bar.int).toBe(1) + expect(observeHandler.mostRecentCall.args[0].oldValue.foo.bar.int).toBe(undefined) describe ".observe(keyPath)", -> [observeHandler, observeSubscription] = [] @@ -454,6 +460,10 @@ describe "Config", -> atom.config.set('foo.bar.baz', "value 1") expect(observeHandler).toHaveBeenCalledWith("value 1") + observeHandler.reset() + atom.config.loadUserConfig() + expect(observeHandler).toHaveBeenCalledWith(undefined) + it "fires the callback when the observed value is deleted", -> observeHandler.reset() # clear the initial call atom.config.set('foo.bar.baz', undefined) @@ -1185,25 +1195,25 @@ describe "Config", -> atom.config.onDidChange [".source.coffee", ".string.quoted.double.coffee"], keyPath, changeSpy = jasmine.createSpy() atom.config.set("foo.bar.baz", 12) - expect(changeSpy).toHaveBeenCalledWith({oldValue: undefined, newValue: 12, keyPath}) + expect(changeSpy).toHaveBeenCalledWith({oldValue: undefined, newValue: 12}) changeSpy.reset() disposable1 = atom.config.addScopedSettings("a", ".source .string.quoted.double", foo: bar: baz: 22) - expect(changeSpy).toHaveBeenCalledWith({oldValue: 12, newValue: 22, keyPath}) + expect(changeSpy).toHaveBeenCalledWith({oldValue: 12, newValue: 22}) changeSpy.reset() disposable2 = atom.config.addScopedSettings("b", ".source.coffee .string.quoted.double.coffee", foo: bar: baz: 42) - expect(changeSpy).toHaveBeenCalledWith({oldValue: 22, newValue: 42, keyPath}) + expect(changeSpy).toHaveBeenCalledWith({oldValue: 22, newValue: 42}) changeSpy.reset() disposable2.dispose() - expect(changeSpy).toHaveBeenCalledWith({oldValue: 42, newValue: 22, keyPath}) + expect(changeSpy).toHaveBeenCalledWith({oldValue: 42, newValue: 22}) changeSpy.reset() disposable1.dispose() - expect(changeSpy).toHaveBeenCalledWith({oldValue: 22, newValue: 12, keyPath}) + expect(changeSpy).toHaveBeenCalledWith({oldValue: 22, newValue: 12}) changeSpy.reset() atom.config.set("foo.bar.baz", undefined) - expect(changeSpy).toHaveBeenCalledWith({oldValue: 12, newValue: undefined, keyPath}) + expect(changeSpy).toHaveBeenCalledWith({oldValue: 12, newValue: undefined}) changeSpy.reset() diff --git a/src/config.coffee b/src/config.coffee index dcad15b56..821e17243 100644 --- a/src/config.coffee +++ b/src/config.coffee @@ -800,19 +800,22 @@ class Config defaultValue = _.valueForKeyPath(@defaultSettings, keyPath) value = undefined if _.isEqual(defaultValue, value) - oldValue = _.clone(@get(keyPath)) _.setValueForKeyPath(@settings, keyPath, value) - newValue = @get(keyPath) - @emitter.emit 'did-change', {oldValue, newValue, keyPath} unless _.isEqual(newValue, oldValue) + @emitter.emit 'did-change' observeKeyPath: (keyPath, options, callback) -> - callback(_.clone(@get(keyPath))) unless options.callNow == false - @emitter.on 'did-change', (event) => - callback(event.newValue) if keyPath? and @isSubKeyPath(keyPath, event?.keyPath) + callback(@get(keyPath)) + @onDidChangeKeyPath keyPath, (event) -> callback(event.newValue) onDidChangeKeyPath: (keyPath, callback) -> - @emitter.on 'did-change', (event) => - callback(event) if not keyPath? or (keyPath? and @isSubKeyPath(keyPath, event?.keyPath)) + oldValue = @get(keyPath) + + didChange = => + newValue = @get(keyPath) + callback({oldValue, newValue}) unless _.isEqual(oldValue, newValue) + oldValue = newValue + + @emitter.on 'did-change', didChange isSubKeyPath: (keyPath, subKeyPath) -> return false unless keyPath? and subKeyPath? @@ -821,10 +824,8 @@ class Config _.isEqual(pathTokens, pathSubTokens) setRawDefault: (keyPath, value) -> - oldValue = _.clone(@get(keyPath)) _.setValueForKeyPath(@defaultSettings, keyPath, value) - newValue = @get(keyPath) - @emitter.emit 'did-change', {oldValue, newValue, keyPath} unless _.isEqual(newValue, oldValue) + @emitter.emit 'did-change' setDefaults: (keyPath, defaults) -> if defaults? and isPlainObject(defaults) @@ -928,7 +929,7 @@ class Config oldValue = @get(scopeDescriptor, keyPath) didChange = => newValue = @get(scopeDescriptor, keyPath) - callback({oldValue, newValue, keyPath}) unless _.isEqual(oldValue, newValue) + callback({oldValue, newValue}) unless _.isEqual(oldValue, newValue) oldValue = newValue @emitter.on 'did-change', didChange From 28ac51d1406ca90ecc1e923e4020d255fb14c463 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 17 Dec 2014 12:21:42 -0800 Subject: [PATCH 2/2] Add Config::transact Use this method to avoid emitting unecessary config events when activating or deactivating multiple packages --- spec/config-spec.coffee | 20 ++++++++++++++++++++ src/config.coffee | 31 ++++++++++++++++++++++++------- src/package-manager.coffee | 6 ++++-- 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/spec/config-spec.coffee b/spec/config-spec.coffee index db5146507..f0b13cdf8 100644 --- a/spec/config-spec.coffee +++ b/spec/config-spec.coffee @@ -492,6 +492,26 @@ describe "Config", -> atom.config.set('foo.bar.baz', "value 10") expect(bazCatHandler).not.toHaveBeenCalled() + describe ".transact(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 callback", -> + atom.config.transact -> + atom.config.set("foo.bar.baz", 1) + atom.config.set("foo.bar.baz", 2) + atom.config.set("foo.bar.baz", 3) + + expect(changeSpy.callCount).toBe(1) + expect(changeSpy.argsForCall[0][0]).toEqual(newValue: 3, oldValue: undefined) + + it "does not emit an event if no changes occur while paused", -> + atom.config.transact -> + expect(changeSpy).not.toHaveBeenCalled() + describe ".initializeConfigDirectory()", -> beforeEach -> if fs.existsSync(dotAtomPath) diff --git a/src/config.coffee b/src/config.coffee index 821e17243..acc01806a 100644 --- a/src/config.coffee +++ b/src/config.coffee @@ -318,6 +318,7 @@ class Config @configFileHasErrors = false @configFilePath = fs.resolve(@configDirPath, 'config', ['json', 'cson']) @configFilePath ?= path.join(@configDirPath, 'config.cson') + @transactDepth = 0 ### Section: Config Subscription @@ -614,6 +615,19 @@ class Config getUserConfigPath: -> @configFilePath + # Extended: Suppress calls to handler functions registered with {::onDidChange} + # and {::observe} for the duration of `callback`. After `callback` executes, + # handlers will be called once if the value for their key-path has changed. + # + # * `callback` {Function} to execute while suppressing calls to handlers. + transact: (callback) -> + @transactDepth++ + try + callback() + finally + @transactDepth-- + @emitChangeEvent() + ### Section: Deprecated ### @@ -749,7 +763,7 @@ class Config resetUserSettings: (newSettings) -> unless isPlainObject(newSettings) @settings = {} - @emitter.emit 'did-change' + @emitChangeEvent() return if newSettings.global? @@ -801,7 +815,7 @@ class Config value = undefined if _.isEqual(defaultValue, value) _.setValueForKeyPath(@settings, keyPath, value) - @emitter.emit 'did-change' + @emitChangeEvent() observeKeyPath: (keyPath, options, callback) -> callback(@get(keyPath)) @@ -825,7 +839,7 @@ class Config setRawDefault: (keyPath, value) -> _.setValueForKeyPath(@defaultSettings, keyPath, value) - @emitter.emit 'did-change' + @emitChangeEvent() setDefaults: (keyPath, defaults) -> if defaults? and isPlainObject(defaults) @@ -883,20 +897,23 @@ class Config Section: Private Scoped Settings ### + emitChangeEvent: -> + @emitter.emit 'did-change' unless @transactDepth > 0 + resetUserScopedSettings: (newScopedSettings) -> @usersScopedSettings?.dispose() @usersScopedSettings = new CompositeDisposable @usersScopedSettings.add @scopedSettingsStore.addProperties('user-config', newScopedSettings, @usersScopedSettingPriority) - @emitter.emit 'did-change' + @emitChangeEvent() addScopedSettings: (source, selector, value, options) -> settingsBySelector = {} settingsBySelector[selector] = value disposable = @scopedSettingsStore.addProperties(source, settingsBySelector, options) - @emitter.emit 'did-change' + @emitChangeEvent() new Disposable => disposable.dispose() - @emitter.emit 'did-change' + @emitChangeEvent() setRawScopedValue: (selector, keyPath, value) -> if keyPath? @@ -907,7 +924,7 @@ class Config settingsBySelector = {} settingsBySelector[selector] = value @usersScopedSettings.add @scopedSettingsStore.addProperties('user-config', settingsBySelector, @usersScopedSettingPriority) - @emitter.emit 'did-change' + @emitChangeEvent() getRawScopedValue: (scopeDescriptor, keyPath) -> scopeDescriptor = ScopeDescriptor.fromObject(scopeDescriptor) diff --git a/src/package-manager.coffee b/src/package-manager.coffee index 05fa0f637..31b9046a2 100644 --- a/src/package-manager.coffee +++ b/src/package-manager.coffee @@ -329,7 +329,8 @@ class PackageManager @packageActivators.push([activator, types]) activatePackages: (packages) -> - @activatePackage(pack.name) for pack in packages + atom.config.transact => + @activatePackage(pack.name) for pack in packages @observeDisabledPackages() # Activate a single package by name @@ -344,7 +345,8 @@ class PackageManager # Deactivate all packages deactivatePackages: -> - @deactivatePackage(pack.name) for pack in @getLoadedPackages() + atom.config.transact => + @deactivatePackage(pack.name) for pack in @getLoadedPackages() @unobserveDisabledPackages() # Deactivate the package with the given name