From e88525fa1991d0e5685fe376147ac4625669ea7a Mon Sep 17 00:00:00 2001 From: vikingowl Date: Wed, 13 May 2026 03:52:06 +0200 Subject: [PATCH] fix(providers): surface dynamic providers in doctor + providers list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ProviderManager::available_providers() and available_provider_types() iterated only the static `Vec>` field, never the parallel `Vec>`. Result: calc, conv, websearch, filesearch ran fine but were silently absent from `owlry doctor` and `owlry providers ` outputs. Fix: - Add prefix(), icon(), tab_label(), search_noun() as defaulted methods on the DynamicProvider trait (matching the static Provider trait shape). Default values: None / "application-x-addon" / None / None. - Override those methods on the four dynamic provider impls with sensible values (e.g. calculator: ":calc" / "accessories-calculator" / "Calc" / "math expression"). - Extend available_providers() and available_provider_types() to iterate builtin_dynamic too. Each dynamic provider always reports as ProviderType::Plugin() — Application/Command/Dmenu variants are static-only, but the match is defensive. Tests added (TDD characterization for 2.0.1): - available_providers_includes_dynamic_providers — synthetic DynRich provider with every overridable method set; asserts the descriptor comes back with the right id/prefix/icon/position/tab_label/search_noun. - available_provider_types_includes_dynamic_providers — verifies the type list returns both static and dynamic types. - dynamic_provider_trait_defaults_return_documented_values — minimal impl returns the documented defaults. After this fix, `owlry doctor` reports 11 providers instead of 7 on a default --features full build, and `owlry providers calc` returns the calculator's full metadata instead of 'No provider with id'. 248 tests pass with --features full (was 245). --- crates/owlry/src/providers/calculator.rs | 16 +++ crates/owlry/src/providers/converter/mod.rs | 16 +++ crates/owlry/src/providers/filesearch.rs | 16 +++ crates/owlry/src/providers/mod.rs | 149 +++++++++++++++++++- crates/owlry/src/providers/websearch.rs | 16 +++ 5 files changed, 211 insertions(+), 2 deletions(-) diff --git a/crates/owlry/src/providers/calculator.rs b/crates/owlry/src/providers/calculator.rs index 900b0d6..7e19825 100644 --- a/crates/owlry/src/providers/calculator.rs +++ b/crates/owlry/src/providers/calculator.rs @@ -22,6 +22,22 @@ impl DynamicProvider for CalculatorProvider { 10_000 } + fn prefix(&self) -> Option<&str> { + Some(":calc") + } + + fn icon(&self) -> &str { + "accessories-calculator" + } + + fn tab_label(&self) -> Option<&str> { + Some("Calc") + } + + fn search_noun(&self) -> Option<&str> { + Some("math expression") + } + fn query(&self, query: &str) -> Vec { let expr = match extract_expression(query) { Some(e) if !e.is_empty() => e, diff --git a/crates/owlry/src/providers/converter/mod.rs b/crates/owlry/src/providers/converter/mod.rs index 2c36e95..1541704 100644 --- a/crates/owlry/src/providers/converter/mod.rs +++ b/crates/owlry/src/providers/converter/mod.rs @@ -28,6 +28,22 @@ impl DynamicProvider for ConverterProvider { 9_000 } + fn prefix(&self) -> Option<&str> { + Some(":conv") + } + + fn icon(&self) -> &str { + PROVIDER_ICON + } + + fn tab_label(&self) -> Option<&str> { + Some("Convert") + } + + fn search_noun(&self) -> Option<&str> { + Some("unit / currency conversion") + } + fn query(&self, query: &str) -> Vec { let query_str = query.trim(); // Strip prefix diff --git a/crates/owlry/src/providers/filesearch.rs b/crates/owlry/src/providers/filesearch.rs index 602ec36..e2a1114 100644 --- a/crates/owlry/src/providers/filesearch.rs +++ b/crates/owlry/src/providers/filesearch.rs @@ -192,6 +192,22 @@ impl DynamicProvider for FileSearchProvider { 8_000 } + fn prefix(&self) -> Option<&str> { + Some(":file") + } + + fn icon(&self) -> &str { + "system-file-manager" + } + + fn tab_label(&self) -> Option<&str> { + Some("Files") + } + + fn search_noun(&self) -> Option<&str> { + Some("files") + } + fn query(&self, query: &str) -> Vec { self.evaluate(query) } diff --git a/crates/owlry/src/providers/mod.rs b/crates/owlry/src/providers/mod.rs index 1a44cf6..3ae4b4c 100644 --- a/crates/owlry/src/providers/mod.rs +++ b/crates/owlry/src/providers/mod.rs @@ -217,6 +217,23 @@ pub trait DynamicProvider: Send + Sync { fn query(&self, query: &str) -> Vec; fn priority(&self) -> u32; + /// Optional search prefix (e.g. ":calc"). None = no prefix. + fn prefix(&self) -> Option<&str> { + None + } + /// Icon name (XDG icon theme). + fn icon(&self) -> &str { + "application-x-addon" + } + /// Tab button label. + fn tab_label(&self) -> Option<&str> { + None + } + /// Search placeholder noun. + fn search_noun(&self) -> Option<&str> { + None + } + /// Handle a plugin action command. Returns true if handled. fn execute_action(&self, _command: &str) -> bool { false @@ -631,12 +648,19 @@ impl ProviderManager { } /// Get all available provider types (for UI tabs). + /// + /// Includes both static `Provider`s and dynamic `DynamicProvider`s so + /// callers like `owlry doctor` / `owlry providers` see the full picture. #[allow(dead_code)] pub fn available_provider_types(&self) -> Vec { - self.providers.iter().map(|p| p.provider_type()).collect() + self.providers + .iter() + .map(|p| p.provider_type()) + .chain(self.builtin_dynamic.iter().map(|p| p.provider_type())) + .collect() } - /// Get descriptors for all registered providers. + /// Get descriptors for all registered providers (static + dynamic). /// /// Used by the IPC server to report what providers are available to clients. pub fn available_providers(&self) -> Vec { @@ -675,6 +699,27 @@ impl ProviderManager { }); } + // Dynamic providers (calc, conv, websearch, filesearch). They're always + // ProviderType::Plugin() — the only other variants are + // Application/Command/Dmenu which are static-only. + for provider in &self.builtin_dynamic { + let id = match provider.provider_type() { + ProviderType::Plugin(type_id) => type_id, + // Defensive: keep the surface honest even if a future dynamic + // provider claims a non-Plugin type. + other => other.to_string(), + }; + descs.push(ProviderDescriptor { + id, + name: provider.name().to_string(), + prefix: provider.prefix().map(String::from), + icon: provider.icon().to_string(), + position: ProviderPosition::Normal.as_str().to_string(), + tab_label: provider.tab_label().map(String::from), + search_noun: provider.search_noun().map(String::from), + }); + } + descs } @@ -1186,6 +1231,106 @@ mod tests { assert!(pm.execute_plugin_action("DYN:thing")); } + #[test] + fn available_providers_includes_dynamic_providers() { + // Regression guard (2.0.1): dynamic providers must surface in the + // diagnostic list so `owlry doctor` and `owlry providers` can report + // them. Pre-2.0.1, only static providers were enumerated and the + // dynamic ones were silently absent despite running fine. + struct DynRich; + impl DynamicProvider for DynRich { + fn name(&self) -> &str { + "Rich Dynamic" + } + fn provider_type(&self) -> ProviderType { + ProviderType::Plugin("rich-dyn".into()) + } + fn query(&self, _q: &str) -> Vec { + Vec::new() + } + fn priority(&self) -> u32 { + 7_000 + } + fn prefix(&self) -> Option<&str> { + Some(":rdyn") + } + fn icon(&self) -> &str { + "rich-dynamic-icon" + } + fn tab_label(&self) -> Option<&str> { + Some("Rich") + } + fn search_noun(&self) -> Option<&str> { + Some("rich queries") + } + } + + let pm = ProviderManager::new(Vec::new(), vec![Box::new(DynRich)]); + let descs = pm.available_providers(); + assert_eq!(descs.len(), 1); + let d = &descs[0]; + assert_eq!(d.id, "rich-dyn"); + assert_eq!(d.name, "Rich Dynamic"); + assert_eq!(d.prefix.as_deref(), Some(":rdyn")); + assert_eq!(d.icon, "rich-dynamic-icon"); + assert_eq!(d.position, "normal"); + assert_eq!(d.tab_label.as_deref(), Some("Rich")); + assert_eq!(d.search_noun.as_deref(), Some("rich queries")); + } + + #[test] + fn available_provider_types_includes_dynamic_providers() { + struct DynStub; + impl DynamicProvider for DynStub { + fn name(&self) -> &str { + "stub" + } + fn provider_type(&self) -> ProviderType { + ProviderType::Plugin("stub".into()) + } + fn query(&self, _q: &str) -> Vec { + Vec::new() + } + fn priority(&self) -> u32 { + 0 + } + } + + let mock = MockProvider::new("Apps", ProviderType::Application); + let pm = ProviderManager::new(vec![Box::new(mock)], vec![Box::new(DynStub)]); + let types = pm.available_provider_types(); + assert_eq!(types.len(), 2); + assert!(types.contains(&ProviderType::Application)); + assert!(types.contains(&ProviderType::Plugin("stub".into()))); + } + + #[test] + fn dynamic_provider_trait_defaults_return_documented_values() { + // 2.0.1 added prefix/icon/tab_label/search_noun as defaulted methods + // on DynamicProvider so the trait can describe its own UI metadata + // without falling back to a hardcoded match table. + struct Minimal; + impl DynamicProvider for Minimal { + fn name(&self) -> &str { + "m" + } + fn provider_type(&self) -> ProviderType { + ProviderType::Plugin("m".into()) + } + fn query(&self, _: &str) -> Vec { + Vec::new() + } + fn priority(&self) -> u32 { + 0 + } + } + let m = Minimal; + assert_eq!(m.prefix(), None); + assert_eq!(m.icon(), "application-x-addon"); + assert_eq!(m.tab_label(), None); + assert_eq!(m.search_noun(), None); + } + #[test] fn execute_plugin_action_returns_false_when_nothing_handles() { let prov = RichMockProvider { diff --git a/crates/owlry/src/providers/websearch.rs b/crates/owlry/src/providers/websearch.rs index d08fa4d..1998f54 100644 --- a/crates/owlry/src/providers/websearch.rs +++ b/crates/owlry/src/providers/websearch.rs @@ -150,6 +150,22 @@ impl DynamicProvider for WebSearchProvider { 9_000 } + fn prefix(&self) -> Option<&str> { + Some(":web") + } + + fn icon(&self) -> &str { + "applications-internet" + } + + fn tab_label(&self) -> Option<&str> { + Some("Web") + } + + fn search_noun(&self) -> Option<&str> { + Some("the web") + } + fn query(&self, query: &str) -> Vec { match self.evaluate(query) { Some(item) => vec![item],