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.

Sign up for GitHub

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

Jump to bottom

Valid JSON configuration #177

Open
nbelzer opened this issue Jul 31, 2020 · 0 comments
Open

Valid JSON configuration #177

nbelzer opened this issue Jul 31, 2020 · 0 comments
Labels
feature request

Comments

@nbelzer
Copy link
Collaborator

nbelzer commented Jul 31, 2020

Given the merge of #175 we find user preferences back in to Vimari. The implementation however does not warn the user about an invalid configuration (say you missed some json syntax). The default behaviour when the configuration is invalid is to load the default configuration.

It would be great to improve transparency about this to the user. Most simple would be to show some indicator in the Vimari.app or some sort of alert in the browser itself. Another option would be to try and prevent the user from ever saving an invalid configuration file to begin with, however this would require more work as we'd have to take control over the editing process (which is currently left to TextEdit).
nbelzer added a commit that referenced this issue Sep 20, 2020
@nbelzer
Change settings to override defaults
4a4845d 
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.
@nbelzer nbelzer mentioned this issue Sep 20, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@nbelzer