tyler.durden
/
bottom
mirror of https://github.com/ClementTsang/bottom.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

refactor: clean up some options code (#1029) 

* remove some dead code

* use macros to help clean up clutter for binary flags

* add test

* group

* fix using gpu feature
This commit is contained in:
Clement Tsang 2023-02-25 04:24:38 -05:00 committed by GitHub
parent 449d735601
commit cabc594279
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 155 additions and 315 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/app.rs
View File

@ -31,7 +31,7 @@ pub mod states;
use frozen_state::FrozenState;
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Eq, PartialEq)]
pub enum AxisScaling {
Log,
Linear,
@ -45,7 +45,7 @@ impl Default for AxisScaling {
/// AppConfigFields is meant to cover basic fields that would normally be set
/// by config files or launch options.
#[derive(Debug, Default)]
#[derive(Debug, Default, Eq, PartialEq)]
pub struct AppConfigFields {
pub update_rate_in_milliseconds: u64,
pub temperature_type: temperature::TemperatureType,

2
src/app/data_harvester/temperature.rs
View File

@ -24,7 +24,7 @@ pub struct TempHarvest {
pub temperature: f32,
}
#[derive(Clone, Debug, Copy)]
#[derive(Clone, Debug, Copy, PartialEq, Eq)]
pub enum TemperatureType {
Celsius,
Kelvin,

1
src/clap.rs
View File

@ -443,6 +443,7 @@ use CPU (3) as the default instead.
app
}
#[cfg(test)]
mod test {
use super::*;

439
src/options.rs
View File

@ -43,18 +43,6 @@ pub struct Config {
pub net_filter: Option<IgnoreList>,
}
impl Config {
pub fn get_config_as_bytes(&self) -> anyhow::Result<Vec<u8>> {
let config_string: Vec<Cow<'_, str>> = vec![
// Top level
CONFIG_TOP_HEAD.into(),
toml::to_string_pretty(self)?.into(),
];
Ok(config_string.concat().as_bytes().to_vec())
}
}
#[derive(Clone, Debug, Default, Deserialize, Serialize, TypedBuilder)]
pub struct ConfigFlags {
pub hide_avg_cpu: Option<bool>,
@ -81,7 +69,7 @@ pub struct ConfigFlags {
pub battery: Option<bool>,
pub disable_click: Option<bool>,
pub no_write: Option<bool>,
// For built-in colour palettes.
/// For built-in colour palettes.
pub color: Option<String>,
pub mem_as_value: Option<bool>,
pub tree: Option<bool>,
@ -97,24 +85,6 @@ pub struct ConfigFlags {
pub retention: Option<Duration>,
}
#[derive(Clone, Default, Debug, Deserialize, Serialize)]
pub struct WidgetIdEnabled {
id: u64,
enabled: bool,
}
impl WidgetIdEnabled {
pub fn create_from_hashmap(hashmap: &HashMap<u64, bool>) -> Vec<WidgetIdEnabled> {
hashmap
.iter()
.map(|(id, enabled)| WidgetIdEnabled {
id: *id,
enabled: *enabled,
})
.collect()
}
}
#[derive(Clone, Debug, Default, Deserialize, Serialize)]
pub struct ConfigColours {
pub table_header_color: Option<Cow<'static, str>>,
@ -143,11 +113,10 @@ pub struct ConfigColours {
}
impl ConfigColours {
/// Returns `true` if there is a [`ConfigColours`] that is empty or there isn't one at all.
pub fn is_empty(&self) -> bool {
if let Ok(serialized_string) = toml::to_string(self) {
if !serialized_string.is_empty() {
return false;
}
return serialized_string.is_empty();
}
true
@ -174,6 +143,28 @@ pub struct IgnoreList {
pub whole_word: bool,
}
macro_rules! is_flag_enabled {
($flag_name:ident, $matches:expr, $config:expr) => {
if $matches.contains_id(stringify!($flag_name)) {
true
} else if let Some(flags) = &$config.flags {
flags.$flag_name.unwrap_or(false)
} else {
false
}
};
($cmd_flag:literal, $cfg_flag:ident, $matches:expr, $config:expr) => {
if $matches.contains_id($cmd_flag) {
true
} else if let Some(flags) = &$config.flags {
flags.$cfg_flag.unwrap_or(false)
} else {
false
}
};
}
pub fn build_app(
matches: &ArgMatches, config: &mut Config, widget_layout: &BottomLayout,
default_widget_id: u64, default_widget_type_option: &Option<BottomWidgetType>,
@ -183,16 +174,18 @@ pub fn build_app(
let retention_ms =
get_retention_ms(matches, config).context("Update `retention` in your config file.")?;
let autohide_time = get_autohide_time(matches, config);
let autohide_time = is_flag_enabled!(autohide_time, matches, config);
let default_time_value = get_default_time_value(matches, config, retention_ms)
.context("Update 'default_time_value' in your config file.")?;
let use_basic_mode = get_use_basic_mode(matches, config);
let use_basic_mode = is_flag_enabled!(basic, matches, config);
let expanded_upon_startup = is_flag_enabled!(expanded_on_startup, matches, config);
// For processes
let is_grouped = get_app_grouping(matches, config);
let is_case_sensitive = get_app_case_sensitive(matches, config);
let is_match_whole_word = get_app_match_whole_word(matches, config);
let is_use_regex = get_app_use_regex(matches, config);
let is_grouped = is_flag_enabled!("group", group_processes, matches, config);
let is_case_sensitive = is_flag_enabled!(case_sensitive, matches, config);
let is_match_whole_word = is_flag_enabled!(whole_word, matches, config);
let is_use_regex = is_flag_enabled!(regex, matches, config);
let mut widget_map = HashMap::new();
let mut cpu_state_map: HashMap<u64, CpuWidgetState> = HashMap::new();
@ -214,14 +207,14 @@ pub fn build_app(
let is_custom_layout = config.row.is_some();
let mut used_widget_set = HashSet::new();
let show_memory_as_values = get_mem_as_value(matches, config);
let is_default_tree = get_is_default_tree(matches, config);
let is_default_command = get_is_default_process_command(matches, config);
let is_advanced_kill = !get_is_advanced_kill_disabled(matches, config);
let show_memory_as_values = is_flag_enabled!(mem_as_value, matches, config);
let is_default_tree = is_flag_enabled!(tree, matches, config);
let is_default_command = is_flag_enabled!(process_command, matches, config);
let is_advanced_kill = !(is_flag_enabled!(disable_advanced_kill, matches, config));
let network_unit_type = get_network_unit_type(matches, config);
let network_scale_type = get_network_scale_type(matches, config);
let network_use_binary_prefix = get_network_use_binary_prefix(matches, config);
let network_use_binary_prefix = is_flag_enabled!(network_use_binary_prefix, matches, config);
let app_config_fields = AppConfigFields {
update_rate_in_milliseconds: get_update_rate_in_milliseconds(matches, config)
@ -229,21 +222,21 @@ pub fn build_app(
temperature_type: get_temperature(matches, config)
.context("Update 'temperature_type' in your config file.")?,
show_average_cpu: get_show_average_cpu(matches, config),
use_dot: get_use_dot(matches, config),
left_legend: get_use_left_legend(matches, config),
use_current_cpu_total: get_use_current_cpu_total(matches, config),
unnormalized_cpu: get_unnormalized_cpu(matches, config),
use_dot: is_flag_enabled!(dot_marker, matches, config),
left_legend: is_flag_enabled!(left_legend, matches, config),
use_current_cpu_total: is_flag_enabled!(current_usage, matches, config),
unnormalized_cpu: is_flag_enabled!(unnormalized_cpu, matches, config),
use_basic_mode,
default_time_value,
time_interval: get_time_interval(matches, config, retention_ms)
.context("Update 'time_delta' in your config file.")?,
hide_time: get_hide_time(matches, config),
hide_time: is_flag_enabled!(hide_time, matches, config),
autohide_time,
use_old_network_legend: get_use_old_network_legend(matches, config),
table_gap: u16::from(!get_hide_table_gap(matches, config)),
disable_click: get_disable_click(matches, config),
use_old_network_legend: is_flag_enabled!(use_old_network_legend, matches, config),
table_gap: u16::from(!(is_flag_enabled!(hide_table_gap, matches, config))),
disable_click: is_flag_enabled!(disable_click, matches, config),
enable_gpu_memory: get_enable_gpu_memory(matches, config),
show_table_scroll_position: get_show_table_scroll_position(matches, config),
show_table_scroll_position: is_flag_enabled!(show_table_scroll_position, matches, config),
is_advanced_kill,
network_scale_type,
network_unit_type,
@ -407,8 +400,6 @@ pub fn build_app(
let net_filter =
get_ignore_list(&config.net_filter).context("Update 'net_filter' in your config file")?;
let expanded_upon_startup = get_expanded_on_startup(matches, config);
Ok(App::builder()
.app_config_fields(app_config_fields)
.cpu_state(CpuState::init(cpu_state_map))
@ -435,12 +426,13 @@ pub fn build_app(
pub fn get_widget_layout(
matches: &ArgMatches, config: &Config,
) -> error::Result<(BottomLayout, u64, Option<BottomWidgetType>)> {
let left_legend = get_use_left_legend(matches, config);
let left_legend = is_flag_enabled!(left_legend, matches, config);
let (default_widget_type, mut default_widget_count) =
get_default_widget_and_count(matches, config)?;
let mut default_widget_id = 1;
let bottom_layout = if get_use_basic_mode(matches, config) {
let bottom_layout = if is_flag_enabled!(basic, matches, config) {
default_widget_id = DEFAULT_WIDGET_ID;
BottomLayout::init_basic_default(get_use_battery(matches, config))
@ -562,65 +554,6 @@ fn get_show_average_cpu(matches: &ArgMatches, config: &Config) -> bool {
true
}
fn get_use_dot(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("dot_marker") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(dot_marker) = flags.dot_marker {
return dot_marker;
}
}
false
}
fn get_use_left_legend(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("left_legend") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(left_legend) = flags.left_legend {
return left_legend;
}
}
false
}
fn get_use_current_cpu_total(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("current_usage") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(current_usage) = flags.current_usage {
return current_usage;
}
}
false
}
fn get_unnormalized_cpu(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("unnormalized_cpu") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(unnormalized_cpu) = flags.unnormalized_cpu {
return unnormalized_cpu;
}
}
false
}
fn get_use_basic_mode(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("basic") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(basic) = flags.basic {
return basic;
}
}
false
}
/// FIXME: Let this accept human times.
fn get_default_time_value(
matches: &ArgMatches, config: &Config, retention_ms: u64,
@ -689,82 +622,6 @@ fn get_time_interval(
Ok(time_interval)
}
pub fn get_app_grouping(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("group") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(grouping) = flags.group_processes {
return grouping;
}
}
false
}
pub fn get_app_case_sensitive(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("case_sensitive") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(case_sensitive) = flags.case_sensitive {
return case_sensitive;
}
}
false
}
pub fn get_app_match_whole_word(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("whole_word") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(whole_word) = flags.whole_word {
return whole_word;
}
}
false
}
pub fn get_app_use_regex(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("regex") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(regex) = flags.regex {
return regex;
}
}
false
}
fn get_hide_time(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("hide_time") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(hide_time) = flags.hide_time {
return hide_time;
}
}
false
}
fn get_autohide_time(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("autohide_time") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(autohide_time) = flags.autohide_time {
return autohide_time;
}
}
false
}
fn get_expanded_on_startup(matches: &ArgMatches, config: &Config) -> bool {
matches.contains_id("expanded_on_startup")
|| config
.flags
.as_ref()
.and_then(|x| x.expanded_on_startup)
.unwrap_or(false)
}
fn get_default_widget_and_count(
matches: &ArgMatches, config: &Config,
) -> error::Result<(Option<BottomWidgetType>, u64)> {
@ -816,50 +673,18 @@ fn get_default_widget_and_count(
}
}
fn get_disable_click(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("disable_click") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(disable_click) = flags.disable_click {
return disable_click;
}
}
false
}
fn get_use_old_network_legend(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("use_old_network_legend") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(use_old_network_legend) = flags.use_old_network_legend {
return use_old_network_legend;
}
}
false
}
fn get_hide_table_gap(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("hide_table_gap") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(hide_table_gap) = flags.hide_table_gap {
return hide_table_gap;
}
}
false
}
#[allow(unused_variables)]
fn get_use_battery(matches: &ArgMatches, config: &Config) -> bool {
#[cfg(feature = "battery")]
if let Ok(battery_manager) = Manager::new() {
if let Ok(batteries) = battery_manager.batteries() {
if batteries.count() == 0 {
return false;
{
if let Ok(battery_manager) = Manager::new() {
if let Ok(batteries) = battery_manager.batteries() {
if batteries.count() == 0 {
return false;
}
}
}
}
if cfg!(feature = "battery") {
if matches.contains_id("battery") {
return true;
} else if let Some(flags) = &config.flags {
@ -868,11 +693,14 @@ fn get_use_battery(matches: &ArgMatches, config: &Config) -> bool {
}
}
}
false
}
#[allow(unused_variables)]
fn get_enable_gpu_memory(matches: &ArgMatches, config: &Config) -> bool {
if cfg!(feature = "gpu") {
#[cfg(feature = "gpu")]
{
if matches.contains_id("enable_gpu_memory") {
return true;
} else if let Some(flags) = &config.flags {
@ -881,18 +709,7 @@ fn get_enable_gpu_memory(matches: &ArgMatches, config: &Config) -> bool {
}
}
}
false
}
#[allow(dead_code)]
fn get_no_write(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("no_write") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(no_write) = flags.no_write {
return no_write;
}
}
false
}
@ -958,61 +775,6 @@ pub fn get_color_scheme(matches: &ArgMatches, config: &Config) -> error::Result<
Ok(ColourScheme::Default)
}
fn get_mem_as_value(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("mem_as_value") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(mem_as_value) = flags.mem_as_value {
return mem_as_value;
}
}
false
}
fn get_is_default_tree(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("tree") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(tree) = flags.tree {
return tree;
}
}
false
}
fn get_show_table_scroll_position(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("show_table_scroll_position") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(show_table_scroll_position) = flags.show_table_scroll_position {
return show_table_scroll_position;
}
}
false
}
fn get_is_default_process_command(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("process_command") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(process_command) = flags.process_command {
return process_command;
}
}
false
}
fn get_is_advanced_kill_disabled(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("disable_advanced_kill") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(disable_advanced_kill) = flags.disable_advanced_kill {
return disable_advanced_kill;
}
}
false
}
fn get_network_unit_type(matches: &ArgMatches, config: &Config) -> DataUnit {
if matches.contains_id("network_use_bytes") {
return DataUnit::Byte;
@ -1041,17 +803,6 @@ fn get_network_scale_type(matches: &ArgMatches, config: &Config) -> AxisScaling
AxisScaling::Linear
}
fn get_network_use_binary_prefix(matches: &ArgMatches, config: &Config) -> bool {
if matches.contains_id("network_use_binary_prefix") {
return true;
} else if let Some(flags) = &config.flags {
if let Some(network_use_binary_prefix) = flags.network_use_binary_prefix {
return network_use_binary_prefix;
}
}
false
}
fn get_retention_ms(matches: &ArgMatches, config: &Config) -> error::Result<u64> {
const DEFAULT_RETENTION_MS: u64 = 600 * 1000; // Keep 10 minutes of data.
@ -1069,3 +820,71 @@ fn get_retention_ms(matches: &ArgMatches, config: &Config) -> error::Result<u64>
Ok(DEFAULT_RETENTION_MS)
}
}
#[cfg(test)]
mod test {
use clap::ArgMatches;
use crate::{app::App, canvas::canvas_styling::CanvasColours};
use super::{get_color_scheme, get_widget_layout, Config};
fn create_app(mut config: Config, matches: ArgMatches) -> App {
let (layout, id, ty) = get_widget_layout(&matches, &config).unwrap();
let colours =
CanvasColours::new(get_color_scheme(&matches, &config).unwrap(), &config).unwrap();
super::build_app(&matches, &mut config, &layout, id, &ty, &colours).unwrap()
}
// TODO: There's probably a better way to create clap options AND unify together to avoid the possibility of
// typos/mixing up. Use macros!
#[test]
fn verify_cli_options_build() {
let app = crate::clap::build_app();
let default_app = {
let app = app.clone();
let config = Config::default();
let matches = app.get_matches_from([""]);
create_app(config, matches)
};
// Skip battery since it's tricky to test depending on the platform testing.
let skip = ["help", "version", "celsius", "battery"];
for arg in app.get_arguments().collect::<Vec<_>>() {
let arg_name = arg
.get_long_and_visible_aliases()
.unwrap()
.first()
.unwrap()
.to_owned();
if !arg.is_takes_value_set() && !skip.contains(&arg_name) {
let arg = format!("--{arg_name}");
let arguments = vec!["btm", &arg];
let app = app.clone();
let config = Config::default();
let matches = app.get_matches_from(arguments);
let testing_app = create_app(config, matches);
if (default_app.app_config_fields == testing_app.app_config_fields)
&& default_app.is_expanded == testing_app.is_expanded
&& default_app
.proc_state
.widget_states
.iter()
.zip(testing_app.proc_state.widget_states.iter())
.all(|(a, b)| (a.1.test_equality(b.1)))
{
panic!("failed on {arg_name}");
}
}
}
}
}

2
src/units/data_units.rs
View File

@ -1,4 +1,4 @@
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Eq, PartialEq)]
pub enum DataUnit {
Byte,
Bit,

22
src/widgets/process_table.rs
View File

@ -61,7 +61,7 @@ impl ProcessSearchState {
}
}
#[derive(Clone, Debug)]
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum ProcWidgetMode {
Tree { collapsed_pids: FxHashSet<Pid> },
Grouped,
@ -814,6 +814,26 @@ impl ProcWidgetState {
self.is_sort_open = false;
self.force_rerender_and_update();
}
#[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
.table
.columns
.iter()
.map(|c| c.header())
.collect::<Vec<_>>()
== other
.table
.columns
.iter()
.map(|c| c.header())
.collect::<Vec<_>>()
}
}
#[inline]