Change settings to override defaults #204
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit introduces a method `getMergedSettings` to the ConfigurationModelProtocol which merges the users settings with the default settings, prioritising the settings set by the user. This allows us to fill up missing settings (say a newly introduced binding) with a default value. This prevents the extension from crashing when a new setting is missing from the user-defined settings. Therefore allowing us to gradually introduce new settings without breaking previous configurations. This does not touch the users configuration file so there is a potential that this could introduce confusion to the user when a certain binding is introduced but not displayed in their configuration. This should be addressed in #177. I was not able to introduce tests for the written merging code, this because Xcode does not allow us to test Extension targets. If anyone knows how this works please let me know.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #178
Before continuing work on some of the issues that have been created since the release of
v2.1we need this mechanism to introduce new settings without breaking current configurations by users.From commit 4a4845d:
This commit introduces a method
getMergedSettingsto theConfigurationModelProtocolwhich merges the users settings with the default settings, prioritising the settings set by the user. This allows us to fill up missing settings (say a newly introduced binding) with a default value. This prevents the extension from crashing when a new setting is missing from the user-defined settings. Therefore allowing us to gradually introduce new settings without breaking previous configurations.This does not touch the users configuration file so there is a potential that this could introduce confusion to the user when a certain binding is introduced but not displayed in their configuration. This should be addressed in #177.
I was not able to introduce tests for the written merging code, this because Xcode does not allow us to test Extension targets. If anyone knows how this works please let me know.
TLDR: This PR introduces a
SettingsMergerthat recursively merges a dictionary of settings with the default settings, choosing the user defined settings over the default ones in case of a conflict. The extension will now retrieve the configuration by utilising this new merger. This allows us to introduce new settings without breaking current configurations.