From d1c44dcb54994eb733eff0bfde69c8454aeff22c Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 18 Apr 2015 12:39:20 +1200 Subject: [PATCH] Never load config settings from disk when a save is pending MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #5771 We load the user’s settings from disk when we detect a change to their config.cson file. However, if there’s a save pending, doing this will end up blowing away the values we intend to save. --- spec/config-spec.coffee | 17 +++++++++++++++++ src/config.coffee | 24 ++++++++++++++++-------- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/spec/config-spec.coffee b/spec/config-spec.coffee index b3af6f931..2c5ba2fc2 100644 --- a/spec/config-spec.coffee +++ b/spec/config-spec.coffee @@ -486,6 +486,7 @@ describe "Config", -> atom.config.set('foo.bar.baz', "value 1") expect(observeHandler).toHaveBeenCalledWith("value 1") + advanceClock(100) # complete pending save that was requested in ::set observeHandler.reset() atom.config.loadUserConfig() @@ -789,6 +790,22 @@ describe "Config", -> expect(warnSpy).toHaveBeenCalled() expect(warnSpy.mostRecentCall.args[0]).toContain "foo.int" + describe "when there is a pending save", -> + it "does not change the config settings", -> + fs.writeFileSync atom.config.configFilePath, "'*': foo: bar: 'baz'" + + atom.config.set("foo.bar", "quux") + atom.config.loadUserConfig() + expect(atom.config.get("foo.bar")).toBe "quux" + + advanceClock(100) + expect(atom.config.save.callCount).toBe 1 + + expect(atom.config.get("foo.bar")).toBe "quux" + atom.config.loadUserConfig() + expect(atom.config.get("foo.bar")).toBe "baz" + + describe ".observeUserConfig()", -> updatedHandler = null diff --git a/src/config.coffee b/src/config.coffee index 6c440f219..b97ce99ec 100644 --- a/src/config.coffee +++ b/src/config.coffee @@ -332,9 +332,16 @@ class Config @configFilePath = fs.resolve(@configDirPath, 'config', ['json', 'cson']) @configFilePath ?= path.join(@configDirPath, 'config.cson') @transactDepth = 0 + @savePending = false - @debouncedSave = _.debounce(@save, 100) - @debouncedLoad = _.debounce(@loadUserConfig, 100) + @requestLoad = _.debounce(@loadUserConfig, 100) + @requestSave = => + @savePending = true + debouncedSave.call(this) + save = => + @savePending = false + @save() + debouncedSave = _.debounce(save, 100) ### Section: Config Subscription @@ -605,7 +612,7 @@ class Config else @setRawValue(keyPath, value) - @debouncedSave() if source is @getUserConfigPath() and shouldSave and not @configFileHasErrors + @requestSave() if source is @getUserConfigPath() and shouldSave and not @configFileHasErrors true # Essential: Restore the setting at `keyPath` to its default value. @@ -635,7 +642,7 @@ class Config _.setValueForKeyPath(settings, keyPath, undefined) settings = withoutEmptyObjects(settings) @set(null, settings, {scopeSelector, source, priority: @priorityForSource(source)}) if settings? - @debouncedSave() + @requestSave() else @scopedSettingsStore.removePropertiesForSourceAndSelector(source, scopeSelector) @emitChangeEvent() @@ -757,9 +764,10 @@ class Config CSON.writeFileSync(@configFilePath, {}) try - userConfig = CSON.readFileSync(@configFilePath) - @resetUserSettings(userConfig) - @configFileHasErrors = false + unless @savePending + userConfig = CSON.readFileSync(@configFilePath) + @resetUserSettings(userConfig) + @configFileHasErrors = false catch error @configFileHasErrors = true message = "Failed to load `#{path.basename(@configFilePath)}`" @@ -776,7 +784,7 @@ class Config observeUserConfig: -> try @watchSubscription ?= pathWatcher.watch @configFilePath, (eventType) => - @debouncedLoad() if eventType is 'change' and @watchSubscription? + @requestLoad() if eventType is 'change' and @watchSubscription? catch error @notifyFailure """ Unable to watch path: `#{path.basename(@configFilePath)}`. Make sure you have permissions to