Continue loading the config on error

Author: antoyo

Cargo.lock

@@ -179,7 +179,7 @@ pub struct IndentationConfiguration {
 
 /// Configuration for auto pairs
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case", deny_unknown_fields, untagged)]
+#[serde(rename_all = "kebab-case", untagged)]
 pub enum AutoPairConfig {
     /// Enables or disables auto pairing. False means disabled. True means to use the default pairs.
     Enable(bool),

helix-core/src/syntax.rs

@@ -61,6 +61,7 @@ toml = "0.5"
 
 serde_json = "1.0"
 serde = { version = "1.0", features = ["derive"] }
+serde_ignored = "0.1.2"
 
 # ripgrep for global search
 grep-regex = "0.1.9"

helix-term/Cargo.toml

@@ -19,6 +19,7 @@ use crate::{
 
 use log::{error, warn};
 use std::{
+    collections::BTreeSet,
     io::{stdin, stdout, Write},
     sync::Arc,
     time::{Duration, Instant},
@@ -271,10 +272,11 @@ impl Application {
     }
 
     fn refresh_config(&mut self) {
-        let config = Config::load(helix_loader::config_file()).unwrap_or_else(|err| {
-            self.editor.set_error(err.to_string());
-            Config::default()
-        });
+        let config = Config::load(helix_loader::config_file(), &mut BTreeSet::new())
+            .unwrap_or_else(|err| {
+                self.editor.set_error(err.to_string());
+                Config::default()
+            });
 
         // Refresh theme
         if let Some(theme) = config.theme.clone() {

helix-term/src/application.rs

@@ -1,19 +1,23 @@
 use crate::keymap::{default::default, merge_keys, Keymap};
 use helix_view::document::Mode;
 use serde::Deserialize;
-use std::collections::HashMap;
+use std::collections::{BTreeSet, HashMap};
 use std::fmt::Display;
 use std::io::Error as IOError;
 use std::path::PathBuf;
 use toml::de::Error as TomlError;
 
+use helix_view::editor::ok_or_default;
+
+// NOTE: The fields in this struct use the deserializer ok_or_default to continue parsing when
+// there is an error. In that case, it will use the default value.
 #[derive(Debug, Clone, PartialEq, Deserialize)]
-#[serde(deny_unknown_fields)]
 pub struct Config {
+    #[serde(default, deserialize_with = "ok_or_default")]
     pub theme: Option<String>,
-    #[serde(default = "default")]
+    #[serde(default = "default", deserialize_with = "ok_or_default")]
     pub keys: HashMap<Mode, Keymap>,
-    #[serde(default)]
+    #[serde(default, deserialize_with = "ok_or_default")]
     pub editor: helix_view::editor::Config,
 }
 
@@ -43,17 +47,24 @@ impl Display for ConfigLoadError {
 }
 
 impl Config {
-    pub fn load(config_path: PathBuf) -> Result<Config, ConfigLoadError> {
+    pub fn load(
+        config_path: PathBuf,
+        ignored_keys: &mut BTreeSet<String>,
+    ) -> Result<Config, ConfigLoadError> {
         match std::fs::read_to_string(config_path) {
-            Ok(config) => toml::from_str(&config)
+            Ok(config) => {
+                serde_ignored::deserialize(&mut toml::Deserializer::new(&config), |path| {
+                    ignored_keys.insert(path.to_string());
+                })
                 .map(merge_keys)
-                .map_err(ConfigLoadError::BadConfig),
+                .map_err(ConfigLoadError::BadConfig)
+            }
             Err(err) => Err(ConfigLoadError::Error(err)),
         }
     }
 
-    pub fn load_default() -> Result<Config, ConfigLoadError> {
-        Config::load(helix_loader::config_file())
+    pub fn load_default(ignored_keys: &mut BTreeSet<String>) -> Result<Config, ConfigLoadError> {
+        Config::load(helix_loader::config_file(), ignored_keys)
     }
 }
 
@@ -104,4 +115,51 @@ mod tests {
         let default_keys = Config::default().keys;
         assert_eq!(default_keys, default());
     }
+
+    #[test]
+    fn partial_config_parsing() {
+        use crate::keymap;
+        use crate::keymap::Keymap;
+        use helix_core::hashmap;
+        use helix_view::document::Mode;
+
+        let sample_keymaps = r#"
+            theme = false
+
+            [editor]
+            line-number = false
+            mous = "false"
+            scrolloff = 7
+
+            [editor.search]
+            smart-case = false
+
+            [keys.insert]
+            y = "move_line_down"
+            SC-a = "delete_selection"
+
+            [keys.normal]
+            A-F12 = "move_next_word_end"
+        "#;
+
+        let mut editor = helix_view::editor::Config::default();
+        editor.search.smart_case = false;
+        editor.scrolloff = 7;
+
+        assert_eq!(
+            toml::from_str::<Config>(sample_keymaps).unwrap(),
+            Config {
+                keys: hashmap! {
+                    Mode::Insert => Keymap::new(keymap!({ "Insert mode"
+                        "y" => move_line_down,
+                    })),
+                    Mode::Normal => Keymap::new(keymap!({ "Normal mode"
+                        "A-F12" => move_next_word_end,
+                    })),
+                },
+                editor,
+                ..Default::default()
+            }
+        );
+    }
 }

helix-term/src/config.rs

@@ -34,6 +34,18 @@ impl<'de> Deserialize<'de> for KeyTrieNode {
         D: serde::Deserializer<'de>,
     {
         let map = HashMap::<KeyEvent, KeyTrie>::deserialize(deserializer)?;
+        let map: HashMap<_, _> = map
+            .into_iter()
+            .filter_map(|(key, value)| {
+                // Filter the KeyEvents that has an invalid value because those come from a
+                // parsing error and we should just ignore them.
+                if key == KeyEvent::invalid() {
+                    None
+                } else {
+                    Some((key, value))
+                }
+            })
+            .collect();
         let order = map.keys().copied().collect::<Vec<_>>(); // NOTE: map.keys() has arbitrary order
         Ok(Self {
             map,

helix-term/src/keymap.rs

@@ -2,6 +2,8 @@ use anyhow::{Context, Error, Result};
 use helix_term::application::Application;
 use helix_term::args::Args;
 use helix_term::config::{Config, ConfigLoadError};
+use helix_view::input::{get_config_error, set_config_error};
+use std::collections::BTreeSet;
 use std::path::PathBuf;
 
 fn setup_logging(logpath: PathBuf, verbosity: u64) -> Result<()> {
@@ -114,7 +116,8 @@ FLAGS:
         std::fs::create_dir_all(&conf_dir).ok();
     }
 
-    let config = match Config::load_default() {
+    let mut ignored_keys = BTreeSet::new();
+    let config = match Config::load_default(&mut ignored_keys) {
         Ok(config) => config,
         Err(err) => {
             match err {
@@ -134,6 +137,19 @@ FLAGS:
         }
     };
 
+    if !ignored_keys.is_empty() {
+        let keys = ignored_keys.into_iter().collect::<Vec<_>>().join(", ");
+        eprintln!("Ignored keys in config: {}", keys);
+        set_config_error();
+    }
+
+    if get_config_error() {
+        eprintln!("Press <ENTER> to continue");
+        use std::io::Read;
+        // This waits for an enter press.
+        let _ = std::io::stdin().read(&mut []);
+    }
+
     setup_logging(logpath, args.verbosity).context("failed to initialize logging")?;
 
     // TODO: use the thread local executor to spawn the application task separately from the work pool

helix-term/src/main.rs

@@ -3,7 +3,7 @@ use crate::{
     document::{Mode, SCRATCH_BUFFER_NAME},
     graphics::{CursorKind, Rect},
     info::Info,
-    input::KeyEvent,
+    input::{set_config_error, KeyEvent},
     theme::{self, Theme},
     tree::{self, Tree},
     Document, DocumentId, View, ViewId,
@@ -44,6 +44,20 @@ use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer}
 
 use arc_swap::access::{DynAccess, DynGuard};
 
+pub fn ok_or_default<'a, T, D>(deserializer: D) -> Result<T, D::Error>
+where
+    T: Deserialize<'a> + Default,
+    D: Deserializer<'a>,
+{
+    let result = T::deserialize(deserializer);
+    if let Err(ref error) = result {
+        // FIXME: the error message does not contain the key or the position.
+        eprintln!("Bad config for value: {}", error);
+        set_config_error();
+    }
+    Ok(result.unwrap_or_default())
+}
+
 fn deserialize_duration_millis<'de, D>(deserializer: D) -> Result<Duration, D::Error>
 where
     D: serde::Deserializer<'de>,
@@ -65,7 +79,7 @@ where
 }
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case", default, deny_unknown_fields)]
+#[serde(rename_all = "kebab-case", default)]
 pub struct FilePickerConfig {
     /// IgnoreOptions
     /// Enables ignoring hidden files.
@@ -104,26 +118,36 @@ impl Default for FilePickerConfig {
     }
 }
 
+// NOTE: The fields in this struct use the deserializer ok_or_default to continue parsing when
+// there is an error. In that case, it will use the default value.
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case", default, deny_unknown_fields)]
+#[serde(rename_all = "kebab-case", default)]
 pub struct Config {
     /// Padding to keep between the edge of the screen and the cursor when scrolling. Defaults to 5.
+    #[serde(deserialize_with = "ok_or_default")]
     pub scrolloff: usize,
     /// Number of lines to scroll at once. Defaults to 3
+    #[serde(deserialize_with = "ok_or_default")]
     pub scroll_lines: isize,
     /// Mouse support. Defaults to true.
+    #[serde(deserialize_with = "ok_or_default")]
     pub mouse: bool,
     /// Shell to use for shell commands. Defaults to ["cmd", "/C"] on Windows and ["sh", "-c"] otherwise.
+    #[serde(deserialize_with = "ok_or_default")]
     pub shell: Vec<String>,
     /// Line number mode.
+    #[serde(deserialize_with = "ok_or_default")]
     pub line_number: LineNumber,
     /// Middle click paste support. Defaults to true.
+    #[serde(deserialize_with = "ok_or_default")]
     pub middle_click_paste: bool,
     /// Automatic insertion of pairs to parentheses, brackets,
     /// etc. Optionally, this can be a list of 2-tuples to specify a
     /// global list of characters to pair. Defaults to true.
+    #[serde(deserialize_with = "ok_or_default")]
     pub auto_pairs: AutoPairConfig,
     /// Automatic auto-completion, automatically pop up without user trigger. Defaults to true.
+    #[serde(deserialize_with = "ok_or_default")]
     pub auto_completion: bool,
     /// Time in milliseconds since last keypress before idle timers trigger.
     /// Used for autocompletion, set to 0 for instant. Defaults to 400ms.
@@ -132,28 +156,35 @@ pub struct Config {
         deserialize_with = "deserialize_duration_millis"
     )]
     pub idle_timeout: Duration,
+    #[serde(deserialize_with = "ok_or_default")]
     pub completion_trigger_len: u8,
     /// Whether to display infoboxes. Defaults to true.
+    #[serde(deserialize_with = "ok_or_default")]
     pub auto_info: bool,
+    #[serde(deserialize_with = "ok_or_default")]
     pub file_picker: FilePickerConfig,
     /// Shape for cursor in each mode
+    #[serde(deserialize_with = "ok_or_default")]
     pub cursor_shape: CursorShapeConfig,
     /// Set to `true` to override automatic detection of terminal truecolor support in the event of a false negative. Defaults to `false`.
+    #[serde(deserialize_with = "ok_or_default")]
     pub true_color: bool,
     /// Search configuration.
-    #[serde(default)]
+    #[serde(default, deserialize_with = "ok_or_default")]
     pub search: SearchConfig,
+    #[serde(default, deserialize_with = "ok_or_default")]
     pub lsp: LspConfig,
 }
 
 #[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case", deny_unknown_fields)]
+#[serde(rename_all = "kebab-case")]
 pub struct LspConfig {
+    #[serde(deserialize_with = "ok_or_default")]
     pub display_messages: bool,
 }
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case", default, deny_unknown_fields)]
+#[serde(rename_all = "kebab-case", default)]
 pub struct SearchConfig {
     /// Smart case: Case insensitive searching unless pattern contains upper case characters. Defaults to true.
     pub smart_case: bool,
@@ -226,6 +257,12 @@ pub enum LineNumber {
     Relative,
 }
 
+impl Default for LineNumber {
+    fn default() -> Self {
+        Self::Absolute
+    }
+}
+
 impl std::str::FromStr for LineNumber {
     type Err = anyhow::Error;