Skip to content
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

Fix configmanager #787

Merged
merged 3 commits into from
Aug 10, 2017
Merged

Fix configmanager #787

merged 3 commits into from
Aug 10, 2017

Conversation

asmsuechan
Copy link
Contributor

context

It's a bug happens on v0.8.13.

before

The settings of Boostnote was reset and used DEFAULT_CONFIG unless ~/.boostnoterc was not created and set settings.

after

Boostnote reads settings from localStorage and overwrites the settings when ~/.boostnoterc exists.


config = Object.assign({}, DEFAULT_CONFIG, JSON.parse(config))

config = Object.assign({}, DEFAULT_CONFIG, boostnotercConfig)
Copy link
Contributor Author

@asmsuechan asmsuechan Aug 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is the very reason why the bug happened.

@asmsuechan asmsuechan merged commit f14ce0d into BoostIO:master Aug 10, 2017
@asmsuechan asmsuechan deleted the fix-configmanager branch August 10, 2017 15:41
@vvs
Copy link

vvs commented Aug 10, 2017

Is this really desired behavior when the .boostnoterc settings always override the user settings set in the UI?

So, a user modifies something in the UI, everything works as he/she likes, then he/she restarts the app, and the settings is gone, replaced by the one from the .boostnote file.

Or is this .boostnoterc file not really designed to be used by users, and only be used to fix/reset some of the user settings (for example, to be able to reset the settings if they gone bad for some reason)?

@asmsuechan
Copy link
Contributor Author

Is this really desired behavior when the .boostnoterc settings always override the user settings set in the UI?

Yes, it is. But I'll reconsider it.

@kazup01 kazup01 mentioned this pull request Aug 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants