refactor: Unify filter plugin creation and option handling

Co-authored-by: aider (openai/andrew/openrouter/deepseek/deepseek-chat-v3.1) <aider@aider.chat>

src/filter_plugin/mod.rs

@@ -25,9 +25,9 @@ pub trait FilterPlugin: Send {
     fn options(&self) -> Vec<FilterOption>;
 }
 
-#[derive(Debug, EnumString, strum::VariantNames)]
+#[derive(Debug, EnumString, strum::VariantNames, strum::Display)]
 #[strum(serialize_all = "snake_case")]
-enum FilterType {
+pub enum FilterType {
     HeadBytes,
     HeadLines,
     TailBytes,
@@ -143,35 +143,7 @@ pub fn parse_filter_string(filter_str: &str) -> Result<FilterChain> {
                 
                 // Create the appropriate filter plugin
                 if let Ok(filter_type) = FilterType::from_str(filter_name) {
-                    let plugin = match filter_type {
+                    let plugin = create_filter_with_options(filter_type, &unnamed_params, &options)?;
-                        FilterType::Grep => {
-                            create_filter_with_options::<grep::GrepFilter>(&unnamed_params, &options)?
-                        }
-                        FilterType::HeadBytes => {
-                            create_filter_with_options::<head::HeadBytesFilter>(&unnamed_params, &options)?
-                        }
-                        FilterType::HeadLines => {
-                            create_filter_with_options::<head::HeadLinesFilter>(&unnamed_params, &options)?
-                        }
-                        FilterType::TailBytes => {
-                            create_filter_with_options::<tail::TailBytesFilter>(&unnamed_params, &options)?
-                        }
-                        FilterType::TailLines => {
-                            create_filter_with_options::<tail::TailLinesFilter>(&unnamed_params, &options)?
-                        }
-                        FilterType::SkipBytes => {
-                            create_filter_with_options::<skip::SkipBytesFilter>(&unnamed_params, &options)?
-                        }
-                        FilterType::SkipLines => {
-                            create_filter_with_options::<skip::SkipLinesFilter>(&unnamed_params, &options)?
-                        }
-                        FilterType::StripAnsi => {
-                            return Err(std::io::Error::new(
-                                std::io::ErrorKind::InvalidInput,
-                                "strip_ansi filter doesn't take parameters"
-                            ));
-                        }
-                    };
                     chain.add_plugin(plugin);
                     continue;
                 }
@@ -205,12 +177,23 @@ pub fn parse_filter_string(filter_str: &str) -> Result<FilterChain> {
 }
 
 // Helper function to create filter with proper option handling
-fn create_filter_with_options<T: FilterPlugin + Default>(
+fn create_filter_with_options(
+    filter_type: FilterType,
     unnamed_params: &[serde_json::Value],
     named_options: &HashMap<String, serde_json::Value>,
 ) -> Result<Box<dyn FilterPlugin>> {
-    let mut plugin = T::default();
+    // Get the default options for this filter type by creating a temporary instance
-    let option_defs = plugin.options();
+    // To do this, we need to create a default instance of the appropriate filter
+    let option_defs = match filter_type {
+        FilterType::Grep => grep::GrepFilter::new("".to_string())?.options(),
+        FilterType::HeadBytes => head::HeadBytesFilter::new(0).options(),
+        FilterType::HeadLines => head::HeadLinesFilter::new(0).options(),
+        FilterType::TailBytes => tail::TailBytesFilter::new(0).options(),
+        FilterType::TailLines => tail::TailLinesFilter::new(0).options(),
+        FilterType::SkipBytes => skip::SkipBytesFilter::new(0).options(),
+        FilterType::SkipLines => skip::SkipLinesFilter::new(0).options(),
+        FilterType::StripAnsi => strip_ansi::StripAnsiFilter::new().options(),
+    };
     
     let mut options = HashMap::new();
     
@@ -257,23 +240,95 @@ fn create_filter_with_options<T: FilterPlugin + Default>(
     }
     
     // Create the specific filter with the processed options
-    // This part needs to be implemented for each filter type
+    create_specific_filter(filter_type, &options)
-    // For now, we'll use a match on the type name
-    // Note: This is a placeholder - you'll need to implement proper constructors for each filter
-    Ok(create_specific_filter::<T>(&options)?)
 }
 
 // Helper to create specific filter instances based on options
-fn create_specific_filter<T: FilterPlugin + Default>(
+fn create_specific_filter(
+    filter_type: FilterType,
     options: &HashMap<String, serde_json::Value>,
 ) -> Result<Box<dyn FilterPlugin>> {
-    // This is a simplified implementation
+    match filter_type {
-    // In practice, you'd need to handle each filter type specifically
+        FilterType::Grep => {
-    let mut plugin = T::default();
+            let pattern = options.get("pattern")
-    
+                .and_then(|v| v.as_str())
-    // For now, just return the default plugin
+                .ok_or_else(|| std::io::Error::new(
-    // You'll need to implement proper initialization based on options
+                    std::io::ErrorKind::InvalidInput,
-    Ok(Box::new(plugin))
+                    "grep filter requires 'pattern' parameter"
+                ))?;
+            grep::GrepFilter::new(pattern.to_string()).map(|f| Box::new(f) as Box<dyn FilterPlugin>)
+        }
+        FilterType::HeadBytes => {
+            let count = options.get("count")
+                .and_then(|v| v.as_u64())
+                .map(|n| n as usize)
+                .ok_or_else(|| std::io::Error::new(
+                    std::io::ErrorKind::InvalidInput,
+                    "head_bytes filter requires 'count' parameter"
+                ))?;
+            Ok(Box::new(head::HeadBytesFilter::new(count)))
+        }
+        FilterType::HeadLines => {
+            let count = options.get("count")
+                .and_then(|v| v.as_u64())
+                .map(|n| n as usize)
+                .ok_or_else(|| std::io::Error::new(
+                    std::io::ErrorKind::InvalidInput,
+                    "head_lines filter requires 'count' parameter"
+                ))?;
+            Ok(Box::new(head::HeadLinesFilter::new(count)))
+        }
+        FilterType::TailBytes => {
+            let count = options.get("count")
+                .and_then(|v| v.as_u64())
+                .map(|n| n as usize)
+                .ok_or_else(|| std::io::Error::new(
+                    std::io::ErrorKind::InvalidInput,
+                    "tail_bytes filter requires 'count' parameter"
+                ))?;
+            Ok(Box::new(tail::TailBytesFilter::new(count)))
+        }
+        FilterType::TailLines => {
+            let count = options.get("count")
+                .and_then(|v| v.as_u64())
+                .map(|n| n as usize)
+                .ok_or_else(|| std::io::Error::new(
+                    std::io::ErrorKind::InvalidInput,
+                    "tail_lines filter requires 'count' parameter"
+                ))?;
+            Ok(Box::new(tail::TailLinesFilter::new(count)))
+        }
+        FilterType::SkipBytes => {
+            let count = options.get("count")
+                .and_then(|v| v.as_u64())
+                .map(|n| n as usize)
+                .ok_or_else(|| std::io::Error::new(
+                    std::io::ErrorKind::InvalidInput,
+                    "skip_bytes filter requires 'count' parameter"
+                ))?;
+            Ok(Box::new(skip::SkipBytesFilter::new(count)))
+        }
+        FilterType::SkipLines => {
+            let count = options.get("count")
+                .and_then(|v| v.as_u64())
+                .map(|n| n as usize)
+                .ok_or_else(|| std::io::Error::new(
+                    std::io::ErrorKind::InvalidInput,
+                    "skip_lines filter requires 'count' parameter"
+                ))?;
+            Ok(Box::new(skip::SkipLinesFilter::new(count)))
+        }
+        FilterType::StripAnsi => {
+            // StripAnsi doesn't take any parameters
+            if !options.is_empty() {
+                return Err(std::io::Error::new(
+                    std::io::ErrorKind::InvalidInput,
+                    "strip_ansi filter doesn't take parameters"
+                ));
+            }
+            Ok(Box::new(strip_ansi::StripAnsiFilter::new()))
+        }
+    }
 }
 
 // Helper function to parse option values