From c0a8c347e12ae7924ed78ee9c35f2acd7152193e Mon Sep 17 00:00:00 2001
From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com>
Date: Wed, 18 Nov 2020 20:00:31 -0500
Subject: [PATCH] bug: remove buggy auto-generated CPU colour implementation
 (#308)

Removes the random automatically generated colours for the CPU metrics. This was not supported in all terminal emulators, and would cause some of them to break (namely macOS Terminal).

Instead we'll default to colours we can be more certain will work and loop through them as required. Users can still override these colours with their own.
---
 CHANGELOG.md                              |  2 +
 src/canvas.rs                             |  1 -
 src/canvas/canvas_colours.rs              | 21 +++-----
 src/canvas/canvas_colours/colour_utils.rs | 60 ++++-------------------
 src/canvas/widgets/cpu_graph.rs           | 15 +++---
 src/constants.rs                          |  2 -
 6 files changed, 27 insertions(+), 74 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8c25a651..9ddeff40 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -68,6 +68,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - [#296](https://github.com/ClementTsang/bottom/pull/296): Removes an accidental extra comma in one of the headers in the disk widget.
 
+- [#308](https://github.com/ClementTsang/bottom/pull/308): Removes the automatically generated CPU colours method.
+
 ## [0.4.7] - 2020-08-26
 
 ### Bug Fixes
diff --git a/src/canvas.rs b/src/canvas.rs
index 7af29f33..9bd4f96d 100644
--- a/src/canvas.rs
+++ b/src/canvas.rs
@@ -197,7 +197,6 @@ impl Painter {
         } else {
             painter.generate_colour_scheme(colour_scheme)?;
         }
-        painter.colours.generate_remaining_cpu_colours();
         painter.complete_painter_init();
 
         Ok(painter)
diff --git a/src/canvas/canvas_colours.rs b/src/canvas/canvas_colours.rs
index a42eb88c..fd8d4eb0 100644
--- a/src/canvas/canvas_colours.rs
+++ b/src/canvas/canvas_colours.rs
@@ -1,4 +1,4 @@
-use crate::{constants::*, options::ConfigColours, utils::error};
+use crate::{options::ConfigColours, utils::error};
 use anyhow::Context;
 use colour_utils::*;
 use tui::style::{Color, Style};
@@ -48,7 +48,7 @@ impl Default for CanvasColours {
             total_tx_style: Style::default().fg(STANDARD_FOURTH_COLOUR),
             all_colour_style: Style::default().fg(ALL_COLOUR),
             avg_colour_style: Style::default().fg(AVG_COLOUR),
-            cpu_colour_styles: Vec::new(),
+            cpu_colour_styles: colour_utils::get_default_cpu_colours(),
             border_style: Style::default().fg(text_colour),
             highlighted_border_style: Style::default().fg(STANDARD_HIGHLIGHT_COLOUR),
             text_style: Style::default().fg(text_colour),
@@ -248,22 +248,13 @@ impl CanvasColours {
     }
 
     pub fn set_cpu_colours(&mut self, colours: &[String]) -> error::Result<()> {
-        let max_amount = std::cmp::min(colours.len(), NUM_COLOURS);
-        for (itx, colour) in colours.iter().enumerate() {
-            if itx >= max_amount {
-                break;
-            }
-            self.cpu_colour_styles.push(get_style_from_config(colour)?);
-        }
+        self.cpu_colour_styles = colours
+            .iter()
+            .map(|colour| get_style_from_config(colour))
+            .collect::<error::Result<Vec<Style>>>()?;
         Ok(())
     }
 
-    pub fn generate_remaining_cpu_colours(&mut self) {
-        let remaining_num_colours = NUM_COLOURS.saturating_sub(self.cpu_colour_styles.len());
-        self.cpu_colour_styles
-            .extend(gen_n_styles(remaining_num_colours));
-    }
-
     pub fn set_scroll_entry_text_color(&mut self, colour: &str) -> error::Result<()> {
         self.currently_selected_text_colour = get_colour_from_config(colour)?;
         self.currently_selected_text_style = Style::default()
diff --git a/src/canvas/canvas_colours/colour_utils.rs b/src/canvas/canvas_colours/colour_utils.rs
index d104d483..048f9317 100644
--- a/src/canvas/canvas_colours/colour_utils.rs
+++ b/src/canvas/canvas_colours/colour_utils.rs
@@ -3,9 +3,8 @@ use std::collections::HashMap;
 
 use tui::style::{Color, Style};
 
-use crate::utils::{error, gen_util::*};
+use crate::utils::error;
 
-const GOLDEN_RATIO: f32 = 0.618_034;
 // Approx, good enough for use (also Clippy gets mad if it's too long)
 pub const STANDARD_FIRST_COLOUR: Color = Color::LightMagenta;
 pub const STANDARD_SECOND_COLOUR: Color = Color::LightYellow;
@@ -41,60 +40,21 @@ lazy_static! {
     .collect();
 }
 
-/// Generates random colours.  Strategy found from
-/// https://martin.ankerl.com/2009/12/09/how-to-create-random-colors-programmatically/
-pub fn gen_n_styles(num_to_gen: usize) -> Vec<Style> {
-    fn gen_hsv(h: f32) -> f32 {
-        let new_val = h + GOLDEN_RATIO;
-        if new_val > 1.0 {
-            new_val.fract()
-        } else {
-            new_val
-        }
-    }
-    /// This takes in an h, s, and v value of range [0, 1]
-    /// For explanation of what this does, see
-    /// https://en.wikipedia.org/wiki/HSL_and_HSV#HSV_to_RGB_alternative
-    fn hsv_to_rgb(hue: f32, saturation: f32, value: f32) -> (u8, u8, u8) {
-        fn hsv_helper(num: u32, hu: f32, sat: f32, val: f32) -> f32 {
-            let k = (num as f32 + hu * 6.0) % 6.0;
-            val - val * sat * float_max(float_min(k, float_min(4.1 - k, 1.1)), 0.0)
-        }
-
-        (
-            (hsv_helper(5, hue, saturation, value) * 255.0) as u8,
-            (hsv_helper(3, hue, saturation, value) * 255.0) as u8,
-            (hsv_helper(1, hue, saturation, value) * 255.0) as u8,
-        )
-    }
-
-    // Generate colours
-    // Why do we need so many colours?  Because macOS default terminal
-    // throws a tantrum if you don't give it supported colours, but so
-    // does PowerShell with some colours (Magenta and Yellow)!
-    let mut colour_vec: Vec<Style> = vec![
-        Style::default().fg(STANDARD_FIRST_COLOUR),
-        Style::default().fg(STANDARD_SECOND_COLOUR),
-        Style::default().fg(STANDARD_THIRD_COLOUR),
-        Style::default().fg(STANDARD_FOURTH_COLOUR),
+/// We take basically no chances with this.  If the user wants prettier colours, they can
+/// set it on their own - unfortunately, supported colour detection is kinda a PITA.
+pub fn get_default_cpu_colours() -> Vec<Style> {
+    vec![
+        Style::default().fg(Color::LightMagenta),
+        Style::default().fg(Color::LightYellow),
+        Style::default().fg(Color::LightCyan),
+        Style::default().fg(Color::LightGreen),
         Style::default().fg(Color::LightBlue),
         Style::default().fg(Color::LightRed),
         Style::default().fg(Color::Cyan),
         Style::default().fg(Color::Green),
         Style::default().fg(Color::Blue),
         Style::default().fg(Color::Red),
-    ];
-
-    let mut h: f32 = 0.4; // We don't need random colours... right?
-    if num_to_gen - 10 > 0 {
-        for _i in 0..(num_to_gen - 10) {
-            h = gen_hsv(h);
-            let result = hsv_to_rgb(h, 0.5, 0.95);
-            colour_vec.push(Style::default().fg(Color::Rgb(result.0, result.1, result.2)));
-        }
-    }
-
-    colour_vec
+    ]
 }
 
 pub fn convert_hex_to_color(hex: &str) -> error::Result<Color> {
diff --git a/src/canvas/widgets/cpu_graph.rs b/src/canvas/widgets/cpu_graph.rs
index e03c9032..571af6ca 100644
--- a/src/canvas/widgets/cpu_graph.rs
+++ b/src/canvas/widgets/cpu_graph.rs
@@ -217,7 +217,7 @@ impl CpuGraphWidget for Painter {
                     .style(if show_avg_cpu && current_scroll_position == AVG_POSITION {
                         self.colours.avg_colour_style
                     } else {
-                        self.colours.cpu_colour_styles[cpu_widget_state
+                        self.colours.cpu_colour_styles[(cpu_widget_state
                             .scroll_state
                             .current_scroll_position
                             - 1 // Because of the all position
@@ -225,7 +225,8 @@ impl CpuGraphWidget for Painter {
                                 AVG_POSITION
                             } else {
                                 0
-                            }) % self.colours.cpu_colour_styles.len()]
+                            }))
+                            % self.colours.cpu_colour_styles.len()]
                     })
                     .data(&cpu.cpu_data[..])
                     .graph_type(tui::widgets::GraphType::Line)]
@@ -375,14 +376,16 @@ impl CpuGraphWidget for Painter {
                             if itx + start_position == AVG_POSITION {
                                 self.colours.avg_colour_style
                             } else {
-                                self.colours.cpu_colour_styles[itx + start_position
+                                self.colours.cpu_colour_styles[(itx + start_position
                                     - AVG_POSITION
-                                    - 1 % self.colours.cpu_colour_styles.len()]
+                                    - 1)
+                                    % self.colours.cpu_colour_styles.len()]
                             }
                         } else {
-                            self.colours.cpu_colour_styles[itx + start_position
+                            self.colours.cpu_colour_styles[(itx + start_position
                                 - ALL_POSITION
-                                - 1 % self.colours.cpu_colour_styles.len()]
+                                - 1)
+                                % self.colours.cpu_colour_styles.len()]
                         },
                     ))
                 }
diff --git a/src/constants.rs b/src/constants.rs
index 0bccec77..1f0c9544 100644
--- a/src/constants.rs
+++ b/src/constants.rs
@@ -18,8 +18,6 @@ pub const TICK_RATE_IN_MILLISECONDS: u64 = 200;
 // How fast the screen refreshes
 pub const DEFAULT_REFRESH_RATE_IN_MILLISECONDS: u64 = 1000;
 pub const MAX_KEY_TIMEOUT_IN_MILLISECONDS: u64 = 1000;
-// Number of colours to generate for the CPU chart/table
-pub const NUM_COLOURS: usize = 256;
 
 // Limits for when we should stop showing table gaps/labels (anything less means not shown)
 pub const TABLE_GAP_HEIGHT_LIMIT: u16 = 7;