diff --git a/src/commands/findings.rs b/src/commands/findings.rs index 29bc786..871fcd4 100644 --- a/src/commands/findings.rs +++ b/src/commands/findings.rs @@ -1,7 +1,7 @@ //! `cora findings` subcommand — manage review findings stored in cora.db. use anyhow::Result; -use colored::Colorize; +use colored::{Color, Colorize}; use crate::engine::review_store::{self, FindingFilter, FindingStats, ReviewStore, Transition}; @@ -19,7 +19,11 @@ pub enum FindingsAction { all: bool, /// Filter by severity (info, minor, major, critical) - #[clap(long)] + #[clap( + long, + value_parser = ["info", "minor", "major", "critical"], + ignore_case = true + )] severity: Option, /// Filter by file path substring @@ -107,6 +111,16 @@ pub fn execute_findings(action: &FindingsAction) -> Result { } } +/// Colour for a stored severity (matched case-insensitively); `None` = dimmed. +fn severity_color(severity: &str) -> Option { + match severity.to_ascii_lowercase().as_str() { + "critical" => Some(Color::Red), + "major" => Some(Color::Yellow), + "minor" => Some(Color::Green), + _ => None, + } +} + fn list_findings(store: &ReviewStore<'_>, filter: &FindingFilter, json: bool) -> Result { let rows = store.list_findings(filter)?; @@ -126,10 +140,10 @@ fn list_findings(store: &ReviewStore<'_>, filter: &FindingFilter, json: bool) -> rows.len().to_string().bold() ); for r in &rows { - let sev = match r.severity.as_str() { - "CRITICAL" => r.severity.clone().red().to_string(), - "MAJOR" => r.severity.clone().yellow().to_string(), - "MINOR" => r.severity.clone().green().to_string(), + let sev = match severity_color(&r.severity) { + Some(Color::Red) => r.severity.clone().red().to_string(), + Some(Color::Yellow) => r.severity.clone().yellow().to_string(), + Some(Color::Green) => r.severity.clone().green().to_string(), _ => r.severity.clone().dimmed().to_string(), }; let status_tag = match r.status.as_str() { @@ -222,3 +236,43 @@ fn reopen(store: &ReviewStore<'_>, id: i64) -> Result { } } } + +#[cfg(test)] +mod tests { + use super::*; + use clap::Parser; + + #[derive(Parser, Debug)] + struct Harness { + #[clap(subcommand)] + action: FindingsAction, + } + + fn parse_severity(arg: &str) -> Result, clap::Error> { + let h = Harness::try_parse_from(["t", "list", "--severity", arg])?; + match h.action { + FindingsAction::List { severity, .. } => Ok(severity), + _ => unreachable!(), + } + } + + #[test] + fn severity_arg_accepts_any_case_and_rejects_unknown() { + for (arg, want) in [("major", "major"), ("MAJOR", "major"), ("Info", "info")] { + // clap keeps the typed case; the store lowercases it. + let got = parse_severity(arg).unwrap().unwrap(); + assert!(got.eq_ignore_ascii_case(want)); + } + assert!(parse_severity("bogus").is_err()); + } + + #[test] + fn severity_color_is_case_insensitive() { + assert_eq!(severity_color("critical"), Some(Color::Red)); + assert_eq!(severity_color("CRITICAL"), Some(Color::Red)); + assert_eq!(severity_color("major"), Some(Color::Yellow)); + assert_eq!(severity_color("Minor"), Some(Color::Green)); + assert_eq!(severity_color("info"), None); + assert_eq!(severity_color("weird"), None); + } +} diff --git a/src/engine/review_store.rs b/src/engine/review_store.rs index 03b275f..b81811e 100644 --- a/src/engine/review_store.rs +++ b/src/engine/review_store.rs @@ -60,7 +60,7 @@ pub struct ReviewRecord<'a> { pub struct FindingFilter { /// Include resolved/dismissed findings (default: open only). pub all: bool, - /// Exact severity match; compared upper-cased. + /// Exact severity match, case-insensitive (stored lowercase). pub severity: Option, /// Substring match on the file path. pub file: Option, @@ -361,7 +361,7 @@ impl<'a> ReviewStore<'a> { } if let Some(s) = &filter.severity { wheres.push("f.severity = ?"); - params.push(Box::new(s.to_uppercase())); + params.push(Box::new(s.to_lowercase())); } if let Some(f) = &filter.file { wheres.push("f.file_path LIKE ?"); @@ -716,11 +716,16 @@ mod tests { assert_eq!(rows.len(), 1); assert_eq!(rows[0].title, "one"); - // Legacy behaviour kept on purpose (refactor, not a fix): severities are - // stored lowercase but the filter upper-cases its argument, so - // `--severity` never matches. Tracked separately; this pins the status quo. + // Severities are stored lowercase; the filter is case-insensitive. + for arg in ["minor", "MINOR", "Minor"] { + let mut f = filter(true); + f.severity = Some(arg.into()); + assert_eq!(store.list_findings(&f).unwrap().len(), 2, "arg {arg}"); + } let mut f = filter(true); - f.severity = Some("minor".into()); + f.severity = Some("critical".into()); + assert_eq!(store.list_findings(&f).unwrap().len(), 1); + f.severity = Some("bogus".into()); assert_eq!(store.list_findings(&f).unwrap().len(), 0); let mut f = filter(true);