fix(providers): surface dynamic providers in doctor + providers list
ProviderManager::available_providers() and available_provider_types() iterated only the static `Vec<Box<dyn Provider>>` field, never the parallel `Vec<Box<dyn DynamicProvider>>`. Result: calc, conv, websearch, filesearch ran fine but were silently absent from `owlry doctor` and `owlry providers <id>` 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(<type_id>) — 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).
This commit is contained in:
@@ -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<LaunchItem> {
|
||||
let expr = match extract_expression(query) {
|
||||
Some(e) if !e.is_empty() => e,
|
||||
|
||||
@@ -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<LaunchItem> {
|
||||
let query_str = query.trim();
|
||||
// Strip prefix
|
||||
|
||||
@@ -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<LaunchItem> {
|
||||
self.evaluate(query)
|
||||
}
|
||||
|
||||
@@ -217,6 +217,23 @@ pub trait DynamicProvider: Send + Sync {
|
||||
fn query(&self, query: &str) -> Vec<LaunchItem>;
|
||||
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<ProviderType> {
|
||||
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<ProviderDescriptor> {
|
||||
@@ -675,6 +699,27 @@ impl ProviderManager {
|
||||
});
|
||||
}
|
||||
|
||||
// Dynamic providers (calc, conv, websearch, filesearch). They're always
|
||||
// ProviderType::Plugin(<type_id>) — 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<LaunchItem> {
|
||||
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<LaunchItem> {
|
||||
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<LaunchItem> {
|
||||
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 {
|
||||
|
||||
@@ -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<LaunchItem> {
|
||||
match self.evaluate(query) {
|
||||
Some(item) => vec![item],
|
||||
|
||||
Reference in New Issue
Block a user