New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid sending async updates on config update #18545

Merged
merged 1 commit into from Dec 6, 2018

Conversation

Projects
None yet
2 participants
@captbaritone
Copy link
Contributor

captbaritone commented Dec 5, 2018

In 7a5d727 the 'set-user-settings' callback was changed so that it no longer returned the promise created by ConfigFile.update(). This meant that the logic in ApplicateDelegate.onDidChangeUserSetting() which uses .pendingSettingUpdateCount to avoid triggering in response to calls to
ApplicationDelegate.setUserSettings was no longer effective.

I noticed this when updating settings via the setting view. If you type at just the right (wrong?) cadence, you'll notice that your input can get updated with stale values.

Something like this:

You type            "a"      "b"            "c"
config.set          "a"      "ab"           "ac"
config.observe                        "a"         "ab"    "ac"
Input value         "a"      "ab"     "a"   "ac"  "ab"    "ac"

It's possible that this is the same as atom/settings-view#1062

After this change typing in a settings input seems to behave as expected.

Avoid sending async updates on config update
In
7a5d727
the 'set-user-settings' callback was changed so that it nolonger
returned the promise created by `ConfigFile.update()`. This meant that
the logic in
[`ApplicateDelegate.onDidChangeUserSetting()`](https://github.com/atom/atom/blob/5f0231b/src/application-delegate.js#L193-L206) which uses
`.pendingSettingUpdateCount` to avoid triggering in response to calls to
ApplicationDelegate.setUserSettings` was no longer effective.

I noticed this when updating settings via the setting view. If you type
at just the right (wrong?) cadence, you'll notice that your input can
get updated with stale values.

Something like this:

```
You type            "a"      "b"            "c"
config.set          "a"      "ab"           "ac"
config.observe                        "a"         "ab"    "ac"
Input value         "a"      "ab"     "a"   "ac"  "ab"    "ac"
```

It's possible that is the same as atom/settings-view#1062

After this change typing in settings input seems to behave as expected.

@captbaritone captbaritone merged commit de363cf into atom:master Dec 6, 2018

3 checks passed

Atom Pull Requests #20181205.1 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@maxbrunsfeld

This comment has been minimized.

Copy link
Contributor

maxbrunsfeld commented Dec 6, 2018

Thanks for the fix, @captbaritone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment