From b3445b3044a94b7fa2c5bdb6c751a4c02870bb12 Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Mon, 5 Jan 2026 03:51:43 -0500 Subject: [PATCH] refactor: clean up some query code (#1949) * tmp * intermediate work * delete some code * hook things up * use QueryOptions everywhere instead * comments * more cleanup * even more cleanup * fmt * update comment * some more tests while I'm at it * add todo for additional tests * more empty quotes * more empty quote testing * even more tests * driveby test for arg tests --- src/canvas/widgets/process_table.rs | 6 +- src/widgets/process_table.rs | 34 +- src/widgets/process_table/query.rs | 569 +++++++++++++++---- src/widgets/process_table/query/and.rs | 56 +- src/widgets/process_table/query/attribute.rs | 141 +++++ src/widgets/process_table/query/or.rs | 55 +- src/widgets/process_table/query/prefix.rs | 362 +++--------- tests/integration/arg_tests.rs | 10 + 8 files changed, 728 insertions(+), 505 deletions(-) create mode 100644 src/widgets/process_table/query/attribute.rs diff --git a/src/canvas/widgets/process_table.rs b/src/canvas/widgets/process_table.rs index 961df4a7..2652ea06 100644 --- a/src/canvas/widgets/process_table.rs +++ b/src/canvas/widgets/process_table.rs @@ -197,19 +197,19 @@ impl Painter { })]; // Text options shamelessly stolen from VS Code. - let case_style = if !proc_widget_state.proc_search.is_ignoring_case { + let case_style = if !proc_widget_state.proc_search.query_options.ignore_case { self.styles.selected_text_style } else { self.styles.text_style }; - let whole_word_style = if proc_widget_state.proc_search.is_searching_whole_word { + let whole_word_style = if proc_widget_state.proc_search.query_options.whole_word { self.styles.selected_text_style } else { self.styles.text_style }; - let regex_style = if proc_widget_state.proc_search.is_searching_with_regex { + let regex_style = if proc_widget_state.proc_search.query_options.use_regex { self.styles.selected_text_style } else { self.styles.text_style diff --git a/src/widgets/process_table.rs b/src/widgets/process_table.rs index 588f6f09..3919e1d8 100644 --- a/src/widgets/process_table.rs +++ b/src/widgets/process_table.rs @@ -14,6 +14,7 @@ use query::{ProcessQuery, parse_query}; use rustc_hash::{FxHashMap as HashMap, FxHashSet as HashSet}; use sort_table::SortTableColumn; +use crate::widgets::query::QueryOptions; use crate::{ app::{ AppConfigFields, AppSearchState, @@ -29,35 +30,23 @@ use crate::{ /// ProcessSearchState only deals with process' search's current settings and /// state. +#[derive(Default)] pub struct ProcessSearchState { pub search_state: AppSearchState, - pub is_ignoring_case: bool, - pub is_searching_whole_word: bool, - pub is_searching_with_regex: bool, -} - -impl Default for ProcessSearchState { - fn default() -> Self { - ProcessSearchState { - search_state: AppSearchState::default(), - is_ignoring_case: true, - is_searching_whole_word: false, - is_searching_with_regex: false, - } - } + pub query_options: QueryOptions, } impl ProcessSearchState { pub fn search_toggle_ignore_case(&mut self) { - self.is_ignoring_case = !self.is_ignoring_case; + self.query_options.ignore_case = !self.query_options.ignore_case; } pub fn search_toggle_whole_word(&mut self) { - self.is_searching_whole_word = !self.is_searching_whole_word; + self.query_options.whole_word = !self.query_options.whole_word; } pub fn search_toggle_regex(&mut self) { - self.is_searching_with_regex = !self.is_searching_with_regex; + self.query_options.use_regex = !self.query_options.use_regex; } } @@ -1089,6 +1078,9 @@ impl ProcWidgetState { &self.proc_search.search_state.current_search_query } + /// Update the current search query. + /// + /// TODO: Maybe debounce this. pub fn update_query(&mut self) { if self .proc_search @@ -1102,9 +1094,7 @@ impl ProcWidgetState { } else { match parse_query( &self.proc_search.search_state.current_search_query, - self.proc_search.is_searching_whole_word, - self.proc_search.is_ignoring_case, - self.proc_search.is_searching_with_regex, + &self.proc_search.query_options, ) { Ok(parsed_query) => { self.proc_search.search_state.query = Some(parsed_query); @@ -1153,9 +1143,7 @@ impl ProcWidgetState { #[cfg(test)] pub(crate) fn test_equality(&self, other: &Self) -> bool { self.mode == other.mode - && self.proc_search.is_ignoring_case == other.proc_search.is_ignoring_case - && self.proc_search.is_searching_whole_word == other.proc_search.is_searching_whole_word - && self.proc_search.is_searching_with_regex == other.proc_search.is_searching_with_regex + && self.proc_search.query_options == other.proc_search.query_options && self .table .columns diff --git a/src/widgets/process_table/query.rs b/src/widgets/process_table/query.rs index a9be65f4..85e33ece 100644 --- a/src/widgets/process_table/query.rs +++ b/src/widgets/process_table/query.rs @@ -2,23 +2,21 @@ //! //! Yes, this is a hand-rolled parser. I originally wrote this back in uni where writing //! a parser was basically a thing I did every year, and parsing crate options were not -//! as good as they are now. +//! as good as they are now. This will be rewritten as time goes on, though. mod and; +mod attribute; mod error; mod or; mod prefix; +use std::{collections::VecDeque, time::Duration}; + use and::And; +use attribute::ProcessAttribute; use error::{QueryError, QueryResult}; use or::Or; use prefix::Prefix; -use std::{ - collections::VecDeque, - fmt::{Debug, Formatter}, - time::Duration, -}; - use regex::Regex; use crate::{collection::processes::ProcessHarvest, multi_eq_ignore_ascii_case}; @@ -29,11 +27,62 @@ const COMPARISON_LIST: [&str; 3] = [">", "=", "<"]; /// A node type that can take a query and read it, advancing the current read state /// and returning an instance of the node. trait QueryProcessor { - fn process(query: &mut VecDeque) -> QueryResult + fn process(query: &mut VecDeque, regex_options: &QueryOptions) -> QueryResult where Self: Sized; } +/// Process a new regex given a `base` string and some settings. +/// +/// TODO: Push this into a struct so I don't have to throw the options around so much. +fn new_regex(base: &str, regex_options: &QueryOptions) -> QueryResult { + let QueryOptions { + whole_word: is_searching_whole_word, + ignore_case: is_ignoring_case, + use_regex: is_searching_with_regex, + } = regex_options; + let escaped_regex: String; // Needed for ownership reasons. + + let final_regex_string = &format!( + "{}{}{}{}", + if *is_searching_whole_word { "^" } else { "" }, + if *is_ignoring_case { "(?i)" } else { "" }, + if !(*is_searching_with_regex) { + escaped_regex = regex::escape(base); + &escaped_regex + } else { + base + }, + if *is_searching_whole_word { "$" } else { "" }, + ); + + Ok(Regex::new(final_regex_string)?) +} + +/// Options when creating a new query. +#[derive(PartialEq, Eq)] +pub struct QueryOptions { + /// Whether we only allow matches on the entire word. + pub whole_word: bool, + + /// Whether to ignore case-sensitivity when searching. On by default. + pub ignore_case: bool, + + /// Whether we should use regex syntax when searching. If not set, then it + /// should treat everything as a literal string. + pub use_regex: bool, +} + +impl Default for QueryOptions { + fn default() -> Self { + Self { + ignore_case: true, + whole_word: false, + use_regex: false, + } + } +} + /// In charge of parsing the given query, case-insensitive, possibly marked /// by a prefix. For example: /// @@ -54,16 +103,15 @@ trait QueryProcessor { /// adjacent non-prefixed or quoted elements after splitting to treat as process /// names. Furthermore, we want to support boolean joiners like AND and OR, and /// brackets. -pub(crate) fn parse_query( - search_query: &str, is_searching_whole_word: bool, is_ignoring_case: bool, - is_searching_with_regex: bool, -) -> QueryResult { - fn process_string_to_filter(query: &mut VecDeque) -> QueryResult { - let lhs = Or::process(query)?; +pub(crate) fn parse_query(search_query: &str, options: &QueryOptions) -> QueryResult { + fn process_string_to_filter( + query: &mut VecDeque, options: &QueryOptions, + ) -> QueryResult { + let lhs = Or::process(query, options)?; let mut list_of_ors = vec![lhs]; while query.front().is_some() { - list_of_ors.push(Or::process(query)?); + list_of_ors.push(Or::process(query, options)?); } Ok(ProcessQuery { query: list_of_ors }) @@ -72,7 +120,7 @@ pub(crate) fn parse_query( let mut split_query = VecDeque::new(); search_query.split_whitespace().for_each(|s| { - // From https://stackoverflow.com/a/56923739 in order to get a split, but include the parentheses + // From https://stackoverflow.com/a/56923739 get a split but include the parentheses let mut last = 0; for (index, matched) in s.match_indices(|x| DELIMITER_LIST.contains(&x)) { if last != index { @@ -86,37 +134,16 @@ pub(crate) fn parse_query( } }); - let mut process_filter = process_string_to_filter(&mut split_query)?; - process_filter.process_regexes( - is_searching_whole_word, - is_ignoring_case, - is_searching_with_regex, - )?; - - Ok(process_filter) + process_string_to_filter(&mut split_query, options) } +#[derive(Debug)] pub struct ProcessQuery { /// Remember, AND > OR, but AND must come after OR when we parse. query: Vec, } impl ProcessQuery { - fn process_regexes( - &mut self, is_searching_whole_word: bool, is_ignoring_case: bool, - is_searching_with_regex: bool, - ) -> QueryResult<()> { - for or in &mut self.query { - or.process_regexes( - is_searching_whole_word, - is_ignoring_case, - is_searching_with_regex, - )?; - } - - Ok(()) - } - pub(crate) fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { self.query .iter() @@ -124,22 +151,16 @@ impl ProcessQuery { } } -impl Debug for ProcessQuery { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - f.write_fmt(format_args!("{:?}", self.query)) - } -} - #[derive(Debug)] enum PrefixType { Pid, - PCpu, + CpuPercentage, MemBytes, - PMem, - Rps, - Wps, - TRead, - TWrite, + MemPercentage, + ReadPerSecond, + WritePerSecond, + TotalRead, + TotalWrite, Name, State, User, @@ -148,12 +169,11 @@ enum PrefixType { Nice, Priority, #[cfg(feature = "gpu")] - PGpu, + GpuPercentage, #[cfg(feature = "gpu")] - GMem, + GpuMemoryBytes, #[cfg(feature = "gpu")] - PGMem, - __Nonexhaustive, + GpuMemoryPercentage, } impl std::str::FromStr for PrefixType { @@ -167,19 +187,19 @@ impl std::str::FromStr for PrefixType { let mut result = Name; if multi_eq_ignore_ascii_case!(s, "cpu" | "cpu%") { - result = PCpu; + result = CpuPercentage; } else if multi_eq_ignore_ascii_case!(s, "mem" | "mem%") { - result = PMem; + result = MemPercentage; } else if multi_eq_ignore_ascii_case!(s, "memb") { result = MemBytes; } else if multi_eq_ignore_ascii_case!(s, "read" | "r/s" | "rps") { - result = Rps; + result = ReadPerSecond; } else if multi_eq_ignore_ascii_case!(s, "write" | "w/s" | "wps") { - result = Wps; + result = WritePerSecond; } else if multi_eq_ignore_ascii_case!(s, "tread" | "t.read") { - result = TRead; + result = TotalRead; } else if multi_eq_ignore_ascii_case!(s, "twrite" | "t.write") { - result = TWrite; + result = TotalWrite; } else if multi_eq_ignore_ascii_case!(s, "pid") { result = Pid; } else if multi_eq_ignore_ascii_case!(s, "state") { @@ -199,11 +219,11 @@ impl std::str::FromStr for PrefixType { #[cfg(feature = "gpu")] { if multi_eq_ignore_ascii_case!(s, "gmem") { - result = GMem; + result = GpuMemoryBytes; } else if multi_eq_ignore_ascii_case!(s, "gmem%") { - result = PGMem; + result = GpuMemoryPercentage; } else if multi_eq_ignore_ascii_case!(s, "gpu%") { - result = PGpu; + result = GpuPercentage; } } Ok(result) @@ -219,30 +239,49 @@ enum QueryComparison { GreaterOrEqual, } -#[derive(Debug)] -enum StringQuery { - Value(String), - Regex(Regex), -} - -#[derive(Debug)] -enum ComparableQuery { - Numerical(NumericalQuery), - Time(TimeQuery), -} - #[derive(Debug)] struct NumericalQuery { condition: QueryComparison, value: f64, } +impl NumericalQuery { + /// Compare `lhs` to the value in the query as `rhs`. + fn check>(&self, lhs: I) -> bool { + let lhs: f64 = lhs.into(); + let rhs: f64 = self.value; + + match self.condition { + QueryComparison::Equal => (lhs - rhs).abs() < f64::EPSILON, + QueryComparison::Less => lhs < rhs, + QueryComparison::Greater => lhs > rhs, + QueryComparison::LessOrEqual => lhs <= rhs, + QueryComparison::GreaterOrEqual => lhs >= rhs, + } + } +} + #[derive(Debug)] struct TimeQuery { condition: QueryComparison, duration: Duration, } +impl TimeQuery { + /// Compare `lhs` to the value in the query as `rhs`. + fn check(&self, lhs: Duration) -> bool { + let rhs = self.duration; + + match self.condition { + QueryComparison::Equal => lhs == rhs, + QueryComparison::Less => lhs < rhs, + QueryComparison::Greater => lhs > rhs, + QueryComparison::LessOrEqual => lhs <= rhs, + QueryComparison::GreaterOrEqual => lhs >= rhs, + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -254,9 +293,20 @@ mod tests { } } + fn parse_query_no_options(query: &str) -> QueryResult { + parse_query( + query, + &QueryOptions { + whole_word: false, + ignore_case: false, + use_regex: false, + }, + ) + } + #[test] fn basic_query() { - let query = parse_query("test", false, false, false).unwrap(); + let query = parse_query_no_options("test").unwrap(); let exact_match = simple_process("test"); let contains = simple_process("test string"); @@ -269,7 +319,7 @@ mod tests { #[test] fn basic_or_query() { - let query = parse_query("a or b", false, false, false).unwrap(); + let query = parse_query_no_options("a or b").unwrap(); let a = simple_process("a"); let b = simple_process("b"); @@ -282,7 +332,7 @@ mod tests { #[test] fn basic_and_query() { - let query = parse_query("a and b", false, false, false).unwrap(); + let query = parse_query_no_options("a and b").unwrap(); let a = simple_process("a"); let b = simple_process("b"); @@ -295,11 +345,26 @@ mod tests { assert!(query.check(&a_and_b, false)); } + #[test] + fn implied_and_query() { + let query = parse_query_no_options("a b c").unwrap(); + + let a = simple_process("a"); + let b = simple_process("b"); + let c = simple_process("c"); + let all = simple_process("a b c"); + + assert!(!query.check(&a, false)); + assert!(!query.check(&b, false)); + assert!(!query.check(&c, false)); + assert!(query.check(&all, false)); + } + /// Ensure that quoted keywords are treated as strings. In this case, rather than `"a" OR "b"`, it should be treated /// as the string `"a or b"`. #[test] fn quoted_query() { - let query = parse_query("a \"or\" b", false, false, false).unwrap(); + let query = parse_query_no_options("a \"or\" b").unwrap(); let a = simple_process("a"); let b = simple_process("b"); @@ -320,7 +385,7 @@ mod tests { /// as the string `"a or b"`. #[test] fn quoted_multi_word_query() { - let query = parse_query("\"a or b\"", false, false, false).unwrap(); + let query = parse_query_no_options("\"a or b\"").unwrap(); let a = simple_process("a"); let b = simple_process("b"); @@ -339,7 +404,7 @@ mod tests { #[test] fn basic_cpu_query() { - let query = parse_query("cpu > 50", false, false, false).unwrap(); + let query = parse_query_no_options("cpu > 50").unwrap(); let mut over = simple_process("a"); over.cpu_usage_percent = 60.0; @@ -357,7 +422,7 @@ mod tests { #[test] fn basic_mem_query() { - let query = parse_query("memb > 1 GiB", false, false, false).unwrap(); + let query = parse_query_no_options("memb > 1 GiB").unwrap(); let mut over = simple_process("a"); over.mem_usage = 2 * 1024 * 1024 * 1024; @@ -376,7 +441,7 @@ mod tests { /// This test sees if parentheses work. #[test] fn nested_query_1() { - let query = parse_query("(a or b) and (c or a)", false, false, false).unwrap(); + let query = parse_query_no_options("(a or b) and (c or a)").unwrap(); let a = simple_process("a"); let b = simple_process("b"); @@ -392,7 +457,7 @@ mod tests { /// This test sees if parentheses and mixed query types work. #[test] fn nested_query_2() { - let query = parse_query("(cpu > 10 or cpu < 5) and (c or a)", false, false, false).unwrap(); + let query = parse_query_no_options("(cpu > 10 or cpu < 5) and (c or a)").unwrap(); let mut a_valid_1 = simple_process("a"); a_valid_1.cpu_usage_percent = 100.0; @@ -424,13 +489,8 @@ mod tests { /// This test adds a further layer of nesting to consider. #[test] fn nested_query_3() { - let query = parse_query( - "((cpu > 10 or cpu < 5) or d) and ((c or a) or d)", - false, - false, - false, - ) - .unwrap(); + let query = + parse_query_no_options("((cpu > 10 or cpu < 5) or d) and ((c or a) or d)").unwrap(); let mut a_valid_1 = simple_process("a"); a_valid_1.cpu_usage_percent = 100.0; @@ -459,18 +519,38 @@ mod tests { assert!(!query.check(&b, false)); } - /// Test an ambiguous or. #[test] - fn ambiguous_or() {} + fn ambiguous_precedence_1() { + let query = parse_query_no_options("a and b or c").unwrap(); + + let a = simple_process("a"); + let b = simple_process("b"); + let c = simple_process("c"); + + assert!(!query.check(&a, false)); + assert!(!query.check(&b, false)); + assert!(query.check(&c, false)); + } + + #[test] + fn ambiguous_precedence_2() { + let query = parse_query_no_options("a or b and c").unwrap(); + + let a = simple_process("a"); + let b = simple_process("b"); + let c = simple_process("c"); + + assert!(query.check(&a, false)); + assert!(!query.check(&b, false)); + assert!(!query.check(&c, false)); + } /// Test if a complicated query even parses. #[test] fn parse_complicated_query() { - parse_query( + parse_query_no_options( "cpu > 10.5 AND (memb = 1 MiB || state = sleeping) and (a or b) and (read >= 0 or write >= 0)", - false, - false, - false, + ) .unwrap(); } @@ -478,27 +558,308 @@ mod tests { /// Test empty quotes works. #[test] fn parse_empty_quotes() { - parse_query("\"\"", false, false, false).unwrap(); + parse_query_no_options("\"\"").unwrap(); + parse_query_no_options("\"\"\"\"").unwrap(); + parse_query_no_options("\"\" OR \"\"").unwrap(); + } + + #[test] + fn search_empty_quotes() { + let a = parse_query_no_options("\"\"").unwrap(); + let b = parse_query_no_options("\"\" OR test").unwrap(); + + let process = simple_process("test"); + + assert!(a.check(&process, false)); + assert!(b.check(&process, false)); } /// Test unfinished quotes error. #[test] fn parse_unfinished_quotes() { - parse_query("\"", false, false, false).unwrap_err(); + parse_query_no_options("\"").unwrap_err(); + parse_query_no_options("\"asdf").unwrap_err(); + parse_query_no_options("asdf\"").unwrap_err(); } /// Test a fix for a bug with closing quotations. The problem seems to arise from quotes being used as an argument /// to a prefix... but this should probably be valid. #[test] fn parse_nested_closing_quotes() { - parse_query("state = \"test\"", false, false, false).unwrap(); - parse_query("state = \"2 words\"", false, false, false).unwrap(); - parse_query("(memb = 1 MiB || state = \"test\")", false, false, false).unwrap(); - parse_query("(memb = 1 MiB || state = \"2 words\")", false, false, false).unwrap(); + parse_query_no_options("state = \"test\"").unwrap(); + parse_query_no_options("state = \"2 words\"").unwrap(); + parse_query_no_options("(memb = 1 MiB || state = \"test\")").unwrap(); + parse_query_no_options("(memb = 1 MiB || state = \"2 words\")").unwrap(); } // TODO: Add this after fixed. // /// Test if units can ignore spaces from their preceding value. // #[test] // fn units_with_and_without_spaces() {} + + #[test] + fn invalid_uncompleted_queries_1() { + parse_query_no_options("state =").unwrap_err(); + parse_query_no_options("a or").unwrap_err(); + parse_query_no_options("a >").unwrap_err(); + } + + #[track_caller] + fn invalid_lhs_rhs(op: &str) { + parse_query_no_options(&format!("a {op} asdf = 100")).unwrap_err(); + parse_query_no_options(&format!("asdf = 100 {op} b")).unwrap_err(); + parse_query_no_options(&format!("a {op} asdf = 100 {op} b")).unwrap_err(); + + parse_query_no_options(&format!("asdf = 100 {op} bsdf = \"")).unwrap_err(); + parse_query_no_options(&format!("a {op} bsdf = \"")).unwrap_err(); + } + + #[test] + fn invalid_or() { + invalid_lhs_rhs("OR"); + invalid_lhs_rhs("||"); + } + + #[test] + fn invalid_and() { + invalid_lhs_rhs("AND"); + invalid_lhs_rhs("&&"); + invalid_lhs_rhs(""); + } + + // /// Test keywords. + // /// + // /// TODO: Should these be invalid...? + // #[test] + // fn invalid_query_x() { + // parse_query_no_options("or").unwrap_err(); + // parse_query_no_options("and").unwrap_err(); + // parse_query_no_options("a or >").unwrap_err(); + // parse_query_no_options("a and >").unwrap_err(); + // } + + #[test] + fn test_command_check() { + let query = parse_query_no_options("command").unwrap(); + + let mut process_a = simple_process("test"); + process_a.command = "command".into(); + + let mut process_b = simple_process("test"); + process_b.command = "no".into(); + + assert!(query.check(&process_a, true)); + assert!(!query.check(&process_b, true)); + } + + #[test] + fn test_non_ascii_only_1() { + let query = parse_query_no_options("食").unwrap(); + + let process_a = simple_process("施氏食獅史"); + let process_b = simple_process("沒有"); + + assert!(query.check(&process_a, false)); + assert!(!query.check(&process_b, false)); + } + + #[test] + fn test_non_ascii_only_2() { + let query = parse_query_no_options("परीक्षा").unwrap(); + + let process_a = simple_process("परीक्षा"); + let process_b = simple_process("उपलब्ध नहीं है"); + + assert!(query.check(&process_a, false)); + assert!(!query.check(&process_b, false)); + } + + #[test] + fn test_non_ascii_only_3() { + let query = parse_query_no_options("🇨🇦").unwrap(); + + let process_a = simple_process("🇨🇦"); + let process_b = simple_process("❤️🇨🇦❤️"); + let process_c = simple_process("❤️"); + + assert!(query.check(&process_a, false)); + assert!(query.check(&process_b, false)); + assert!(!query.check(&process_c, false)); + } + + #[test] + fn test_non_ascii_only_4() { + let query = parse_query_no_options("獅 or 狮").unwrap(); + + let process_a = simple_process("施氏食獅史"); + let process_b = simple_process("施氏食狮史"); + let process_c = simple_process("沒有"); + + assert!(query.check(&process_a, false)); + assert!(query.check(&process_b, false)); + assert!(!query.check(&process_c, false)); + } + + #[test] + fn test_invalid_non_ascii() { + parse_query_no_options("cpu = 食").unwrap_err(); + } + + #[test] + fn test_mixed_unicode() { + let query = parse_query_no_options("食 or test").unwrap(); + + let process_a = simple_process("施氏食獅史"); + let process_b = simple_process("test"); + let process_c = simple_process("施氏食獅史test"); + let process_d = simple_process("沒有"); + let process_e = simple_process("nope"); + + assert!(query.check(&process_a, false)); + assert!(query.check(&process_b, false)); + assert!(query.check(&process_c, false)); + assert!(!query.check(&process_d, false)); + assert!(!query.check(&process_e, false)); + } + + #[test] + fn test_regex_1() { + let query = parse_query( + "(a|b)", + &QueryOptions { + whole_word: false, + ignore_case: true, + use_regex: true, + }, + ) + .unwrap(); + + let process_a = simple_process("abc"); + let process_b = simple_process("test"); + + assert!(query.check(&process_a, false)); + assert!(!query.check(&process_b, false)); + } + + #[test] + fn test_regex_2() { + let query = parse_query( + "^a.*z$", + &QueryOptions { + whole_word: false, + ignore_case: true, + use_regex: true, + }, + ) + .unwrap(); + + let process_a = simple_process("atoz"); + let process_b = simple_process("atob"); + let process_c = simple_process("ytoz"); + let process_d = simple_process("atozoops"); + + assert!(query.check(&process_a, false)); + assert!(!query.check(&process_b, false)); + assert!(!query.check(&process_c, false)); + assert!(!query.check(&process_d, false)); + } + + #[test] + fn test_whole_word_1() { + let query = parse_query( + "test", + &QueryOptions { + whole_word: true, + ignore_case: true, + use_regex: false, + }, + ) + .unwrap(); + + let process_a = simple_process("test"); + let process_b = simple_process("testa"); + let process_c = simple_process("atest"); + + assert!(query.check(&process_a, false)); + assert!(!query.check(&process_b, false)); + assert!(!query.check(&process_c, false)); + } + + #[test] + fn test_case_sensitive_1() { + let query = parse_query( + "tEsT", + &QueryOptions { + whole_word: false, + ignore_case: false, + use_regex: false, + }, + ) + .unwrap(); + + let process_a = simple_process("tEsT"); + let process_b = simple_process("tEsT a"); + let process_c = simple_process("a tEsT"); + + assert!(query.check(&process_a, false)); + assert!(query.check(&process_b, false)); + assert!(query.check(&process_c, false)); + + let process_d = simple_process("test"); + let process_e = simple_process("test a"); + let process_f = simple_process("a test"); + + assert!(!query.check(&process_d, false)); + assert!(!query.check(&process_e, false)); + assert!(!query.check(&process_f, false)); + } + + #[cfg(feature = "gpu")] + #[test] + fn test_gpu_queries() { + let mem = parse_query_no_options("gmem > 50 b").unwrap(); + let mem_percent = parse_query_no_options("gmem% = 50").unwrap(); + let use_percent = parse_query_no_options("gpu% = 50").unwrap(); + + let mut process_a = simple_process("test"); + process_a.gpu_mem = 100; + process_a.gpu_mem_percent = 50.0; + process_a.gpu_util = 50; + + assert!(mem.check(&process_a, false)); + assert!(mem_percent.check(&process_a, false)); + assert!(use_percent.check(&process_a, false)); + + let mut process_b = simple_process("test"); + process_b.gpu_mem = 0; + process_b.gpu_mem_percent = 10.0; + process_b.gpu_util = 10; + + assert!(!mem.check(&process_b, false)); + assert!(!mem_percent.check(&process_b, false)); + assert!(!use_percent.check(&process_b, false)); + } + + /// Test GPU queries that involve invalid string comparisons. + #[cfg(feature = "gpu")] + #[test] + fn test_invalid_gpu_queries() { + parse_query_no_options("gmem = \"what\"").unwrap_err(); + parse_query_no_options("gmem% = \"the\"").unwrap_err(); + parse_query_no_options("gpu% = \"heck\"").unwrap_err(); + } + + // TODO: Test all attribute keywords (e.g. cpu, mem, etc.) + // #[test] + // fn test_all_attribute_keywords() {} + + // TODO: Support 'bytes' or similar + // #[test] + // fn test_bytes_keyword() { + // let mem = parse_query_no_options("mem > 50 bytes").unwrap(); + // let mut process_a = simple_process("test"); + // process_a.mem_usage = 100; + + // assert!(mem.check(&process_a, false)); + // } } diff --git a/src/widgets/process_table/query/and.rs b/src/widgets/process_table/query/and.rs index c773f944..6cd405fa 100644 --- a/src/widgets/process_table/query/and.rs +++ b/src/widgets/process_table/query/and.rs @@ -1,39 +1,22 @@ -use std::fmt::{Debug, Formatter}; +use std::collections::VecDeque; use crate::{ collection::processes::ProcessHarvest, - widgets::query::{COMPARISON_LIST, Or, Prefix, QueryProcessor, QueryResult, error::QueryError}, + widgets::query::{ + COMPARISON_LIST, Or, Prefix, QueryOptions, QueryProcessor, QueryResult, error::QueryError, + }, }; /// A node where both the left hand side or the right hand side are considered. /// Note that the right hand side is optional, as that's how I implemented it a long time ago. -#[derive(Default)] +#[derive(Debug)] pub(super) struct And { pub(super) lhs: Prefix, + // TODO: Maybe don't need to box rhs? pub(super) rhs: Option>, } impl And { - pub(super) fn process_regexes( - &mut self, is_searching_whole_word: bool, is_ignoring_case: bool, - is_searching_with_regex: bool, - ) -> QueryResult<()> { - self.lhs.process_regexes( - is_searching_whole_word, - is_ignoring_case, - is_searching_with_regex, - )?; - if let Some(rhs) = &mut self.rhs { - rhs.process_regexes( - is_searching_whole_word, - is_ignoring_case, - is_searching_with_regex, - )?; - } - - Ok(()) - } - pub(super) fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { if let Some(rhs) = &self.rhs { self.lhs.check(process, is_using_command) && rhs.check(process, is_using_command) @@ -43,23 +26,14 @@ impl And { } } -impl Debug for And { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match &self.rhs { - Some(rhs) => f.write_fmt(format_args!("({:?} AND {:?})", self.lhs, rhs)), - None => f.write_fmt(format_args!("{:?}", self.lhs)), - } - } -} - impl QueryProcessor for And { - fn process(query: &mut std::collections::VecDeque) -> QueryResult + fn process(query: &mut VecDeque, options: &QueryOptions) -> QueryResult where Self: Sized, { const AND_LIST: [&str; 2] = ["and", "&&"]; - let mut lhs = Prefix::process(query)?; + let mut lhs = Prefix::process(query, options)?; let mut rhs: Option> = None; while let Some(queue_top) = query.front() { @@ -67,19 +41,15 @@ impl QueryProcessor for And { if AND_LIST.contains(¤t_lowercase.as_str()) { query.pop_front(); - rhs = Some(Box::new(Prefix::process(query)?)); + rhs = Some(Box::new(Prefix::process(query, options)?)); if let Some(next_queue_top) = query.front() { if AND_LIST.contains(&next_queue_top.to_lowercase().as_str()) { // Must merge LHS and RHS - lhs = Prefix { - or: Some(Box::new(Or { - lhs: And { lhs, rhs }, - rhs: None, - })), - regex_prefix: None, - compare_prefix: None, - }; + lhs = Prefix::Or(Box::new(Or { + lhs: And { lhs, rhs }, + rhs: None, + })); rhs = None; } else { break; diff --git a/src/widgets/process_table/query/attribute.rs b/src/widgets/process_table/query/attribute.rs new file mode 100644 index 00000000..0b00dcc1 --- /dev/null +++ b/src/widgets/process_table/query/attribute.rs @@ -0,0 +1,141 @@ +//! Code related to attributes, which should be "searchable" leaf nodes. + +use regex::Regex; + +use crate::{ + collection::processes::ProcessHarvest, + widgets::query::{ + NumericalQuery, PrefixType, QueryOptions, TimeQuery, + error::{QueryError, QueryResult}, + new_regex, + }, +}; + +/// An attribute (leaf node) for a process. +#[derive(Debug)] +pub(super) enum ProcessAttribute { + /// This is a bit of a hack to allow for "empty" attributes. We can fix it properly, + /// but it would potentially require handling "empty" queries better. Currently, we just + /// treat it as a leaf node that always succeeds on matches. + Empty, + Pid(Regex), + CpuPercentage(NumericalQuery), + MemBytes(NumericalQuery), + MemPercentage(NumericalQuery), + ReadPerSecond(NumericalQuery), + WritePerSecond(NumericalQuery), + TotalRead(NumericalQuery), + TotalWrite(NumericalQuery), + /// Note this is an "untagged" attribute (e.g. "btm", "firefox"). + Name(Regex), + State(Regex), + User(Regex), + Time(TimeQuery), + #[cfg(unix)] + Nice(NumericalQuery), + Priority(NumericalQuery), + #[cfg(feature = "gpu")] + GpuPercentage(NumericalQuery), + #[cfg(feature = "gpu")] + GpuMemoryPercentage(NumericalQuery), + #[cfg(feature = "gpu")] + GpuMemoryBytes(NumericalQuery), +} + +impl ProcessAttribute { + pub(super) fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { + match self { + ProcessAttribute::Empty => true, + ProcessAttribute::Pid(re) => re.is_match(process.pid.to_string().as_str()), + ProcessAttribute::CpuPercentage(cmp) => cmp.check(process.cpu_usage_percent), + ProcessAttribute::MemBytes(cmp) => cmp.check(process.mem_usage as f64), + ProcessAttribute::MemPercentage(cmp) => cmp.check(process.mem_usage_percent), + ProcessAttribute::ReadPerSecond(cmp) => cmp.check(process.read_per_sec as f64), + ProcessAttribute::WritePerSecond(cmp) => cmp.check(process.write_per_sec as f64), + ProcessAttribute::TotalRead(cmp) => cmp.check(process.total_read as f64), + ProcessAttribute::TotalWrite(cmp) => cmp.check(process.total_write as f64), + ProcessAttribute::Name(re) => re.is_match(if is_using_command { + process.command.as_str() + } else { + process.name.as_str() + }), + ProcessAttribute::State(re) => re.is_match(process.process_state.0), + ProcessAttribute::User(re) => match process.user.as_ref() { + Some(user) => re.is_match(user), + None => re.is_match("N/A"), + }, + ProcessAttribute::Time(time) => time.check(process.time), + // TODO: It's a bit silly for some of these, like nice/priority, where it's casted to an f64. + #[cfg(unix)] + ProcessAttribute::Nice(cmp) => cmp.check(process.nice as f64), + ProcessAttribute::Priority(cmp) => cmp.check(process.priority as f64), + #[cfg(feature = "gpu")] + ProcessAttribute::GpuPercentage(cmp) => cmp.check(process.gpu_util as f64), + #[cfg(feature = "gpu")] + ProcessAttribute::GpuMemoryPercentage(cmp) => cmp.check(process.gpu_mem_percent as f64), + #[cfg(feature = "gpu")] + ProcessAttribute::GpuMemoryBytes(cmp) => cmp.check(process.gpu_mem as f64), + } + } +} + +/// Given a string prefix type, obtain the appropriate [`ProcessAttribute`]. +pub(super) fn new_string_attribute( + prefix_type: PrefixType, base: &str, regex_options: &QueryOptions, +) -> QueryResult { + match prefix_type { + PrefixType::Pid | PrefixType::Name | PrefixType::State | PrefixType::User => { + let re = new_regex(base, regex_options)?; + + match prefix_type { + PrefixType::Pid => Ok(ProcessAttribute::Pid(re)), + PrefixType::Name => Ok(ProcessAttribute::Name(re)), + PrefixType::State => Ok(ProcessAttribute::State(re)), + PrefixType::User => Ok(ProcessAttribute::User(re)), + _ => unreachable!(), + } + } + _ => Err(QueryError::new(format!( + "process attribute type {prefix_type:?} is not a supported string attribute" + ))), + } +} + +/// Given a time prefix type, obtain the appropriate [`ProcessAttribute`]. +pub(super) fn new_time_attribute( + prefix_type: PrefixType, query: TimeQuery, +) -> QueryResult { + match prefix_type { + PrefixType::Time => Ok(ProcessAttribute::Time(query)), + _ => Err(QueryError::new(format!( + "process attribute type {prefix_type:?} is not a supported time attribute" + ))), + } +} + +/// Given a numerical prefix type, obtain the appropriate [`ProcessAttribute`]. +pub(super) fn new_numerical_attribute( + prefix_type: PrefixType, query: NumericalQuery, +) -> QueryResult { + match prefix_type { + PrefixType::CpuPercentage => Ok(ProcessAttribute::CpuPercentage(query)), + PrefixType::MemBytes => Ok(ProcessAttribute::MemBytes(query)), + PrefixType::MemPercentage => Ok(ProcessAttribute::MemPercentage(query)), + PrefixType::ReadPerSecond => Ok(ProcessAttribute::ReadPerSecond(query)), + PrefixType::WritePerSecond => Ok(ProcessAttribute::WritePerSecond(query)), + PrefixType::TotalRead => Ok(ProcessAttribute::TotalRead(query)), + PrefixType::TotalWrite => Ok(ProcessAttribute::TotalWrite(query)), + #[cfg(unix)] + PrefixType::Nice => Ok(ProcessAttribute::Nice(query)), + PrefixType::Priority => Ok(ProcessAttribute::Priority(query)), + #[cfg(feature = "gpu")] + PrefixType::GpuPercentage => Ok(ProcessAttribute::GpuPercentage(query)), + #[cfg(feature = "gpu")] + PrefixType::GpuMemoryBytes => Ok(ProcessAttribute::GpuMemoryBytes(query)), + #[cfg(feature = "gpu")] + PrefixType::GpuMemoryPercentage => Ok(ProcessAttribute::GpuMemoryPercentage(query)), + _ => Err(QueryError::new(format!( + "process attribute type {prefix_type:?} is not a supported numerical attribute" + ))), + } +} diff --git a/src/widgets/process_table/query/or.rs b/src/widgets/process_table/query/or.rs index d31b939d..4589b3a6 100644 --- a/src/widgets/process_table/query/or.rs +++ b/src/widgets/process_table/query/or.rs @@ -1,44 +1,22 @@ -use std::{ - collections::VecDeque, - fmt::{Debug, Formatter}, -}; +use std::collections::VecDeque; use crate::{ collection::processes::ProcessHarvest, widgets::query::{ - And, COMPARISON_LIST, Prefix, QueryProcessor, QueryResult, error::QueryError, + And, COMPARISON_LIST, Prefix, QueryOptions, QueryProcessor, QueryResult, error::QueryError, }, }; -/// A node where either the left hand side or the right hand side are considered. -/// Note that the right hand side is optional, as that's how I implemented it a long time ago. -#[derive(Default)] +/// A node where either the left-hand side or the right-hand side are considered. +/// Note that the right-hand side is optional, as that's how I implemented it a long time ago. +#[derive(Debug)] pub(super) struct Or { pub(super) lhs: And, + // TODO: Maybe don't need to box rhs? pub(super) rhs: Option>, } impl Or { - pub(super) fn process_regexes( - &mut self, is_searching_whole_word: bool, is_ignoring_case: bool, - is_searching_with_regex: bool, - ) -> QueryResult<()> { - self.lhs.process_regexes( - is_searching_whole_word, - is_ignoring_case, - is_searching_with_regex, - )?; - if let Some(rhs) = &mut self.rhs { - rhs.process_regexes( - is_searching_whole_word, - is_ignoring_case, - is_searching_with_regex, - )?; - } - - Ok(()) - } - pub(super) fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { if let Some(rhs) = &self.rhs { self.lhs.check(process, is_using_command) || rhs.check(process, is_using_command) @@ -48,40 +26,27 @@ impl Or { } } -impl Debug for Or { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match &self.rhs { - Some(rhs) => f.write_fmt(format_args!("({:?} OR {:?})", self.lhs, rhs)), - None => f.write_fmt(format_args!("{:?}", self.lhs)), - } - } -} - impl QueryProcessor for Or { - fn process(query: &mut VecDeque) -> QueryResult + fn process(query: &mut VecDeque, options: &QueryOptions) -> QueryResult where Self: Sized, { const OR_LIST: [&str; 2] = ["or", "||"]; - let mut lhs = And::process(query)?; + let mut lhs = And::process(query, options)?; let mut rhs: Option> = None; while let Some(queue_top) = query.front() { let current_lowercase = queue_top.to_lowercase(); if OR_LIST.contains(¤t_lowercase.as_str()) { query.pop_front(); - rhs = Some(Box::new(And::process(query)?)); + rhs = Some(Box::new(And::process(query, options)?)); if let Some(queue_next) = query.front() { if OR_LIST.contains(&queue_next.to_lowercase().as_str()) { // Must merge LHS and RHS lhs = And { - lhs: Prefix { - or: Some(Box::new(Or { lhs, rhs })), - regex_prefix: None, - compare_prefix: None, - }, + lhs: Prefix::Or(Box::new(Or { lhs, rhs })), rhs: None, }; rhs = None; diff --git a/src/widgets/process_table/query/prefix.rs b/src/widgets/process_table/query/prefix.rs index 0c3f1def..149e9f8f 100644 --- a/src/widgets/process_table/query/prefix.rs +++ b/src/widgets/process_table/query/prefix.rs @@ -1,18 +1,15 @@ -use std::{ - collections::VecDeque, - fmt::{Debug, Formatter}, - time::Duration, -}; +use std::{collections::VecDeque, fmt::Debug}; use humantime::parse_duration; -use regex::Regex; use crate::{ collection::processes::ProcessHarvest, utils::data_units::*, widgets::query::{ - And, ComparableQuery, NumericalQuery, Or, PrefixType, QueryComparison, QueryProcessor, - QueryResult, StringQuery, TimeQuery, error::QueryError, + And, NumericalQuery, Or, PrefixType, ProcessAttribute, QueryComparison, QueryOptions, + QueryProcessor, QueryResult, TimeQuery, + attribute::{new_numerical_attribute, new_string_attribute, new_time_attribute}, + error::QueryError, }, }; @@ -54,188 +51,28 @@ fn process_prefix_units(query: &mut VecDeque, value: &mut f64) { } } -/// Either contains a further `Or` recursively, or a "prefix" which is a leaf that can be searched. +/// Either contains a further `Or` recursively, or an attribute that can be queried, possibly as +/// part of a larger query. /// -// TODO: Represent this using an enum instead or something... -#[derive(Default)] -pub(super) struct Prefix { - pub(super) or: Option>, - pub(super) regex_prefix: Option<(PrefixType, StringQuery)>, - pub(super) compare_prefix: Option<(PrefixType, ComparableQuery)>, +/// In theory, this can be made generic to work on all table types, though for now, it's +/// hardcoded for processes. +#[derive(Debug)] +pub(super) enum Prefix { + Or(Box), + Attribute(ProcessAttribute), } impl Prefix { - pub(super) fn process_regexes( - &mut self, is_searching_whole_word: bool, is_ignoring_case: bool, - is_searching_with_regex: bool, - ) -> QueryResult<()> { - if let Some(or) = &mut self.or { - return or.process_regexes( - is_searching_whole_word, - is_ignoring_case, - is_searching_with_regex, - ); - } else if let Some(( - PrefixType::Pid | PrefixType::Name | PrefixType::State | PrefixType::User, - StringQuery::Value(regex_string), - )) = &mut self.regex_prefix - { - let escaped_regex: String; - let final_regex_string = &format!( - "{}{}{}{}", - if is_searching_whole_word { "^" } else { "" }, - if is_ignoring_case { "(?i)" } else { "" }, - if !is_searching_with_regex { - escaped_regex = regex::escape(regex_string); - &escaped_regex - } else { - regex_string - }, - if is_searching_whole_word { "$" } else { "" }, - ); - - let taken_pwc = self.regex_prefix.take(); - if let Some((taken_pt, _)) = taken_pwc { - self.regex_prefix = Some(( - taken_pt, - StringQuery::Regex(Regex::new(final_regex_string)?), - )); - } - } - - Ok(()) - } - pub(super) fn check(&self, process: &ProcessHarvest, is_using_command: bool) -> bool { - fn matches_condition, J: Into>( - condition: &QueryComparison, lhs: I, rhs: J, - ) -> bool { - let lhs: f64 = lhs.into(); - let rhs: f64 = rhs.into(); - - match condition { - QueryComparison::Equal => (lhs - rhs).abs() < f64::EPSILON, - QueryComparison::Less => lhs < rhs, - QueryComparison::Greater => lhs > rhs, - QueryComparison::LessOrEqual => lhs <= rhs, - QueryComparison::GreaterOrEqual => lhs >= rhs, - } - } - - fn matches_duration(condition: &QueryComparison, lhs: Duration, rhs: Duration) -> bool { - match condition { - QueryComparison::Equal => lhs == rhs, - QueryComparison::Less => lhs < rhs, - QueryComparison::Greater => lhs > rhs, - QueryComparison::LessOrEqual => lhs <= rhs, - QueryComparison::GreaterOrEqual => lhs >= rhs, - } - } - - if let Some(and) = &self.or { - and.check(process, is_using_command) - } else if let Some((prefix_type, query_content)) = &self.regex_prefix { - if let StringQuery::Regex(r) = query_content { - match prefix_type { - PrefixType::Name => r.is_match(if is_using_command { - process.command.as_str() - } else { - process.name.as_str() - }), - PrefixType::Pid => r.is_match(process.pid.to_string().as_str()), - PrefixType::State => r.is_match(process.process_state.0), - PrefixType::User => match process.user.as_ref() { - Some(user) => r.is_match(user), - None => r.is_match("N/A"), - }, - _ => true, // TODO: Change prefix types to be tied to the query type so we don't have the wildcard. - } - } else { - true - } - } else if let Some((prefix_type, comparable_query)) = &self.compare_prefix { - match comparable_query { - ComparableQuery::Numerical(numerical_query) => match prefix_type { - PrefixType::PCpu => matches_condition( - &numerical_query.condition, - process.cpu_usage_percent, - numerical_query.value, - ), - PrefixType::PMem => matches_condition( - &numerical_query.condition, - process.mem_usage_percent, - numerical_query.value, - ), - PrefixType::MemBytes => matches_condition( - &numerical_query.condition, - process.mem_usage as f64, - numerical_query.value, - ), - PrefixType::Rps => matches_condition( - &numerical_query.condition, - process.read_per_sec as f64, - numerical_query.value, - ), - PrefixType::Wps => matches_condition( - &numerical_query.condition, - process.write_per_sec as f64, - numerical_query.value, - ), - PrefixType::TRead => matches_condition( - &numerical_query.condition, - process.total_read as f64, - numerical_query.value, - ), - PrefixType::TWrite => matches_condition( - &numerical_query.condition, - process.total_write as f64, - numerical_query.value, - ), - #[cfg(feature = "gpu")] - PrefixType::PGpu => matches_condition( - &numerical_query.condition, - process.gpu_util, - numerical_query.value, - ), - #[cfg(feature = "gpu")] - PrefixType::GMem => matches_condition( - &numerical_query.condition, - process.gpu_mem as f64, - numerical_query.value, - ), - #[cfg(feature = "gpu")] - PrefixType::PGMem => matches_condition( - &numerical_query.condition, - process.gpu_mem_percent, - numerical_query.value, - ), - #[cfg(unix)] - PrefixType::Nice => matches_condition( - &numerical_query.condition, - process.nice, - numerical_query.value, - ), - PrefixType::Priority => matches_condition( - &numerical_query.condition, - process.priority, - numerical_query.value, - ), - _ => true, - }, - ComparableQuery::Time(time_query) => match prefix_type { - PrefixType::Time => { - matches_duration(&time_query.condition, process.time, time_query.duration) - } - _ => true, - }, - } - } else { - // Somehow we have an empty condition... oh well. Return true. - true + match self { + Prefix::Or(or) => or.check(process, is_using_command), + Prefix::Attribute(attribute) => attribute.check(process, is_using_command), } } - fn process_in_quotes(query: &mut VecDeque) -> QueryResult { + fn process_in_quotes( + query: &mut VecDeque, options: &QueryOptions, + ) -> QueryResult { if let Some(queue_top) = query.pop_front() { if queue_top == "\"" { // This means we hit something like "". Return an empty prefix, and to deal @@ -243,11 +80,7 @@ impl Prefix { // stack. Ugly fix but whatever. query.push_front("\"".to_string()); - Ok(Prefix { - or: None, - regex_prefix: Some((PrefixType::Name, StringQuery::Value(String::default()))), - compare_prefix: None, - }) + Ok(Prefix::Attribute(ProcessAttribute::Empty)) } else { let mut intern_string = vec![queue_top]; @@ -263,11 +96,11 @@ impl Prefix { let quoted_string = intern_string.join(" "); - Ok(Prefix { - or: None, - regex_prefix: Some((PrefixType::Name, StringQuery::Value(quoted_string))), - compare_prefix: None, - }) + Ok(Prefix::Attribute(new_string_attribute( + PrefixType::Name, + "ed_string, + options, + )?)) } } else { // Uh oh, there's nothing left in the stack, but we're inside quotes! @@ -276,22 +109,8 @@ impl Prefix { } } -impl Debug for Prefix { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - if let Some(or) = &self.or { - f.write_fmt(format_args!("{or:?}")) - } else if let Some(regex_prefix) = &self.regex_prefix { - f.write_fmt(format_args!("{regex_prefix:?}")) - } else if let Some(compare_prefix) = &self.compare_prefix { - f.write_fmt(format_args!("{compare_prefix:?}")) - } else { - f.write_str("") - } - } -} - impl QueryProcessor for Prefix { - fn process(query: &mut VecDeque) -> QueryResult + fn process(query: &mut VecDeque, options: &QueryOptions) -> QueryResult where Self: Sized, { @@ -305,59 +124,42 @@ impl QueryProcessor for Prefix { while let Some(in_paren_query_top) = query.front() { if in_paren_query_top != ")" { - list_of_ors.push_back(Or::process(query)?); + list_of_ors.push_back(Or::process(query, options)?); } else { break; } } - // Ensure not empty - if list_of_ors.is_empty() { + let Some(front) = list_of_ors.pop_front() else { return Err(QueryError::new("No values within parentheses group")); - } + }; // Now convert this back to a OR... - // TODO: This seems like a bad way to do it. + // TODO: is there a better way to do this than converting it? let initial_or = Or { lhs: And { - lhs: Prefix { - or: list_of_ors.pop_front().map(Box::new), - compare_prefix: None, - regex_prefix: None, - }, + lhs: Prefix::Or(Box::new(front)), rhs: None, }, rhs: None, }; let returned_or = list_of_ors.into_iter().fold(initial_or, |lhs, rhs| Or { lhs: And { - lhs: Prefix { - or: Some(Box::new(lhs)), - compare_prefix: None, - regex_prefix: None, - }, - rhs: Some(Box::new(Prefix { - or: Some(Box::new(rhs)), - compare_prefix: None, - regex_prefix: None, - })), + lhs: Prefix::Or(Box::new(lhs)), + rhs: Some(Box::new(Prefix::Or(Box::new(rhs)))), }, rhs: None, }); - if let Some(close_paren) = query.pop_front() { + return if let Some(close_paren) = query.pop_front() { if close_paren == ")" { - return Ok(Prefix { - or: Some(Box::new(returned_or)), - regex_prefix: None, - compare_prefix: None, - }); + Ok(Prefix::Or(Box::new(returned_or))) } else { - return Err(QueryError::new("Missing closing parentheses")); + Err(QueryError::new("Missing closing parentheses")) } } else { - return Err(QueryError::new("Missing closing parentheses")); - } + Err(QueryError::new("Missing closing parentheses")) + }; } else if curr == ")" { return Err(QueryError::new("Missing opening parentheses")); } else if curr == "\"" { @@ -365,16 +167,16 @@ impl QueryProcessor for Prefix { // however, that we will DIRECTLY call another process_prefix // call... - let prefix = Prefix::process_in_quotes(query)?; - if let Some(close_quote) = query.pop_front() { + let prefix = Prefix::process_in_quotes(query, options)?; + return if let Some(close_quote) = query.pop_front() { if close_quote == "\"" { - return Ok(prefix); + Ok(prefix) } else { - return Err(QueryError::new("Missing closing quotation")); + Err(QueryError::new("Missing closing quotation")) } } else { - return Err(QueryError::new("Missing closing quotation")); - } + Err(QueryError::new("Missing closing quotation")) + }; } else { // Get prefix type. let prefix_type = curr.parse::()?; @@ -389,11 +191,11 @@ impl QueryProcessor for Prefix { if let Some(content) = content { match &prefix_type { PrefixType::Name => { - return Ok(Prefix { - or: None, - regex_prefix: Some((prefix_type, StringQuery::Value(content))), - compare_prefix: None, - }); + return Ok(Prefix::Attribute(new_string_attribute( + prefix_type, + &content, + options, + )?)); } PrefixType::Pid | PrefixType::State | PrefixType::User => { // We have to check if someone put an "="... @@ -429,21 +231,18 @@ impl QueryProcessor for Prefix { string_value }; - return Ok(Prefix { - or: None, - regex_prefix: Some(( - prefix_type, - StringQuery::Value(final_value), - )), - compare_prefix: None, - }); + return Ok(Prefix::Attribute(new_string_attribute( + prefix_type, + &final_value, + options, + )?)); } } else { - return Ok(Prefix { - or: None, - regex_prefix: Some((prefix_type, StringQuery::Value(content))), - compare_prefix: None, - }); + return Ok(Prefix::Attribute(new_string_attribute( + prefix_type, + &content, + options, + )?)); } } PrefixType::Time => { @@ -481,17 +280,13 @@ impl QueryProcessor for Prefix { ) .map_err(|err| QueryError::new(err.to_string()))?; - return Ok(Prefix { - or: None, - regex_prefix: None, - compare_prefix: Some(( - prefix_type, - ComparableQuery::Time(TimeQuery { - condition, - duration, - }), - )), - }); + return Ok(Prefix::Attribute(new_time_attribute( + prefix_type, + TimeQuery { + condition, + duration, + }, + )?)); } } _ => { @@ -549,30 +344,23 @@ impl QueryProcessor for Prefix { match prefix_type { PrefixType::MemBytes - | PrefixType::Rps - | PrefixType::Wps - | PrefixType::TRead - | PrefixType::TWrite => { + | PrefixType::ReadPerSecond + | PrefixType::WritePerSecond + | PrefixType::TotalRead + | PrefixType::TotalWrite => { process_prefix_units(query, &mut value); } #[cfg(feature = "gpu")] - PrefixType::GMem => { + PrefixType::GpuMemoryBytes => { process_prefix_units(query, &mut value); } _ => {} } - return Ok(Prefix { - or: None, - regex_prefix: None, - compare_prefix: Some(( - prefix_type, - ComparableQuery::Numerical(NumericalQuery { - condition, - value, - }), - )), - }); + return Ok(Prefix::Attribute(new_numerical_attribute( + prefix_type, + NumericalQuery { condition, value }, + )?)); } } } diff --git a/tests/integration/arg_tests.rs b/tests/integration/arg_tests.rs index 72d62747..4db47fcc 100644 --- a/tests/integration/arg_tests.rs +++ b/tests/integration/arg_tests.rs @@ -141,6 +141,16 @@ fn test_missing_default_widget_type() { )); } +#[test] +fn test_invalid_default_cpu_entry() { + no_cfg_btm_command() + .arg("--default_cpu_entry") + .arg("invalid") + .assert() + .failure() + .stderr(predicate::str::contains("possible values")); +} + #[test] #[cfg_attr(feature = "battery", ignore)] fn test_battery_flag() {