From a6cec155d09c369db995e788482bc3f7382b4455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Thu, 13 Feb 2025 13:32:01 +0100 Subject: [PATCH] Implement selection groups (#119) * Selection groups 1 * Allow requiring any of a selection group * Fix toggling group options * Fix disabling based on !requirement * Search for group in nested options, too * Display help for categories * Changelog * Clean up --- CHANGELOG.md | 1 + src/config.rs | 331 +++++++++++++++++++++++++++++++++++++++++++--- src/template.rs | 7 +- src/tui.rs | 2 +- xtask/src/main.rs | 4 +- 5 files changed, 321 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53f658f..a8caf6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Support for `ELIF` conditions (#96) - Display help text (#100, #103) - Added an option to enable unstable HAL features (#104) +- Added support for selection groups (#119) ### Changed - Update `probe-rs run` arguments (#90) diff --git a/src/config.rs b/src/config.rs index dc37ef3..7c4fb10 100644 --- a/src/config.rs +++ b/src/config.rs @@ -12,6 +12,13 @@ pub struct ActiveConfiguration<'c> { } impl ActiveConfiguration<'_> { + pub fn is_group_selected(&self, group: &str) -> bool { + self.selected.iter().any(|s| { + let option = find_option(s, self.options).unwrap(); + option.selection_group == group + }) + } + pub fn is_selected(&self, option: &str) -> bool { self.selected_index(option).is_some() } @@ -20,40 +27,118 @@ impl ActiveConfiguration<'_> { self.selected.iter().position(|s| s == option) } + /// Tries to deselect all options in a selection gropu. Returns false if it's prevented by some + /// requirement. + fn deselect_group( + selected: &mut Vec, + options: &[GeneratorOptionItem], + group: &str, + ) -> bool { + // No group, nothing to deselect + if group.is_empty() { + return true; + } + + // Avoid deselecting some options then failing. + if !selected.iter().all(|s| { + let o = find_option(s, options).unwrap(); + if o.selection_group == group { + // We allow deselecting group options because we are changing the options in the + // group, so after this operation the group have a selected item still. + Self::can_be_disabled_impl(selected, options, s, true) + } else { + true + } + }) { + return false; + } + + selected.retain(|s| { + let option = find_option(s, options).unwrap(); + option.selection_group != group + }); + + true + } + pub fn select(&mut self, option: String) { + let o = find_option(&option, self.options).unwrap(); + if !self.is_option_active(o) { + return; + } + if !Self::deselect_group(&mut self.selected, self.options, &o.selection_group) { + return; + } self.selected.push(option); } + /// Returns whether an item is active (can be selected). + /// + /// This function is different from `is_option_active` in that it handles categories as well. pub fn is_active(&self, item: &GeneratorOptionItem) -> bool { match item { - GeneratorOptionItem::Category(options) => { - for sub in options.options.iter() { + GeneratorOptionItem::Category(category) => { + if !self.requirements_met(&category.requires) { + return false; + } + for sub in category.options.iter() { if self.is_active(sub) { return true; } } false } - GeneratorOptionItem::Option(option) => self.requirements_met(option), + GeneratorOptionItem::Option(option) => self.is_option_active(option), } } - pub fn requirements_met(&self, option: &GeneratorOption) -> bool { - if !option.chips.is_empty() && !option.chips.contains(&self.chip) { - return false; - } - - // Are this option's requirements met? - for requirement in option.requires.iter() { + /// Returns whether all requirements are met. + /// + /// A requirement may be: + /// - an `option` + /// - the absence of an `!option` + /// - a `selection_group`, which means one option in that selection group must be selected + /// - the absence of a `!selection_group`, which means no option in that selection group must + /// be selected + /// + /// A selection group must not have the same name as an option. + fn requirements_met(&self, requires: &[String]) -> bool { + for requirement in requires { let (key, expected) = if let Some(requirement) = requirement.strip_prefix('!') { (requirement, false) } else { (requirement.as_str(), true) }; - if self.is_selected(key) != expected { - return false; + // Requirement is an option that must be selected? + if self.is_selected(key) == expected { + continue; } + + // Requirement is a group that must have a selected option? + let is_group = Self::group_exists(key, self.options); + if is_group && self.is_group_selected(key) == expected { + continue; + } + + return false; + } + + true + } + + /// Returns whether an option is active (can be selected). + /// + /// This involves checking if the option is available for the current chip, if it's not + /// disabled by any other selected option, and if all its requirements are met. + pub fn is_option_active(&self, option: &GeneratorOption) -> bool { + if !option.chips.is_empty() && !option.chips.contains(&self.chip) { + return false; + } + + // Are this option's requirements met? + if !self.requirements_met(&option.requires) { + return false; } // Does any of the enabled options have a requirement against this one? @@ -77,12 +162,23 @@ impl ActiveConfiguration<'_> { // An option can only be disabled if it's not required by any other selected option. pub fn can_be_disabled(&self, option: &str) -> bool { - for selected in self.selected.iter() { - let Some(selected_option) = find_option(selected, self.options) else { - ratatui::restore(); - panic!("selected option not found: {selected}"); - }; - if selected_option.requires.iter().any(|o| o == option) { + Self::can_be_disabled_impl(&self.selected, self.options, option, false) + } + + fn can_be_disabled_impl( + selected: &[String], + options: &[GeneratorOptionItem], + option: &str, + allow_deselecting_group: bool, + ) -> bool { + let op = find_option(option, options).unwrap(); + for selected in selected.iter() { + let selected_option = find_option(selected, options).unwrap(); + if selected_option + .requires + .iter() + .any(|o| o == option || (o == &op.selection_group && !allow_deselecting_group)) + { return false; } } @@ -125,6 +221,13 @@ impl ActiveConfiguration<'_> { disabled_by, } } + + fn group_exists(key: &str, options: &[GeneratorOptionItem]) -> bool { + options.iter().any(|o| match o { + GeneratorOptionItem::Option(o) => o.selection_group == key, + GeneratorOptionItem::Category(c) => Self::group_exists(key, &c.options), + }) + } } pub struct Relationships<'a> { @@ -160,8 +263,8 @@ mod test { use esp_metadata::Chip; use crate::{ - config::ActiveConfiguration, - template::{GeneratorOption, GeneratorOptionItem}, + config::{find_option, ActiveConfiguration}, + template::{GeneratorOption, GeneratorOptionCategory, GeneratorOptionItem}, }; #[test] @@ -170,6 +273,7 @@ mod test { GeneratorOptionItem::Option(GeneratorOption { name: "option1".to_string(), display_name: "Foobar".to_string(), + selection_group: "".to_string(), help: "".to_string(), chips: vec![Chip::Esp32], requires: vec!["option2".to_string()], @@ -177,6 +281,7 @@ mod test { GeneratorOptionItem::Option(GeneratorOption { name: "option2".to_string(), display_name: "Barfoo".to_string(), + selection_group: "".to_string(), help: "".to_string(), chips: vec![Chip::Esp32], requires: vec![], @@ -196,4 +301,190 @@ mod test { assert_eq!(rels.requires, <&[&str]>::default()); assert_eq!(rels.required_by, &["option1"]); } + + #[test] + fn selecting_one_in_group_deselects_other() { + let options = &[ + GeneratorOptionItem::Option(GeneratorOption { + name: "option1".to_string(), + display_name: "Foobar".to_string(), + selection_group: "group".to_string(), + help: "".to_string(), + chips: vec![Chip::Esp32], + requires: vec![], + }), + GeneratorOptionItem::Option(GeneratorOption { + name: "option2".to_string(), + display_name: "Barfoo".to_string(), + selection_group: "group".to_string(), + help: "".to_string(), + chips: vec![Chip::Esp32], + requires: vec![], + }), + GeneratorOptionItem::Option(GeneratorOption { + name: "option3".to_string(), + display_name: "Prevents deselecting option2".to_string(), + selection_group: "".to_string(), + help: "".to_string(), + chips: vec![Chip::Esp32], + requires: vec!["option2".to_string()], + }), + ]; + let mut active = ActiveConfiguration { + chip: Chip::Esp32, + selected: vec![], + options, + }; + + active.select("option1".to_string()); + assert_eq!(active.selected, &["option1"]); + + active.select("option2".to_string()); + assert_eq!(active.selected, &["option2"]); + + // Enable option3, which prevents deselecting option2, which disallows selecting option1 + active.select("option3".to_string()); + assert_eq!(active.selected, &["option2", "option3"]); + + active.select("option1".to_string()); + assert_eq!(active.selected, &["option2", "option3"]); + } + + #[test] + fn depending_on_group_allows_changing_group_option() { + let options = &[ + GeneratorOptionItem::Category(GeneratorOptionCategory { + name: "group-options".to_string(), + display_name: "Group options".to_string(), + help: "".to_string(), + requires: vec![], + options: vec![ + GeneratorOptionItem::Option(GeneratorOption { + name: "option1".to_string(), + display_name: "Foobar".to_string(), + selection_group: "group".to_string(), + help: "".to_string(), + chips: vec![Chip::Esp32], + requires: vec![], + }), + GeneratorOptionItem::Option(GeneratorOption { + name: "option2".to_string(), + display_name: "Barfoo".to_string(), + selection_group: "group".to_string(), + help: "".to_string(), + chips: vec![Chip::Esp32], + requires: vec![], + }), + ], + }), + GeneratorOptionItem::Option(GeneratorOption { + name: "option3".to_string(), + display_name: "Requires any in group to be selected".to_string(), + selection_group: "".to_string(), + help: "".to_string(), + chips: vec![Chip::Esp32], + requires: vec!["group".to_string()], + }), + GeneratorOptionItem::Option(GeneratorOption { + name: "option4".to_string(), + display_name: "Extra option that depends on something".to_string(), + selection_group: "".to_string(), + help: "".to_string(), + chips: vec![Chip::Esp32], + requires: vec!["option3".to_string()], + }), + ]; + let mut active = ActiveConfiguration { + chip: Chip::Esp32, + selected: vec![], + options, + }; + + // Nothing is selected in group, so option3 can't be selected + active.select("option3".to_string()); + assert_eq!(active.selected, empty()); + + active.select("option1".to_string()); + assert_eq!(active.selected, &["option1"]); + + active.select("option3".to_string()); + assert_eq!(active.selected, &["option1", "option3"]); + + // The rejection algorithm must not trigger on unrelated options. This option is + // meant to test the group filtering. It prevents disabling "option3" but it does not + // belong to `group`, so it should not prevent selecting between "option1" or "option2". + active.select("option4".to_string()); + assert_eq!(active.selected, &["option1", "option3", "option4"]); + + active.select("option2".to_string()); + assert_eq!(active.selected, &["option3", "option4", "option2"]); + } + + #[test] + fn depending_on_group_prevents_deselecting() { + let options = &[ + GeneratorOptionItem::Option(GeneratorOption { + name: "option1".to_string(), + display_name: "Foobar".to_string(), + selection_group: "group".to_string(), + help: "".to_string(), + chips: vec![Chip::Esp32], + requires: vec![], + }), + GeneratorOptionItem::Option(GeneratorOption { + name: "option2".to_string(), + display_name: "Barfoo".to_string(), + selection_group: "".to_string(), + help: "".to_string(), + chips: vec![Chip::Esp32], + requires: vec!["group".to_string()], + }), + ]; + let mut active = ActiveConfiguration { + chip: Chip::Esp32, + selected: vec![], + options, + }; + + active.select("option1".to_string()); + active.select("option2".to_string()); + + // Option1 can't be deselected because option2 requires that a `group` option is selected + assert!(!active.can_be_disabled("option1")); + } + + #[test] + fn requiring_not_option_only_rejects_existing_group() { + let options = &[ + GeneratorOptionItem::Option(GeneratorOption { + name: "option1".to_string(), + display_name: "Foobar".to_string(), + selection_group: "group".to_string(), + help: "".to_string(), + chips: vec![Chip::Esp32], + requires: vec![], + }), + GeneratorOptionItem::Option(GeneratorOption { + name: "option2".to_string(), + display_name: "Barfoo".to_string(), + selection_group: "".to_string(), + help: "".to_string(), + chips: vec![Chip::Esp32], + requires: vec!["!option1".to_string()], + }), + ]; + let mut active = ActiveConfiguration { + chip: Chip::Esp32, + selected: vec![], + options, + }; + + active.select("option1".to_string()); + let opt2 = find_option("option2", options).unwrap(); + assert!(!active.is_option_active(opt2)); + } + + fn empty() -> &'static [&'static str] { + &[] + } } diff --git a/src/template.rs b/src/template.rs index cf9f4c8..ac828b6 100644 --- a/src/template.rs +++ b/src/template.rs @@ -5,6 +5,9 @@ use serde::{Deserialize, Serialize}; pub struct GeneratorOption { pub name: String, pub display_name: String, + /// Selecting one option in the group deselect other options of the same group. + #[serde(default)] + pub selection_group: String, #[serde(default)] pub help: String, #[serde(default)] @@ -26,6 +29,8 @@ pub struct GeneratorOptionCategory { #[serde(default)] pub help: String, #[serde(default)] + pub requires: Vec, + #[serde(default)] pub options: Vec, } @@ -80,7 +85,7 @@ impl GeneratorOptionItem { pub fn requires(&self) -> &[String] { match self { - GeneratorOptionItem::Category(_) => &[], + GeneratorOptionItem::Category(category) => category.requires.as_slice(), GeneratorOptionItem::Option(option) => option.requires.as_slice(), } } diff --git a/src/tui.rs b/src/tui.rs index f479b1a..a3178a2 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -83,7 +83,7 @@ impl<'app> Repository<'app> { if self.config.can_be_disabled(&option.name) { self.config.selected.swap_remove(i); } - } else if self.config.requirements_met(option) { + } else { self.config.select(option.name.clone()); } } diff --git a/xtask/src/main.rs b/xtask/src/main.rs index dbcb419..dee499c 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -146,7 +146,7 @@ fn enable_config_and_dependencies(config: &mut ActiveConfiguration, option: &str enable_config_and_dependencies(config, dependency)?; } - if !config.requirements_met(option) { + if !config.is_option_active(option) { return Ok(()); } @@ -158,7 +158,7 @@ fn enable_config_and_dependencies(config: &mut ActiveConfiguration, option: &str fn is_valid(config: &ActiveConfiguration) -> bool { for item in config.selected.iter() { let option = find_option(item, &config.options).unwrap(); - if !config.requirements_met(option) { + if !config.is_option_active(option) { return false; } }