From dae65bcd560eef4159e01c7c13cb816d77969005 Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Tue, 9 May 2023 19:20:34 -0400 Subject: [PATCH] bug: fix certain custom column combinations causing issues (#1140) * docs: update some docs related to processes * bug: temp bandaid on column feature to avoid dupes issue --- .../configuration/config-file/processes.md | 2 +- docs/content/usage/widgets/process.md | 2 +- src/app.rs | 2 +- src/options.rs | 9 +- src/options/process_columns.rs | 63 ++-- src/widgets/process_table.rs | 341 +++++++++++++++--- tests/invalid_config_tests.rs | 10 + .../invalid_process_column.toml | 2 + 8 files changed, 345 insertions(+), 86 deletions(-) create mode 100644 tests/invalid_configs/invalid_process_column.toml diff --git a/docs/content/configuration/config-file/processes.md b/docs/content/configuration/config-file/processes.md index 55e5c2c6..b3637f08 100644 --- a/docs/content/configuration/config-file/processes.md +++ b/docs/content/configuration/config-file/processes.md @@ -6,6 +6,6 @@ You can configure which columns are shown by the process widget by setting the ` ```toml [processes] -# Pick which columns you want to use. +# Pick which columns you want to use in any order. columns = ["cpu%", "mem", "mem%", "pid", "count", "name", "command", "read", "write", "Tread", "twrite", "state", "user", "time"] ``` diff --git a/docs/content/usage/widgets/process.md b/docs/content/usage/widgets/process.md index eea4cc0a..a0936f61 100644 --- a/docs/content/usage/widgets/process.md +++ b/docs/content/usage/widgets/process.md @@ -19,7 +19,7 @@ By default, the main process table displays the following information for each p - PID - Name of the process -- CPU use percentage (note this is averaged out per available thread) +- CPU use percentage (note this is averaged out per available thread by default) - Memory use percentage - Reads per second - Writes per second diff --git a/src/app.rs b/src/app.rs index 4496dd79..789329ef 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1243,7 +1243,7 @@ impl App { .proc_state .get_mut_widget_state(self.current_widget.widget_id) { - proc_widget_state.select_column(ProcWidgetColumn::ProcNameOrCmd); + proc_widget_state.select_column(ProcWidgetColumn::ProcNameOrCommand); } } else if let Some(disk) = self .disk_state diff --git a/src/options.rs b/src/options.rs index 5d2fd8ce..22548f4e 100644 --- a/src/options.rs +++ b/src/options.rs @@ -22,10 +22,7 @@ use crate::{ constants::*, units::data_units::DataUnit, utils::error::{self, BottomError}, - widgets::{ - BatteryWidgetState, CpuWidgetState, DiskTableWidget, MemWidgetState, NetWidgetState, - ProcColumn, ProcTableConfig, ProcWidgetMode, ProcWidgetState, TempWidgetState, - }, + widgets::*, }; pub mod layout_options; @@ -35,7 +32,7 @@ use self::process_columns::ProcessConfig; use anyhow::{Context, Result}; -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Clone, Debug, Default, Deserialize)] pub struct Config { pub flags: Option, pub colors: Option, @@ -223,7 +220,7 @@ pub fn build_app( let network_scale_type = get_network_scale_type(matches, config); let network_use_binary_prefix = is_flag_enabled!(network_use_binary_prefix, matches, config); - let proc_columns: Option> = { + let proc_columns: Option> = { let columns = config .processes .as_ref() diff --git a/src/options/process_columns.rs b/src/options/process_columns.rs index 50f010d6..2ec9b50f 100644 --- a/src/options/process_columns.rs +++ b/src/options/process_columns.rs @@ -1,16 +1,16 @@ -use serde::{Deserialize, Serialize}; +use serde::Deserialize; -use crate::widgets::ProcColumn; +use crate::widgets::ProcWidgetColumn; /// Process column settings. -#[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[derive(Clone, Debug, Default, Deserialize)] pub struct ProcessConfig { - pub columns: Option>, + pub columns: Option>, } #[cfg(test)] mod test { - use crate::widgets::ProcColumn; + use crate::widgets::ProcWidgetColumn; use super::ProcessConfig; @@ -31,50 +31,55 @@ mod test { assert_eq!( generated.columns, Some(vec![ - ProcColumn::CpuPercent, - ProcColumn::Pid, - ProcColumn::User, - ProcColumn::MemoryVal, - ProcColumn::TotalRead, - ProcColumn::TotalWrite, - ProcColumn::ReadPerSecond, - ProcColumn::WritePerSecond, - ProcColumn::Time, - ProcColumn::User, - ProcColumn::State, + ProcWidgetColumn::Cpu, + ProcWidgetColumn::PidOrCount, + ProcWidgetColumn::User, + ProcWidgetColumn::Mem, + ProcWidgetColumn::TotalRead, + ProcWidgetColumn::TotalWrite, + ProcWidgetColumn::ReadPerSecond, + ProcWidgetColumn::WritePerSecond, + ProcWidgetColumn::Time, + ProcWidgetColumn::User, + ProcWidgetColumn::State, ]), ); } #[test] fn process_column_settings_2() { - let config = r#"columns = ["MEM%"]"#; - let generated: ProcessConfig = toml_edit::de::from_str(config).unwrap(); - assert_eq!(generated.columns, Some(vec![ProcColumn::MemoryPercent])); - } - - #[test] - fn process_column_settings_3() { - let config = r#"columns = ["MEM%", "TWrite", "Cpuz", "read", "wps"]"#; + let config = r#"columns = ["MEM", "TWrite", "Cpuz", "read", "wps"]"#; toml_edit::de::from_str::(config).expect_err("Should error out!"); } #[test] - fn process_column_settings_4() { + fn process_column_settings_3() { let config = r#"columns = ["Twrite", "T.Write"]"#; let generated: ProcessConfig = toml_edit::de::from_str(config).unwrap(); - assert_eq!(generated.columns, Some(vec![ProcColumn::TotalWrite; 2])); + assert_eq!( + generated.columns, + Some(vec![ProcWidgetColumn::TotalWrite; 2]) + ); let config = r#"columns = ["Tread", "T.read"]"#; let generated: ProcessConfig = toml_edit::de::from_str(config).unwrap(); - assert_eq!(generated.columns, Some(vec![ProcColumn::TotalRead; 2])); + assert_eq!( + generated.columns, + Some(vec![ProcWidgetColumn::TotalRead; 2]) + ); let config = r#"columns = ["read", "rps", "r/s"]"#; let generated: ProcessConfig = toml_edit::de::from_str(config).unwrap(); - assert_eq!(generated.columns, Some(vec![ProcColumn::ReadPerSecond; 3])); + assert_eq!( + generated.columns, + Some(vec![ProcWidgetColumn::ReadPerSecond; 3]) + ); let config = r#"columns = ["write", "wps", "w/s"]"#; let generated: ProcessConfig = toml_edit::de::from_str(config).unwrap(); - assert_eq!(generated.columns, Some(vec![ProcColumn::WritePerSecond; 3])); + assert_eq!( + generated.columns, + Some(vec![ProcWidgetColumn::WritePerSecond; 3]) + ); } } diff --git a/src/widgets/process_table.rs b/src/widgets/process_table.rs index 2566525f..ca1e7a11 100644 --- a/src/widgets/process_table.rs +++ b/src/widgets/process_table.rs @@ -3,6 +3,7 @@ use std::{borrow::Cow, collections::BTreeMap}; use hashbrown::{HashMap, HashSet}; use indexmap::IndexSet; use itertools::Itertools; +use serde::{de::Error, Deserialize}; use crate::{ app::{ @@ -103,14 +104,14 @@ pub struct ProcTableConfig { } /// A hacky workaround for now. -#[derive(PartialEq, Eq, Hash)] +#[derive(PartialEq, Eq, Hash, Clone, Copy, Debug)] pub enum ProcWidgetColumn { PidOrCount, - ProcNameOrCmd, + ProcNameOrCommand, Cpu, Mem, - Rps, - Wps, + ReadPerSecond, + WritePerSecond, TotalRead, TotalWrite, User, @@ -118,6 +119,34 @@ pub enum ProcWidgetColumn { Time, } +impl<'de> Deserialize<'de> for ProcWidgetColumn { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let value = String::deserialize(deserializer)?.to_lowercase(); + match value.as_str() { + "cpu%" => Ok(ProcWidgetColumn::Cpu), + "mem" => Ok(ProcWidgetColumn::Mem), + "mem%" => Ok(ProcWidgetColumn::Mem), + "pid" => Ok(ProcWidgetColumn::PidOrCount), + "count" => Ok(ProcWidgetColumn::PidOrCount), + "name" => Ok(ProcWidgetColumn::ProcNameOrCommand), + "command" => Ok(ProcWidgetColumn::ProcNameOrCommand), + "read" | "r/s" | "rps" => Ok(ProcWidgetColumn::ReadPerSecond), + "write" | "w/s" | "wps" => Ok(ProcWidgetColumn::WritePerSecond), + "tread" | "t.read" => Ok(ProcWidgetColumn::TotalRead), + "twrite" | "t.write" => Ok(ProcWidgetColumn::TotalWrite), + "state" => Ok(ProcWidgetColumn::State), + "user" => Ok(ProcWidgetColumn::User), + "time" => Ok(ProcWidgetColumn::Time), + _ => Err(D::Error::custom("doesn't match any column type")), + } + } +} + +// This is temporary. Switch back to `ProcColumn` later! + pub struct ProcWidgetState { pub mode: ProcWidgetMode, @@ -188,7 +217,7 @@ impl ProcWidgetState { pub fn new( config: &AppConfigFields, mode: ProcWidgetMode, table_config: ProcTableConfig, - colours: &CanvasColours, config_columns: &Option>, + colours: &CanvasColours, config_columns: &Option>, ) -> Self { let process_search_state = { let mut pss = ProcessSearchState::default(); @@ -210,15 +239,50 @@ impl ProcWidgetState { let columns: Vec> = { use ProcColumn::*; - match config_columns { - Some(columns) if !columns.is_empty() => { - columns.iter().cloned().map(make_column).collect() - } - _ => { - let is_count = matches!(mode, ProcWidgetMode::Grouped); - let is_command = table_config.is_command; - let mem_vals = table_config.show_memory_as_values; + let is_count = matches!(mode, ProcWidgetMode::Grouped); + let is_command = table_config.is_command; + let mem_vals = table_config.show_memory_as_values; + match config_columns { + Some(columns) if !columns.is_empty() => columns + .into_iter() + .map(|c| { + let col = match c { + ProcWidgetColumn::PidOrCount => { + if is_count { + Count + } else { + Pid + } + } + ProcWidgetColumn::ProcNameOrCommand => { + if is_command { + Command + } else { + Name + } + } + ProcWidgetColumn::Cpu => CpuPercent, + ProcWidgetColumn::Mem => { + if mem_vals { + MemoryVal + } else { + MemoryPercent + } + } + ProcWidgetColumn::ReadPerSecond => ReadPerSecond, + ProcWidgetColumn::WritePerSecond => WritePerSecond, + ProcWidgetColumn::TotalRead => TotalRead, + ProcWidgetColumn::TotalWrite => TotalWrite, + ProcWidgetColumn::User => User, + ProcWidgetColumn::State => State, + ProcWidgetColumn::Time => Time, + }; + + make_column(col) + }) + .collect(), + _ => { let default_columns = [ if is_count { Count } else { Pid }, if is_command { Command } else { Name }, @@ -246,9 +310,9 @@ impl ProcWidgetState { CpuPercent => ProcWidgetColumn::Cpu, MemoryVal | MemoryPercent => ProcWidgetColumn::Mem, Pid | Count => ProcWidgetColumn::PidOrCount, - Name | Command => ProcWidgetColumn::ProcNameOrCmd, - ReadPerSecond => ProcWidgetColumn::Rps, - WritePerSecond => ProcWidgetColumn::Wps, + Name | Command => ProcWidgetColumn::ProcNameOrCommand, + ReadPerSecond => ProcWidgetColumn::ReadPerSecond, + WritePerSecond => ProcWidgetColumn::WritePerSecond, TotalRead => ProcWidgetColumn::TotalRead, TotalWrite => ProcWidgetColumn::TotalWrite, State => ProcWidgetColumn::State, @@ -302,7 +366,7 @@ impl ProcWidgetState { pub fn is_using_command(&self) -> bool { self.column_mapping - .get_index_of(&ProcWidgetColumn::ProcNameOrCmd) + .get_index_of(&ProcWidgetColumn::ProcNameOrCommand) .and_then(|index| { self.table .columns @@ -746,7 +810,7 @@ impl ProcWidgetState { pub fn toggle_command(&mut self) { if let Some(index) = self .column_mapping - .get_index_of(&ProcWidgetColumn::ProcNameOrCmd) + .get_index_of(&ProcWidgetColumn::ProcNameOrCommand) { if let Some(col) = self.table.columns.get_mut(index) { let inner = col.inner_mut(); @@ -1053,10 +1117,9 @@ mod test { .collect::>() } - fn init_default_state(columns: &[ProcColumn]) -> ProcWidgetState { + fn init_state(table_config: ProcTableConfig, columns: &[ProcWidgetColumn]) -> ProcWidgetState { let config = AppConfigFields::default(); let colours = CanvasColours::default(); - let table_config = ProcTableConfig::default(); let columns = Some(columns.iter().cloned().collect()); ProcWidgetState::new( @@ -1068,33 +1131,49 @@ mod test { ) } + fn init_default_state(columns: &[ProcWidgetColumn]) -> ProcWidgetState { + init_state(ProcTableConfig::default(), columns) + } + #[test] - fn test_custom_columns() { + fn custom_columns() { + let init_columns = vec![ + ProcWidgetColumn::PidOrCount, + ProcWidgetColumn::ProcNameOrCommand, + ProcWidgetColumn::Mem, + ProcWidgetColumn::State, + ]; let columns = vec![ ProcColumn::Pid, - ProcColumn::Command, + ProcColumn::Name, ProcColumn::MemoryPercent, ProcColumn::State, ]; - let state = init_default_state(&columns); + let state = init_default_state(&init_columns); assert_eq!(get_columns(&state.table), columns); } #[test] fn toggle_count_pid() { + let init_columns = [ + ProcWidgetColumn::PidOrCount, + ProcWidgetColumn::ProcNameOrCommand, + ProcWidgetColumn::Mem, + ProcWidgetColumn::State, + ]; let original_columns = vec![ ProcColumn::Pid, - ProcColumn::Command, + ProcColumn::Name, ProcColumn::MemoryPercent, ProcColumn::State, ]; let new_columns = vec![ ProcColumn::Count, - ProcColumn::Command, + ProcColumn::Name, ProcColumn::MemoryPercent, ]; - let mut state = init_default_state(&original_columns); + let mut state = init_default_state(&init_columns); assert_eq!(get_columns(&state.table), original_columns); // This should hide the state. @@ -1108,20 +1187,27 @@ mod test { #[test] fn toggle_count_pid_2() { + let init_columns = [ + ProcWidgetColumn::ProcNameOrCommand, + ProcWidgetColumn::Mem, + ProcWidgetColumn::User, + ProcWidgetColumn::State, + ProcWidgetColumn::PidOrCount, + ]; let original_columns = vec![ - ProcColumn::Command, + ProcColumn::Name, ProcColumn::MemoryPercent, ProcColumn::User, ProcColumn::State, ProcColumn::Pid, ]; let new_columns = vec![ - ProcColumn::Command, + ProcColumn::Name, ProcColumn::MemoryPercent, ProcColumn::Count, ]; - let mut state = init_default_state(&original_columns); + let mut state = init_default_state(&init_columns); assert_eq!(get_columns(&state.table), original_columns); // This should hide the state. @@ -1135,20 +1221,30 @@ mod test { #[test] fn toggle_command() { + let init_columns = [ + ProcWidgetColumn::PidOrCount, + ProcWidgetColumn::State, + ProcWidgetColumn::Mem, + ProcWidgetColumn::ProcNameOrCommand, + ]; let original_columns = vec![ ProcColumn::Pid, - ProcColumn::MemoryPercent, ProcColumn::State, + ProcColumn::MemoryPercent, ProcColumn::Command, ]; let new_columns = vec![ ProcColumn::Pid, - ProcColumn::MemoryPercent, ProcColumn::State, + ProcColumn::MemoryPercent, ProcColumn::Name, ]; - let mut state = init_default_state(&original_columns); + let table_config = ProcTableConfig { + is_command: true, + ..Default::default() + }; + let mut state = init_state(table_config, &init_columns); assert_eq!(get_columns(&state.table), original_columns); state.toggle_command(); @@ -1160,20 +1256,26 @@ mod test { #[test] fn toggle_mem_percentage() { + let init_columns = [ + ProcWidgetColumn::PidOrCount, + ProcWidgetColumn::Mem, + ProcWidgetColumn::State, + ProcWidgetColumn::ProcNameOrCommand, + ]; let original_columns = vec![ ProcColumn::Pid, ProcColumn::MemoryPercent, ProcColumn::State, - ProcColumn::Command, + ProcColumn::Name, ]; let new_columns = vec![ ProcColumn::Pid, ProcColumn::MemoryVal, ProcColumn::State, - ProcColumn::Command, + ProcColumn::Name, ]; - let mut state = init_default_state(&original_columns); + let mut state = init_default_state(&init_columns); assert_eq!(get_columns(&state.table), original_columns); state.toggle_mem_percentage(); @@ -1185,20 +1287,30 @@ mod test { #[test] fn toggle_mem_percentage_2() { - let new_columns = vec![ - ProcColumn::Pid, - ProcColumn::MemoryPercent, - ProcColumn::State, - ProcColumn::Command, + let init_columns = [ + ProcWidgetColumn::PidOrCount, + ProcWidgetColumn::Mem, + ProcWidgetColumn::State, + ProcWidgetColumn::ProcNameOrCommand, ]; let original_columns = vec![ ProcColumn::Pid, ProcColumn::MemoryVal, ProcColumn::State, - ProcColumn::Command, + ProcColumn::Name, + ]; + let new_columns = vec![ + ProcColumn::Pid, + ProcColumn::MemoryPercent, + ProcColumn::State, + ProcColumn::Name, ]; - let mut state = init_default_state(&original_columns); + let table_config = ProcTableConfig { + show_memory_as_values: true, + ..Default::default() + }; + let mut state = init_state(table_config, &init_columns); assert_eq!(get_columns(&state.table), original_columns); state.toggle_mem_percentage(); @@ -1209,15 +1321,25 @@ mod test { } #[test] - fn test_is_using_command() { + fn columns_and_is_using_command() { + let init_columns = [ + ProcWidgetColumn::PidOrCount, + ProcWidgetColumn::Mem, + ProcWidgetColumn::State, + ProcWidgetColumn::ProcNameOrCommand, + ]; let original_columns = vec![ ProcColumn::Pid, - ProcColumn::MemoryVal, + ProcColumn::MemoryPercent, ProcColumn::State, ProcColumn::Command, ]; - let mut state = init_default_state(&original_columns); + let table_config = ProcTableConfig { + is_command: true, + ..Default::default() + }; + let mut state = init_state(table_config, &init_columns); assert_eq!(get_columns(&state.table), original_columns); assert!(state.is_using_command()); @@ -1229,15 +1351,25 @@ mod test { } #[test] - fn test_is_memory() { + fn columns_and_is_memory() { + let init_columns = [ + ProcWidgetColumn::PidOrCount, + ProcWidgetColumn::Mem, + ProcWidgetColumn::State, + ProcWidgetColumn::ProcNameOrCommand, + ]; let original_columns = vec![ ProcColumn::Pid, ProcColumn::MemoryVal, ProcColumn::State, - ProcColumn::Command, + ProcColumn::Name, ]; - let mut state = init_default_state(&original_columns); + let table_config = ProcTableConfig { + show_memory_as_values: true, + ..Default::default() + }; + let mut state = init_state(table_config, &init_columns); assert_eq!(get_columns(&state.table), original_columns); assert!(!state.is_mem_percent()); @@ -1247,4 +1379,117 @@ mod test { state.toggle_mem_percentage(); assert!(!state.is_mem_percent()); } + + /// Tests toggling if both mem and mem% columns are configured. + /// + /// Currently, this test doesn't really do much, since we treat these two columns as the same - this test is + /// intended for use later when we might allow both at the same time. + #[test] + fn double_memory_sim_toggle() { + let init_columns = [ + ProcWidgetColumn::Mem, + ProcWidgetColumn::PidOrCount, + ProcWidgetColumn::State, + ProcWidgetColumn::ProcNameOrCommand, + ProcWidgetColumn::Mem, + ]; + let original_columns = vec![ + ProcColumn::MemoryPercent, + ProcColumn::Pid, + ProcColumn::State, + ProcColumn::Name, + ]; + let new_columns = vec![ + ProcColumn::MemoryVal, + ProcColumn::Pid, + ProcColumn::State, + ProcColumn::Name, + ]; + + let mut state = init_default_state(&init_columns); + assert_eq!(get_columns(&state.table), original_columns); + + state.toggle_mem_percentage(); + assert_eq!(get_columns(&state.table), new_columns); + + state.toggle_mem_percentage(); + assert_eq!(get_columns(&state.table), original_columns); + } + + /// Tests toggling if both pid and count columns are configured. + /// + /// Currently, this test doesn't really do much, since we treat these two columns as the same - this test is + /// intended for use later when we might allow both at the same time. + #[test] + fn pid_and_count_sim_toggle() { + let init_columns = [ + ProcWidgetColumn::ProcNameOrCommand, + ProcWidgetColumn::PidOrCount, + ProcWidgetColumn::Mem, + ProcWidgetColumn::State, + ProcWidgetColumn::PidOrCount, + ]; + let original_columns = vec![ + ProcColumn::Name, + ProcColumn::Pid, + ProcColumn::MemoryPercent, + ProcColumn::State, + ]; + let new_columns = vec![ + ProcColumn::Name, + ProcColumn::Count, + ProcColumn::MemoryPercent, + ]; + + let mut state = init_default_state(&init_columns); + assert_eq!(get_columns(&state.table), original_columns); + + // This should hide the state. + state.toggle_tab(); + assert_eq!(get_columns(&state.table), new_columns); + + // This should re-reveal the state. + state.toggle_tab(); + assert_eq!(get_columns(&state.table), original_columns); + } + + /// Tests toggling if both command and name columns are configured. + /// + /// Currently, this test doesn't really do much, since we treat these two columns as the same - this test is + /// intended for use later when we might allow both at the same time. + #[test] + fn command_name_sim_toggle() { + let init_columns = [ + ProcWidgetColumn::ProcNameOrCommand, + ProcWidgetColumn::PidOrCount, + ProcWidgetColumn::State, + ProcWidgetColumn::Mem, + ProcWidgetColumn::ProcNameOrCommand, + ]; + let original_columns = vec![ + ProcColumn::Command, + ProcColumn::Pid, + ProcColumn::State, + ProcColumn::MemoryPercent, + ]; + let new_columns = vec![ + ProcColumn::Name, + ProcColumn::Pid, + ProcColumn::State, + ProcColumn::MemoryPercent, + ]; + + let table_config = ProcTableConfig { + is_command: true, + ..Default::default() + }; + let mut state = init_state(table_config, &init_columns); + assert_eq!(get_columns(&state.table), original_columns); + + state.toggle_command(); + assert_eq!(get_columns(&state.table), new_columns); + + state.toggle_command(); + assert_eq!(get_columns(&state.table), original_columns); + } } diff --git a/tests/invalid_config_tests.rs b/tests/invalid_config_tests.rs index fc48dde4..d000a5af 100644 --- a/tests/invalid_config_tests.rs +++ b/tests/invalid_config_tests.rs @@ -141,3 +141,13 @@ fn test_invalid_default_widget_count() { .failure() .stderr(predicate::str::contains("number too large")); } + +#[test] +fn test_invalid_process_column() { + btm_command() + .arg("-C") + .arg("./tests/invalid_configs/invalid_process_column.toml") + .assert() + .failure() + .stderr(predicate::str::contains("doesn't match")); +} diff --git a/tests/invalid_configs/invalid_process_column.toml b/tests/invalid_configs/invalid_process_column.toml new file mode 100644 index 00000000..60ef84dd --- /dev/null +++ b/tests/invalid_configs/invalid_process_column.toml @@ -0,0 +1,2 @@ +[processes] +columns = ["cpu", "fake"]