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

@@ -59,6 +59,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

@@ -1,22 +1,27 @@
 use serde::Deserialize;
 
+use helix_view::editor::ok_or_default;
+
 use crate::keymap::Keymaps;
 
+// 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, Default, Clone, PartialEq, Deserialize)]
-#[serde(deny_unknown_fields)]
 pub struct Config {
+    #[serde(default, deserialize_with = "ok_or_default")]
     pub theme: Option<String>,
-    #[serde(default)]
+    #[serde(default, deserialize_with = "ok_or_default")]
     pub lsp: LspConfig,
-    #[serde(default)]
+    #[serde(default, deserialize_with = "ok_or_default")]
     pub keys: Keymaps,
-    #[serde(default)]
+    #[serde(default, deserialize_with = "ok_or_default")]
     pub editor: helix_view::editor::Config,
 }
 
 #[derive(Debug, Default, Clone, PartialEq, 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,
 }
 
@@ -56,4 +61,51 @@ mod tests {
             }
         );
     }
+
+    #[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: Keymaps(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

@@ -146,6 +146,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

@@ -3,6 +3,8 @@ use helix_term::application::Application;
 use helix_term::args::Args;
 use helix_term::config::Config;
 use helix_term::keymap::merge_keys;
+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<()> {
@@ -118,21 +120,38 @@ FLAGS:
         std::fs::create_dir_all(&conf_dir).ok();
     }
 
+    let mut ignored_keys = BTreeSet::new();
+
     let config = match std::fs::read_to_string(helix_loader::config_file()) {
-        Ok(config) => toml::from_str(&config)
-            .map(merge_keys)
-            .unwrap_or_else(|err| {
-                eprintln!("Bad config: {}", err);
-                eprintln!("Press <ENTER> to continue with default config");
-                use std::io::Read;
-                // This waits for an enter press.
-                let _ = std::io::stdin().read(&mut []);
-                Config::default()
-            }),
+        Ok(config) => serde_ignored::deserialize(&mut toml::Deserializer::new(&config), |path| {
+            ignored_keys.insert(path.to_string());
+        })
+        .map(merge_keys)
+        .unwrap_or_else(|err| {
+            eprintln!("Bad config: {}", err);
+            eprintln!("Press <ENTER> to continue with default config");
+            use std::io::Read;
+            // This waits for an enter press.
+            let _ = std::io::stdin().read(&mut []);
+            Config::default()
+        }),
         Err(err) if err.kind() == std::io::ErrorKind::NotFound => Config::default(),
         Err(err) => return Err(Error::new(err)),
     };
 
+    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,
@@ -40,6 +40,20 @@ use helix_dap as dap;
 
 use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize};
 
+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>,
@@ -49,7 +63,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.
@@ -88,45 +102,61 @@ 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.
     #[serde(skip_serializing, 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(deserialize_with = "ok_or_default")]
     pub search: SearchConfig,
 }
 
 #[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,
@@ -199,6 +229,12 @@ pub enum LineNumber {
     Relative,
 }
 
+impl Default for LineNumber {
+    fn default() -> Self {
+        Self::Absolute
+    }
+}
+
 impl std::str::FromStr for LineNumber {
     type Err = anyhow::Error;
 

helix-view/src/editor.rs

@@ -1,11 +1,26 @@
 //! Input event handling, currently backed by crossterm.
 use anyhow::{anyhow, Error};
 use helix_core::unicode::width::UnicodeWidthStr;
-use serde::de::{self, Deserialize, Deserializer};
+use serde::de::{Deserialize, Deserializer};
 use std::fmt;
+use std::sync::atomic::{AtomicBool, Ordering};
 
 use crate::keyboard::{KeyCode, KeyModifiers};
 
+static HAD_CONFIG_ERROR: AtomicBool = AtomicBool::new(false);
+
+/// To be called by the Config deserializer when there's an error so that the editor knows it
+/// should wait the user to press Enter in order for the user to have the time to see the error
+/// before the editor shows up.
+pub fn set_config_error() {
+    HAD_CONFIG_ERROR.store(true, Ordering::SeqCst);
+}
+
+/// Return true if there was an error during the Config deserialization.
+pub fn get_config_error() -> bool {
+    HAD_CONFIG_ERROR.load(Ordering::SeqCst)
+}
+
 /// Represents a key event.
 // We use a newtype here because we want to customize Deserialize and Display.
 #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Hash)]
@@ -22,6 +37,14 @@ impl KeyEvent {
             _ => None,
         }
     }
+
+    /// Return an invalid KeyEvent to use for cases where an event key cannot be parsed.
+    pub fn invalid() -> Self {
+        Self {
+            code: KeyCode::Null,
+            modifiers: KeyModifiers::NONE,
+        }
+    }
 }
 
 pub(crate) mod keys {
@@ -210,7 +233,13 @@ impl<'de> Deserialize<'de> for KeyEvent {
         D: Deserializer<'de>,
     {
         let s = String::deserialize(deserializer)?;
-        s.parse().map_err(de::Error::custom)
+        let key_event = s.parse();
+        if let Err(ref error) = key_event {
+            // TODO: show error position.
+            eprintln!("Bad config for key: {}", error);
+            set_config_error();
+        }
+        Ok(key_event.unwrap_or_else(|_| KeyEvent::invalid()))
     }
 }