From 4a4845d63824d3d909e5e859813c53c032794c0e Mon Sep 17 00:00:00 2001 From: Nick Belzer Date: Sun, 20 Sep 2020 17:19:23 +0200 Subject: [PATCH 1/2] Change settings to override defaults 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. --- Vimari Extension/ConfigurationModel.swift | 11 ++++++-- Vimari Extension/SafariExtensionHandler.swift | 11 +------- Vimari Extension/SettingsMerger.swift | 25 +++++++++++++++++++ Vimari.xcodeproj/project.pbxproj | 6 +++++ 4 files changed, 41 insertions(+), 12 deletions(-) create mode 100644 Vimari Extension/SettingsMerger.swift diff --git a/Vimari Extension/ConfigurationModel.swift b/Vimari Extension/ConfigurationModel.swift index 879b677..926a789 100644 --- a/Vimari Extension/ConfigurationModel.swift +++ b/Vimari Extension/ConfigurationModel.swift @@ -10,7 +10,8 @@ protocol ConfigurationModelProtocol { func editConfigFile() throws func resetConfigFile() throws func getDefaultSettings() throws -> [String: Any] - func getUserSettings() throws -> [String : Any] + func getUserSettings() throws -> [String: Any] + func getMergedSettings() throws -> [String: Any] } import Foundation @@ -54,7 +55,13 @@ class ConfigurationModel: ConfigurationModelProtocol { let settingsData = try Data(contentsOf: urlSettingsFile) return try settingsData.toJSONObject() } - + + func getMergedSettings() throws -> [String: Any] { + let userSettings = try getUserSettings() + let defaults = try getDefaultSettings() + return SettingsMerger.mergeWithDefaults(userSettings, defaults: defaults) + } + private func loadSettings(fromFile file: String) throws -> [String : Any] { let settingsData = try Bundle.main.getJSONData(from: file) return try settingsData.toJSONObject() diff --git a/Vimari Extension/SafariExtensionHandler.swift b/Vimari Extension/SafariExtensionHandler.swift index 3b54422..f1812c6 100644 --- a/Vimari Extension/SafariExtensionHandler.swift +++ b/Vimari Extension/SafariExtensionHandler.swift @@ -150,7 +150,7 @@ class SafariExtensionHandler: SFSafariExtensionHandler { private func updateSettings(page: SFSafariPage) { do { let settings: [String: Any] - if let userSettings = try? configuration.getUserSettings() { + if let userSettings = try? configuration.getMergedSettings() { settings = userSettings } else { settings = try configuration.getDefaultSettings() @@ -160,15 +160,6 @@ class SafariExtensionHandler: SFSafariExtensionHandler { NSLog(error.localizedDescription) } } - - private func fallbackSettings(page: SFSafariPage) { - do { - let settings = try configuration.getUserSettings() - page.dispatch(settings: settings) - } catch { - NSLog(error.localizedDescription) - } - } } // MARK: Helpers diff --git a/Vimari Extension/SettingsMerger.swift b/Vimari Extension/SettingsMerger.swift new file mode 100644 index 0000000..8729822 --- /dev/null +++ b/Vimari Extension/SettingsMerger.swift @@ -0,0 +1,25 @@ +// +// SettingsMerger.swift +// Vimari Extension +// +// Created by Nick Belzer on 20/09/2020. +// Copyright © 2020 net.televator. All rights reserved. +// + +import Foundation + +class SettingsMerger { + /** This function merges the user settings with the default values when they are not provided by the user settings. */ + static func mergeWithDefaults(_ settings: [String: Any], defaults: [String: Any]) -> [String: Any] { + return settings.merging(defaults, uniquingKeysWith: solveSettingsMergeConflict) + } + + /** Helper method for conflicts during settings merging. Recursively merges dictionaries with the preference for the first value.*/ + static private func solveSettingsMergeConflict(val: Any, defaultVal: Any) -> Any { + if let subDictUser = val as? [String: Any], let subDictDefault = defaultVal as? [String: Any] { + // Allow for recursive merging of nested dictionaries. + return subDictUser.merging(subDictDefault, uniquingKeysWith: self.solveSettingsMergeConflict) + } + return val; + } +} diff --git a/Vimari.xcodeproj/project.pbxproj b/Vimari.xcodeproj/project.pbxproj index 0abd51e..c7b18e4 100644 --- a/Vimari.xcodeproj/project.pbxproj +++ b/Vimari.xcodeproj/project.pbxproj @@ -7,6 +7,8 @@ objects = { /* Begin PBXBuildFile section */ + 654AAF3E2517A37D00AB44D9 /* SettingsMerger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 654AAF3D2517A37D00AB44D9 /* SettingsMerger.swift */; }; + 654AAF4B2517A81200AB44D9 /* SettingsMerger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 654AAF3D2517A37D00AB44D9 /* SettingsMerger.swift */; }; 659033F124E2B77400432D0E /* svim-scripts.js in Resources */ = {isa = PBXBuildFile; fileRef = 659033F024E2B77400432D0E /* svim-scripts.js */; }; 65E444F324CC3A1B008EA1DC /* SafariExtensionCommunicator.js in Resources */ = {isa = PBXBuildFile; fileRef = 65E444F224CC3A1B008EA1DC /* SafariExtensionCommunicator.js */; }; B1E3C17023A65ED400A56807 /* ConfigurationModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = B1E3C16F23A65ED400A56807 /* ConfigurationModel.swift */; }; @@ -58,6 +60,7 @@ /* End PBXCopyFilesBuildPhase section */ /* Begin PBXFileReference section */ + 654AAF3D2517A37D00AB44D9 /* SettingsMerger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsMerger.swift; sourceTree = ""; }; 659033F024E2B77400432D0E /* svim-scripts.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = "svim-scripts.js"; sourceTree = ""; }; 65E444F224CC3A1B008EA1DC /* SafariExtensionCommunicator.js */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.javascript; path = SafariExtensionCommunicator.js; sourceTree = ""; }; B1E3C16F23A65ED400A56807 /* ConfigurationModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConfigurationModel.swift; sourceTree = ""; }; @@ -163,6 +166,7 @@ E380F2612331806500640547 /* SafariExtensionHandler.swift */, B1E3C16F23A65ED400A56807 /* ConfigurationModel.swift */, E380F2632331806500640547 /* SafariExtensionViewController.swift */, + 654AAF3D2517A37D00AB44D9 /* SettingsMerger.swift */, E380F2652331806500640547 /* SafariExtensionViewController.xib */, B1FD3B9723A588DE00677A52 /* json */, E380F281233183EE00640547 /* css */, @@ -322,6 +326,7 @@ files = ( E380F2512331806400640547 /* ViewController.swift in Sources */, E380F24C2331806400640547 /* AppDelegate.swift in Sources */, + 654AAF4B2517A81200AB44D9 /* SettingsMerger.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -331,6 +336,7 @@ files = ( E380F2642331806500640547 /* SafariExtensionViewController.swift in Sources */, E380F2622331806500640547 /* SafariExtensionHandler.swift in Sources */, + 654AAF3E2517A37D00AB44D9 /* SettingsMerger.swift in Sources */, B1E3C17023A65ED400A56807 /* ConfigurationModel.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; From db9e3fb4b4fe26889cfcbe49dd936641d784eec4 Mon Sep 17 00:00:00 2001 From: Nick Belzer Date: Sun, 20 Sep 2020 18:15:52 +0200 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b24cfa9..fe2d6c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ Changelog ------------- ### Unreleased +* Fill missing configuration with default values [#204](https://github.com/televator-apps/vimari/pull/204). This allows users to reduce the size of their configuration when using the default values. It allows us to introduce new settings without breaking user configurations because they miss this setting in their configuration. + +### 2.1.0 (2020-09-10) * Add `transparentBindings` setting that allows the use of non-bound keys in normal mode ([#188](https://github.com/televator-apps/vimari/issues/188)). * Remove eager link hint triggering [#190](https://github.com/televator-apps/vimari/issues/190) * Use `window.open` for `openNewTab` action [#189](https://github.com/televator-apps/vimari/issues/189)