From 48fca507f5d94ad9527333f5a5652662835e94c4 Mon Sep 17 00:00:00 2001 From: David Knaack Date: Sat, 23 Oct 2021 10:15:46 +0200 Subject: [PATCH] fix(configure): preserve formatting and comments (#3152) * fix(configure): preserve formatting and comments * preserve formatting in changed line * add tests --- Cargo.lock | 32 +++++ Cargo.toml | 1 + src/configure.rs | 312 +++++++++++++++++++++++++++++++++++++---------- 3 files changed, 279 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7a8d6354..892ab08d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -257,6 +257,16 @@ dependencies = [ "vec_map", ] +[[package]] +name = "combine" +version = "4.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a909e4d93292cd8e9c42e189f61681eff9d67b6541f96b8a1a737f23737bd001" +dependencies = [ + "bytes", + "memchr", +] + [[package]] name = "concurrent-queue" version = "1.2.2" @@ -805,6 +815,15 @@ dependencies = [ "libc", ] +[[package]] +name = "kstring" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e8d7e992938cc9078c8db5fd5bdc400e7f9da6efa384c280902a8922b676221" +dependencies = [ + "serde", +] + [[package]] name = "lazy_static" version = "1.4.0" @@ -1761,6 +1780,7 @@ dependencies = [ "tempfile", "terminal_size", "toml", + "toml_edit", "unicode-segmentation", "unicode-width", "urlencoding", @@ -1928,6 +1948,18 @@ dependencies = [ "serde", ] +[[package]] +name = "toml_edit" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b46c9238346fec3169f99251eda26e918ab71cdf382a660e46076ff1b1b16729" +dependencies = [ + "combine", + "indexmap", + "itertools", + "kstring", +] + [[package]] name = "treeline" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 1f51f003..92de92d9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -70,6 +70,7 @@ shadow-rs = "0.7.1" versions = "3.0.3" strsim = "0.10.0" sha-1 = "0.9.8" +toml_edit = "0.6.0" process_control = { version = "3.1.0", features = ["crossbeam-channel"] } diff --git a/src/configure.rs b/src/configure.rs index 33b7c86c..dca7565f 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -3,6 +3,7 @@ use std::ffi::OsString; use std::io::ErrorKind; use std::process; use std::process::Stdio; +use std::str::FromStr; use crate::config::RootModuleConfig; use crate::config::StarshipConfig; @@ -10,9 +11,8 @@ use crate::configs::PROMPT_ORDER; use crate::utils; use std::fs::File; use std::io::Write; -use toml::map::Map; -use toml::value::Table; use toml::Value; +use toml_edit::Document; #[cfg(not(windows))] const STD_EDITOR: &str = "vi"; @@ -20,39 +20,49 @@ const STD_EDITOR: &str = "vi"; const STD_EDITOR: &str = "notepad.exe"; pub fn update_configuration(name: &str, value: &str) { - let keys: Vec<&str> = name.split('.').collect(); - if keys.len() != 2 { - log::error!("Please pass in a config key with a '.'"); - process::exit(1); - } + let mut doc = get_configuration_edit(); - if let Some(table) = get_configuration().as_table_mut() { - if !table.contains_key(keys[0]) { - table.insert(keys[0].to_string(), Value::Table(Map::new())); + match handle_update_configuration(&mut doc, name, value) { + Err(e) => { + eprintln!("{}", e); + process::exit(1); + } + _ => write_configuration(&doc), + } +} + +fn handle_update_configuration(doc: &mut Document, name: &str, value: &str) -> Result<(), String> { + let mut current_item = &mut doc.root; + + for key in name.split('.') { + if !current_item.is_table_like() { + return Err("This command can only index into TOML tables".to_owned()); } - if let Some(values) = table.get(keys[0]).unwrap().as_table() { - let mut updated_values = values.clone(); - - if value.parse::().is_ok() { - updated_values.insert( - keys[1].to_string(), - Value::Boolean(value.parse::().unwrap()), - ); - } else if value.parse::().is_ok() { - updated_values.insert( - keys[1].to_string(), - Value::Integer(value.parse::().unwrap()), - ); - } else { - updated_values.insert(keys[1].to_string(), Value::String(value.to_string())); - } - - table.insert(keys[0].to_string(), Value::Table(updated_values)); + if key.is_empty() { + return Err("Empty table keys are not supported".to_owned()); } - write_configuration(table); + let table = current_item.as_table_like_mut().unwrap(); + + if !table.contains_key(key) { + table.insert(key, toml_edit::table()); + } + + current_item = table.get_mut(key).unwrap(); } + + let mut new_value = toml_edit::Value::from_str(value) + .map(toml_edit::Item::Value) + .unwrap_or_else(|_| toml_edit::value(value)); + + if let Some(value) = current_item.as_value() { + *new_value.as_value_mut().unwrap().decor_mut() = value.decor().clone(); + } + + *current_item = new_value; + + Ok(()) } pub fn print_configuration(use_default: bool) { @@ -96,44 +106,46 @@ pub fn print_configuration(use_default: bool) { } pub fn toggle_configuration(name: &str, key: &str) { - if let Some(table) = get_configuration().as_table_mut() { - match table.get(name) { - Some(v) => { - if let Some(values) = v.as_table() { - let mut updated_values = values.clone(); + let mut doc = get_configuration_edit(); - let current: bool = match updated_values.get(key) { - Some(v) => match v.as_bool() { - Some(b) => b, - _ => { - log::error!( - "Given config key '{}' must be in 'boolean' format", - key - ); - process::exit(1); - } - }, - _ => { - log::error!("Given config key '{}' must be exist in config file", key); - process::exit(1); - } - }; - - updated_values.insert(key.to_string(), Value::Boolean(!current)); - - table.insert(name.to_string(), Value::Table(updated_values)); - - write_configuration(table); - } - } - _ => { - log::error!("Given module '{}' not found in config file", name); - process::exit(1); - } - }; + match handle_toggle_configuration(&mut doc, name, key) { + Err(e) => { + eprintln!("{}", e); + process::exit(1); + } + _ => write_configuration(&doc), } } +fn handle_toggle_configuration(doc: &mut Document, name: &str, key: &str) -> Result<(), String> { + if name.is_empty() || key.is_empty() { + return Err("Empty table keys are not supported".to_owned()); + } + + let table = doc.as_table_mut(); + + let values = table + .get_mut(name) + .ok_or_else(|| format!("Given module '{}' not found in config file", name))? + .as_table_like_mut() + .ok_or_else(|| format!("Given config entry '{}' is not a module", key))?; + + let old_value = values + .get(key) + .ok_or_else(|| format!("Given config key '{}' must exist in config file", key))?; + + let old = old_value + .as_bool() + .ok_or_else(|| format!("Given config key '{}' must be in 'boolean' format", key))?; + + let mut new_value = toml_edit::value(!old); + // Above code already checks if it is a value (bool) + *new_value.as_value_mut().unwrap().decor_mut() = old_value.as_value().unwrap().decor().clone(); + + values.insert(key, new_value); + Ok(()) +} + pub fn get_configuration() -> Value { let starship_config = StarshipConfig::initialize(); @@ -142,11 +154,35 @@ pub fn get_configuration() -> Value { .expect("Failed to load starship config") } -pub fn write_configuration(table: &mut Table) { +pub fn get_configuration_edit() -> Document { + let file_path = get_config_path(); + let toml_content = match utils::read_file(&file_path) { + Ok(content) => { + log::trace!("Config file content: \"\n{}\"", &content); + Some(content) + } + Err(e) => { + let level = if e.kind() == ErrorKind::NotFound { + log::Level::Debug + } else { + log::Level::Error + }; + + log::log!(level, "Unable to read config file content: {}", &e); + None + } + }; + + toml_content + .unwrap_or_default() + .parse::() + .expect("Failed to load starship config") +} + +pub fn write_configuration(doc: &Document) { let config_path = get_config_path(); - let config_str = - toml::to_string_pretty(&table).expect("Failed to serialize the config to string"); + let config_str = doc.to_string(); File::create(&config_path) .and_then(|mut file| file.write_all(config_str.as_ref())) @@ -261,4 +297,148 @@ mod tests { let actual = get_editor_internal(None, None); assert_eq!(STD_EDITOR, actual); } + + fn create_doc() -> Document { + let config = concat!( + " # comment\n", + " [status] # comment\n", + "disabled = false # comment\n", + "# comment\n", + "\n" + ); + + config.parse::().unwrap() + } + + #[test] + fn test_toggle_simple() { + let mut doc = create_doc(); + + assert!(!doc["status"]["disabled"].as_bool().unwrap()); + + handle_toggle_configuration(&mut doc, "status", "disabled").unwrap(); + + assert!(doc["status"]["disabled"].as_bool().unwrap()); + + let new_config = concat!( + " # comment\n", + " [status] # comment\n", + "disabled = true # comment\n", + "# comment\n", + "\n" + ); + + assert_eq!(doc.to_string(), new_config) + } + + #[test] + fn test_toggle_missing_module() { + let mut doc = create_doc(); + assert!(handle_toggle_configuration(&mut doc, "missing_module", "disabled").is_err()); + } + + #[test] + fn test_toggle_missing_key() { + let mut doc = create_doc(); + assert!(handle_toggle_configuration(&mut doc, "status", "missing").is_err()); + } + + #[test] + fn test_toggle_wrong_type() { + let mut doc = create_doc(); + doc["status"]["disabled"] = toml_edit::value("a"); + + assert!(handle_toggle_configuration(&mut doc, "status", "disabled").is_err()); + + doc["format"] = toml_edit::value("$all"); + + assert!(handle_toggle_configuration(&mut doc, "format", "disabled").is_err()); + } + + #[test] + fn test_toggle_empty() { + let mut doc = create_doc(); + + doc["status"][""] = toml_edit::value(true); + doc[""]["disabled"] = toml_edit::value(true); + + assert!(handle_toggle_configuration(&mut doc, "status", "").is_err()); + assert!(handle_toggle_configuration(&mut doc, "", "disabled").is_err()); + } + + #[test] + fn test_update_config_wrong_type() { + let mut doc = create_doc(); + + assert!( + handle_update_configuration(&mut doc, "status.disabled.not_a_table", "true").is_err() + ); + } + + #[test] + fn test_update_config_simple() { + let mut doc = create_doc(); + + assert!(!doc["status"]["disabled"].as_bool().unwrap()); + + handle_update_configuration(&mut doc, "status.disabled", "true").unwrap(); + + assert!(doc["status"]["disabled"].as_bool().unwrap()); + + let new_config = concat!( + " # comment\n", + " [status] # comment\n", + "disabled = true # comment\n", + "# comment\n", + "\n" + ); + + assert_eq!(doc.to_string(), new_config) + } + + #[test] + fn test_update_config_parse() { + let mut doc = create_doc(); + + handle_update_configuration(&mut doc, "test", "true").unwrap(); + + assert!(doc["test"].as_bool().unwrap()); + + handle_update_configuration(&mut doc, "test", "0").unwrap(); + + assert_eq!(doc["test"].as_integer().unwrap(), 0); + + handle_update_configuration(&mut doc, "test", "0.0").unwrap(); + + assert!(doc["test"].is_float()); + + handle_update_configuration(&mut doc, "test", "a string").unwrap(); + + assert_eq!(doc["test"].as_str().unwrap(), "a string"); + + handle_update_configuration(&mut doc, "test", "\"true\"").unwrap(); + + assert_eq!(doc["test"].as_str().unwrap(), "true"); + } + + #[test] + fn test_update_config_empty() { + let mut doc = create_doc(); + + assert!(handle_update_configuration(&mut doc, "", "true").is_err()); + assert!(handle_update_configuration(&mut doc, ".....", "true").is_err()); + assert!(handle_update_configuration(&mut doc, "a.a.a..a.a", "true").is_err()); + assert!(handle_update_configuration(&mut doc, "a.a.a.a.a.", "true").is_err()); + } + + #[test] + fn test_update_config_deep() { + let mut doc = create_doc(); + + handle_update_configuration(&mut doc, "a.b.c.d.e.f.g.h", "true").unwrap(); + + assert!(doc["a"]["b"]["c"]["d"]["e"]["f"]["g"]["h"] + .as_bool() + .unwrap()) + } }