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

Serialize settings as JSON when sending them to the main process #16809

Merged
merged 1 commit into from Feb 21, 2018

Conversation

Projects
None yet
2 participants
@maxbrunsfeld
Contributor

maxbrunsfeld commented Feb 21, 2018

Fixes #16789

Config settings of type Color should be written to the config file as a string like #af01cd. To achieve this, the class overrides toJSON .

When I refactored the config system to IPC settings to the main process for writing, this behavior was inadvertently broken because sending a value over electron's IPC does not serialize it as JSON; it uses its own serialization mechanism that does not preserve objects' toJSON methods.

@Arcanemagus

This comment has been minimized.

Contributor

Arcanemagus commented Feb 21, 2018

Thanks! I dug far enough to figure out why it was breaking, but not where 😆.

@maxbrunsfeld

This comment has been minimized.

Contributor

maxbrunsfeld commented Feb 21, 2018

@Arcanemagus Yeah, thanks so much for the info in your report. That made it very easy to fix!

@maxbrunsfeld maxbrunsfeld merged commit a0e231d into master Feb 21, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@maxbrunsfeld maxbrunsfeld deleted the mb-fix-color-config branch Feb 21, 2018

maxbrunsfeld added a commit that referenced this pull request Feb 21, 2018

Merge pull request #16809 from atom/mb-fix-color-config
Serialize settings as JSON when sending them to the main process
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment