From 62941a9f2f561cf648756a25950d88aefdaa04c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Milojevi=C4=87?= Date: Wed, 16 Sep 2026 15:23:58 +0200 Subject: [PATCH] feat: support inline plugin merging by name Shared plugin files previously replaced the complete inherited inline plugin list. Add per-file name merging and removal controls while keeping replacement as the default for existing configurations. Closes #616 --- docs/configuration.md | 70 +++++++++++- src/app/tests.rs | 35 ++++++ src/config.rs | 253 ++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 349 insertions(+), 9 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 098f33c..42054d9 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -74,13 +74,77 @@ keep personal settings in another: TOML and YAML files can be mixed in `conf.d/`. Drop-in files merge before the cluster and context overrides, so those keep the last word. The merge rules are the same as for [overrides](#per-cluster-and-per-context-overrides): tables -merge key by key, and arrays like `[[plugins]]` replace the base value. An +merge key by key, and arrays like `[[plugins]]` replace the base value by default. +Use [inline plugin merging](#inline-plugin-merging) to combine plugins by name. An invalid drop-in file is skipped with a warning. `:config` lists the drop-in files and `:reload` reads them again. The [Home Manager module](home-manager.md) can manage generated TOML settings or existing TOML and YAML files, including cluster and context overrides. +## Inline plugin merging + +By default, each file that contains `plugins` replaces the complete inherited +inline plugin list. To keep shared and personal plugins, set +`plugins_merge = "name"` at the top level of each file that must combine lists. + +For example, keep personal plugins in `config.toml` and put this shared file in +`conf.d/10-team.toml`: + +```toml +plugins_merge = "name" + +[[plugins]] +name = "Team logs" +key = "ctrl-l" +command = "kubectl" +args = ["logs", "-f", "-n", "$NAMESPACE", "$NAME"] +scopes = ["pods"] +output = "terminal" +mutating = false +``` + +Names match exactly and are case-sensitive. A new name is added at the end of +the list. An existing name keeps its position, but the later entry replaces its +complete definition. Fields are not combined: include all required fields, +including `command`, in each replacement. This prevents old arguments or +permissions from carrying into a new command. If a name occurs more than once, +the last definition wins and the first position is kept. + +Use `plugins_remove` to remove inherited inline entries by name. Removal runs +before the entries in the same file are applied. Unknown names have no effect. +A file can remove a name and then add a new definition with that name. + +```toml +# conf.d/20-personal.toml +plugins_merge = "name" +plugins_remove = ["Team logs"] + +[[plugins]] +name = "Personal logs" +palette = "personal-logs" +command = "kubectl" +args = ["logs", "--tail=100", "-n", "$NAMESPACE", "$NAME"] +scopes = ["pods"] +output = "popup" +mutating = false +``` + +Both controls apply only to the file that contains them. A later file uses +`plugins_merge = "replace"` unless it explicitly selects `"name"`. A file with +no `plugins` field keeps the inherited list, apart from requested removals. +In replace mode, `plugins = []` clears the inherited inline list. In name mode, +an empty list adds nothing. Other arrays still use replacement. + +The controls work in TOML and YAML, in the base file, `conf.d/`, cluster files, +and context files. The order stays the same: base, drop-in files in file name +order, cluster, then context. `:reload` applies the same rules. + +Package and bundled plugins load after inline configuration. These controls do +not disable those plugins. Removing an inline entry can expose a package or +bundled plugin that the entry previously replaced. Package terminal support is +unchanged. + ## YAML format Examples below use TOML unless marked as YAML. Use the same field names in @@ -356,7 +420,9 @@ in the same directory. Overrides merge over the base config, cluster level first, then context level. Tables like `[aliases]` and `[skin.colors]` merge key by key. Everything else - -strings, booleans, and arrays like `[[plugins]]` - replaces the base value. +strings, booleans, and arrays like `[[plugins]]` - replaces the base value by +default. [Inline plugin merging](#inline-plugin-merging) can change this rule +for plugins in an individual file. Directory names are the kubeconfig names, with any character that isn't a letter, digit, `.`, `_`, or `-` replaced by `-`. So the EKS context diff --git a/src/app/tests.rs b/src/app/tests.rs index 030e47a..75b741f 100644 --- a/src/app/tests.rs +++ b/src/app/tests.rs @@ -16456,6 +16456,41 @@ columns = [ std::fs::remove_dir_all(dir).unwrap(); } +#[tokio::test] +async fn reload_merges_and_removes_inline_plugins_through_the_palette() { + let dir = + std::env::temp_dir().join(format!("sofka-reload-plugin-merge-{}", std::process::id())); + write_config( + &dir, + "plugins = [{ name = 'personal', palette = 'personal', command = 'old', args = ['old'] }]", + ); + std::fs::create_dir_all(dir.join("conf.d")).unwrap(); + let path = dir.join("conf.d/team.yaml"); + std::fs::write(&path, "plugins_merge: name\nplugins:\n - name: team\n palette: team\n command: team\n - name: personal\n palette: personal\n command: new\n").unwrap(); + let (mut app, _rx) = test_app(); + app.config = crate::config::ConfigLoader::from_dir(Some(dir.clone())); + palette(&mut app, "reload"); + assert!(!app.flash_err, "{}", app.flash); + assert!(app.config_warnings.is_empty(), "{:?}", app.config_warnings); + let personal = app.plugins.iter().find(|p| p.name == "personal").unwrap(); + assert_eq!(personal.command, "new"); + assert!(personal.args.is_empty()); + assert!(app.plugins.iter().any(|p| p.name == "team")); + std::fs::write(&path, "plugins_remove: [personal]\nplugins_merge: name\nplugins:\n - name: team\n palette: team\n command: team\n").unwrap(); + palette(&mut app, "reload"); + assert!(!app.flash_err, "{}", app.flash); + assert!(!app.plugins.iter().any(|p| p.name == "personal")); + assert!(app.plugins.iter().any(|p| p.name == "team")); + std::fs::write(&path, "plugins: []\n").unwrap(); + palette(&mut app, "reload"); + assert!( + !app.plugins + .iter() + .any(|p| p.name == "personal" || p.name == "team") + ); + std::fs::remove_dir_all(dir).unwrap(); +} + #[tokio::test] async fn reload_applies_config_changes_live() { let dir = std::env::temp_dir().join(format!("sofka-app-reload-ok-{}", std::process::id())); diff --git a/src/config.rs b/src/config.rs index e210c85..038ca41 100644 --- a/src/config.rs +++ b/src/config.rs @@ -16,8 +16,8 @@ //! //! Override files are partial configs merged over the base (cluster level //! first, then context level): tables like `[aliases]` and `[skin.colors]` -//! merge key-by-key, everything else — strings, booleans, arrays like -//! `[[plugins]]` — replaces the base value. Directory names are the +//! merge key by key. Other values replace the base value unless a file sets +//! `plugins_merge = "name"` for inline plugins. Directory names are the //! kubeconfig names sanitized for the filesystem: any character other than //! ASCII letters, digits, `.`, `_` and `-` becomes `-`, so an EKS context //! `arn:aws:eks:eu-west-1:123:cluster/prod` lives in @@ -41,6 +41,21 @@ use serde::Deserialize; mod document; mod key_migration; +#[derive(Debug, Default, Deserialize)] +#[serde(default)] +pub struct PluginMergeControls { + plugins_merge: PluginMergeMode, + plugins_remove: Vec, +} + +#[derive(Debug, Default, Deserialize, PartialEq)] +#[serde(rename_all = "lowercase")] +enum PluginMergeMode { + #[default] + Replace, + Name, +} + #[derive(Debug, Default, Deserialize)] #[serde(default)] pub struct Config { @@ -67,6 +82,8 @@ pub struct Config { pub favorite_namespaces: Vec, /// User-defined shell-out plugins bound to keys. pub plugins: Vec, + #[serde(flatten)] + pub plugin_merge: PluginMergeControls, /// Saved navigation commands bound to keys and the palette — see /// [`Bookmark`]. Validated by [`bookmark_warnings`]. pub bookmarks: Vec, @@ -1410,7 +1427,11 @@ impl ConfigLoader { { migrations.push(migration); } - let base = merged.clone(); + let mut base = toml::Value::Table(toml::Table::new()); + if let Err(e) = merge_config(&mut base, merged) { + warnings.push(format!("ignoring invalid base config: {e}")); + } + let mut merged = base.clone(); for path in self.dropin_paths() { match read_dropin(&path) { @@ -1418,7 +1439,9 @@ impl ConfigLoader { if let Some(migration) = key_migration::prepare(&mut v, &path, &mut warnings) { migrations.push(migration); } - merge(&mut merged, v); + if let Err(e) = merge_config(&mut merged, v) { + warnings.push(format!("ignoring invalid {}: {e}", path.display())); + } } Ok(None) => {} Err(e) => warnings.push(format!("ignoring invalid {}: {e}", path.display())), @@ -1431,8 +1454,12 @@ impl ConfigLoader { if let Some(migration) = key_migration::prepare(&mut v, &path, &mut warnings) { migrations.push(migration); } - merge(&mut merged, v.clone()); - merge(&mut overlay, v); + match merge_config(&mut merged, v.clone()) { + Ok(()) => merge(&mut overlay, v), + Err(e) => { + warnings.push(format!("ignoring invalid {}: {e}", path.display())) + } + } } Ok(None) => {} Err(e) => warnings.push(format!("ignoring invalid {}: {e}", path.display())), @@ -1484,6 +1511,56 @@ impl ConfigLoader { } } +/// Apply plugin controls once at the document root, then merge other settings. +fn merge_config(base: &mut toml::Value, mut overlay: toml::Value) -> Result<(), toml::de::Error> { + let controls: PluginMergeControls = overlay.clone().try_into()?; + let Some(table) = overlay.as_table_mut() else { + return Ok(()); + }; + table.remove("plugins_merge"); + table.remove("plugins_remove"); + if let Some(entries) = base.get_mut("plugins").and_then(toml::Value::as_array_mut) { + entries.retain(|entry| { + !entry + .get("name") + .and_then(toml::Value::as_str) + .is_some_and(|name| { + controls + .plugins_remove + .iter() + .any(|removed| removed == name) + }) + }); + } + if controls.plugins_merge == PluginMergeMode::Name + && let Some(toml::Value::Array(entries)) = table.get("plugins") + { + let mut combined = base + .get("plugins") + .and_then(toml::Value::as_array) + .cloned() + .unwrap_or_default(); + // Process both lists to remove repeated names while keeping their first position. + let incoming = combined + .drain(..) + .chain(entries.iter().cloned()) + .collect::>(); + for entry in incoming { + let name = entry.get("name").and_then(toml::Value::as_str); + if let Some(index) = combined.iter().position(|old| { + name.is_some() && old.get("name").and_then(toml::Value::as_str) == name + }) { + combined[index] = entry; + } else { + combined.push(entry); + } + } + table.insert("plugins".into(), toml::Value::Array(combined)); + } + merge(base, overlay); + Ok(()) +} + /// Recursively merge `overlay` into `base`: tables merge key-by-key, any /// other value (scalar or array) replaces the base one wholesale. fn merge(base: &mut toml::Value, overlay: toml::Value) { @@ -1549,7 +1626,14 @@ fn read_dropin(path: &Path) -> Result, String> { fn read_file(path: &Path) -> Result, String> { match std::fs::read_to_string(path) { - Ok(text) => document::parse(path, &text).map(Some), + Ok(text) => { + let value = document::parse(path, &text)?; + let _: PluginMergeControls = value + .clone() + .try_into() + .map_err(|e: toml::de::Error| e.to_string())?; + Ok(Some(value)) + } Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), Err(e) => Err(e.to_string()), } @@ -2053,6 +2137,161 @@ mod tests { parse_doc(s).unwrap() } + #[test] + fn plugin_name_merge_replaces_whole_entries_and_preserves_order() { + let mut base = val(r#" +plugins = [ + { name = "a", command = "old", args = ["old"], mutating = false }, + { name = "b", command = "keep" }, + { name = "a", command = "duplicate" }, +] +bookmarks = [{ name = "old", resource = "pods" }] +"#); + merge_config( + &mut base, + val(r#" +plugins_merge = "name" +plugins = [ + { name = "a", command = "first" }, + { name = "c", command = "new" }, + { name = "a", command = "last" }, + { name = "A", command = "case-sensitive" }, +] +bookmarks = [] +"#), + ) + .unwrap(); + let config: Config = base.clone().try_into().unwrap(); + assert_eq!( + config + .plugins + .iter() + .map(|p| p.name.as_str()) + .collect::>(), + ["a", "b", "c", "A"] + ); + assert_eq!(config.plugins[0].command, "last"); + assert!(config.plugins[0].args.is_empty()); + assert_eq!(config.plugins[0].mutating, None); + assert!(config.bookmarks.is_empty()); + assert!(base.get("plugins_merge").is_none()); + + merge_config(&mut base, val("plugins_merge = 'name'\nplugins = []")).unwrap(); + assert_eq!(base["plugins"].as_array().unwrap().len(), 4); + merge_config(&mut base, val("plugins = []")).unwrap(); + assert!(base["plugins"].as_array().unwrap().is_empty()); + } + + #[test] + fn plugin_removal_runs_before_addition_and_does_not_carry_forward() { + let mut base = + val("plugins = [{ name = 'a', command = 'old' }, { name = 'b', command = 'keep' }]"); + merge_config(&mut base, val("plugins_remove = ['a', 'absent']")).unwrap(); + assert_eq!(base["plugins"].as_array().unwrap().len(), 1); + merge_config( + &mut base, + val("plugins_merge = 'name'\nplugins = [{ name = 'a', command = 'new' }]"), + ) + .unwrap(); + merge_config(&mut base, val("plugins_remove = ['b']\nplugins_merge = 'name'\nplugins = [{ name = 'b', command = 'redefined' }]")).unwrap(); + let config: Config = base.try_into().unwrap(); + assert_eq!( + config + .plugins + .iter() + .map(|p| p.name.as_str()) + .collect::>(), + ["a", "b"] + ); + assert_eq!(config.plugins[1].command, "redefined"); + } + + #[test] + fn invalid_plugin_controls_do_not_change_the_base() { + for text in [ + "plugins_merge = 'invalid'", + "plugins_merge = true", + "plugins_remove = 'a'", + "plugins_remove = [1]", + ] { + assert!(validate(text).is_err(), "{text}"); + let mut base = val("plugins = [{ name = 'a', command = 'keep' }]"); + let before = base.clone(); + assert!(merge_config(&mut base, val(text)).is_err()); + assert_eq!(base, before); + } + } + + #[test] + fn plugin_controls_follow_file_order_in_toml_and_yaml() { + let dir = std::env::temp_dir().join(format!("sofka-plugin-merge-{}", std::process::id())); + let context = dir.join("clusters/c1/ctx"); + std::fs::create_dir_all(&context).unwrap(); + std::fs::create_dir_all(dir.join("conf.d")).unwrap(); + std::fs::write( + dir.join("config.toml"), + "plugins_merge = 'name'\nplugins = [{ name = 'personal', command = 'base' }]", + ) + .unwrap(); + std::fs::write( + dir.join("conf.d/10-team.yaml"), + "plugins_merge: name\nplugins:\n - name: team\n command: team\n", + ) + .unwrap(); + std::fs::write( + dir.join("conf.d/20-personal.toml"), + "plugins_merge = 'name'\nplugins = [{ name = 'personal', command = 'dropin' }]", + ) + .unwrap(); + std::fs::write(dir.join("clusters/c1/config.yaml"), "plugins_merge: name\nplugins_remove: [team]\nplugins:\n - name: personal\n command: cluster\n").unwrap(); + std::fs::write( + context.join("config.toml"), + "plugins_merge = 'name'\nplugins = [{ name = 'personal', command = 'context' }]", + ) + .unwrap(); + let loader = ConfigLoader::from_dir(Some(dir.clone())); + for (ctx, cluster, command, team) in [ + ("", "", "dropin", true), + ("", "c1", "cluster", false), + ("ctx", "c1", "context", false), + ] { + let resolved = loader.resolve(ctx, cluster); + assert!(resolved.warnings.is_empty(), "{:?}", resolved.warnings); + assert_eq!( + resolved + .config + .plugins + .iter() + .find(|p| p.name == "personal") + .unwrap() + .command, + command + ); + assert_eq!( + resolved.config.plugins.iter().any(|p| p.name == "team"), + team + ); + } + std::fs::write(context.join("config.toml"), "plugins = []").unwrap(); + let resolved = loader.resolve("ctx", "c1"); + assert!( + !resolved + .config + .plugins + .iter() + .any(|p| p.name == "personal" || p.name == "team") + ); + std::fs::write( + context.join("config.toml"), + "plugins_merge = 'invalid'\nplugins = []", + ) + .unwrap(); + let resolved = loader.resolve("ctx", "c1"); + assert_eq!(resolved.warnings.len(), 1); + assert!(resolved.config.plugins.iter().any(|p| p.name == "personal")); + std::fs::remove_dir_all(dir).unwrap(); + } + #[test] fn merge_tables_key_by_key_scalars_and_arrays_replace() { let mut base = val(r##"