More basic cleaning (#74)

* Add htop link.

* Move dd and help dialog into separate files

* Move to  folder

* Properly show error message if DD fails on macOS and linux.

README.md

@@ -232,7 +232,7 @@ Thanks to those who have contributed:
 
 - This project is very much inspired by both [gotop](https://github.com/cjbassi/gotop) and [gtop](https://github.com/aksakalli/gtop).
 
-- Basic mode inspired by htop's design.
+- Basic mode inspired by [htop's](https://hisham.hm/htop/) design.
 
 - This application was written with the following libraries, and would otherwise not be possible:
 

src/app/process_killer.rs

@@ -35,7 +35,12 @@ impl Process {
 /// Kills a process, given a PID.
 pub fn kill_process_given_pid(pid: u32) -> crate::utils::error::Result<()> {
     if cfg!(target_os = "linux") || cfg!(target_os = "macos") {
-        Command::new("kill").arg(pid.to_string()).output()?;
+        let output = Command::new("kill").arg(pid.to_string()).output()?;
+        if !(output.status).success() {
+            return Err(BottomError::GenericError(
+                std::str::from_utf8(&output.stderr)?.to_string(),
+            ));
+        }
     } else if cfg!(target_os = "windows") {
         #[cfg(target_os = "windows")]
         {

src/canvas.rs

@@ -3,13 +3,14 @@ use std::collections::HashMap;
 
 use tui::{
     backend::Backend,
-    layout::{Alignment, Constraint, Direction, Layout, Rect},
+    layout::{Constraint, Direction, Layout, Rect},
     terminal::Frame,
-    widgets::{Block, Borders, Paragraph, Text, Widget},
+    widgets::Text,
     Terminal,
 };
 
 use canvas_colours::*;
+use dialogs::*;
 use widgets::*;
 
 use crate::{
@@ -20,6 +21,7 @@ use crate::{
 };
 
 mod canvas_colours;
+mod dialogs;
 mod drawing_utils;
 mod widgets;
 
@@ -147,8 +149,6 @@ impl Painter {
         terminal.autoresize()?;
         terminal.draw(|mut f| {
             if app_state.help_dialog_state.is_showing_help {
-                // Only for the help
-
                 // TODO: [RESIZE] Scrolling dialog boxes is ideal.  This is currently VERY temporary!
                 // The width is currently not good and can wrap... causing this to not go so well!
                 let gen_help_len = GENERAL_HELP_TEXT.len() as u16 + 3;
@@ -186,37 +186,7 @@ impl Painter {
                     )
                     .split(vertical_dialog_chunk[1]);
 
-                const HELP_BASE: &str =
-                    " Help ── 1: General ─── 2: Processes ─── 3: Search ─── Esc to close ";
-                let repeat_num = max(
-                    0,
-                    middle_dialog_chunk[1].width as i32 - HELP_BASE.chars().count() as i32 - 2,
-                );
-                let help_title = format!(
-                    " Help ─{}─ 1: General ─── 2: Processes ─── 3: Search ─── Esc to close ",
-                    "─".repeat(repeat_num as usize)
-                );
-
-                Paragraph::new(
-                    match app_state.help_dialog_state.current_category {
-                        app::AppHelpCategory::General => &self.styled_general_help_text,
-                        app::AppHelpCategory::Process => &self.styled_process_help_text,
-                        app::AppHelpCategory::Search => &self.styled_search_help_text,
-                    }
-                    .iter(),
-                )
-                .block(
-                    Block::default()
-                        .title(&help_title)
-                        .title_style(self.colours.border_style)
-                        .style(self.colours.border_style)
-                        .borders(Borders::ALL)
-                        .border_style(self.colours.border_style),
-                )
-                .style(self.colours.text_style)
-                .alignment(Alignment::Left)
-                .wrap(true)
-                .render(&mut f, middle_dialog_chunk[1]);
+                self.draw_help_dialog(&mut f, app_state, middle_dialog_chunk[1]);
             } else if app_state.delete_dialog_state.is_showing_dd {
                 let bordering = (max(0, f.size().height as i64 - 7) as u16) / 2;
                 let vertical_dialog_chunk = Layout::default()
@@ -253,105 +223,13 @@ impl Painter {
                     .split(vertical_dialog_chunk[1]);
 
                 if let Some(dd_err) = &app_state.dd_err {
-                    let dd_text = [Text::raw(format!(
-                        "\nFailure to properly kill the process - {}",
-                        dd_err
-                    ))];
-
-                    const ERROR_BASE: &str = " Error ── Esc to close ";
-                    let repeat_num = max(
-                        0,
-                        middle_dialog_chunk[1].width as i32 - ERROR_BASE.chars().count() as i32 - 2,
-                    );
-                    let error_title =
-                        format!(" Error ─{}─ Esc to close ", "─".repeat(repeat_num as usize));
-
-                    Paragraph::new(dd_text.iter())
-                        .block(
-                            Block::default()
-                                .title(&error_title)
-                                .title_style(self.colours.border_style)
-                                .style(self.colours.border_style)
-                                .borders(Borders::ALL)
-                                .border_style(self.colours.border_style),
-                        )
-                        .style(self.colours.text_style)
-                        .alignment(Alignment::Center)
-                        .wrap(true)
-                        .render(&mut f, middle_dialog_chunk[1]);
-                } else if let Some(to_kill_processes) = app_state.get_to_delete_processes() {
-                    if let Some(first_pid) = to_kill_processes.1.first() {
-                        let dd_text = vec![
-                            if app_state.is_grouped() {
-                                if to_kill_processes.1.len() != 1 {
-                                    Text::raw(format!(
-                                        "\nKill {} processes with the name {}?",
-                                        to_kill_processes.1.len(),
-                                        to_kill_processes.0
-                                    ))
-                                } else {
-                                    Text::raw(format!(
-                                        "\nKill {} process with the name {}?",
-                                        to_kill_processes.1.len(),
-                                        to_kill_processes.0
-                                    ))
-                                }
-                            } else {
-                                Text::raw(format!(
-                                    "\nKill process {} with PID {}?",
-                                    to_kill_processes.0, first_pid
-                                ))
-                            },
-                            Text::raw("\n\n"),
-                            if app_state.delete_dialog_state.is_on_yes {
-                                Text::styled("Yes", self.colours.currently_selected_text_style)
-                            } else {
-                                Text::raw("Yes")
-                            },
-                            Text::raw("                 "),
-                            if app_state.delete_dialog_state.is_on_yes {
-                                Text::raw("No")
-                            } else {
-                                Text::styled("No", self.colours.currently_selected_text_style)
-                            },
-                        ];
-
-                        const DD_BASE: &str = " Confirm Kill Process ── Esc to close ";
-                        let repeat_num = max(
-                            0,
-                            middle_dialog_chunk[1].width as i32
-                                - DD_BASE.chars().count() as i32
-                                - 2,
-                        );
-                        let dd_title = format!(
-                            " Confirm Kill Process ─{}─ Esc to close ",
-                            "─".repeat(repeat_num as usize)
-                        );
-
-                        Paragraph::new(dd_text.iter())
-                            .block(
-                                Block::default()
-                                    .title(&dd_title)
-                                    .title_style(self.colours.border_style)
-                                    .style(self.colours.border_style)
-                                    .borders(Borders::ALL)
-                                    .border_style(self.colours.border_style),
-                            )
-                            .style(self.colours.text_style)
-                            .alignment(Alignment::Center)
-                            .wrap(true)
-                            .render(&mut f, middle_dialog_chunk[1]);
-                    } else {
-                        // This is a bit nasty, but it works well... I guess.
-                        app_state.delete_dialog_state.is_showing_dd = false;
-                    }
+                    self.draw_dd_error_dialog(&mut f, dd_err, middle_dialog_chunk[1]);
                 } else {
                     // This is a bit nasty, but it works well... I guess.
-                    app_state.delete_dialog_state.is_showing_dd = false;
+                    app_state.delete_dialog_state.is_showing_dd =
+                        self.draw_dd_dialog(&mut f, app_state, middle_dialog_chunk[1]);
                 }
             } else if app_state.is_expanded {
-                // TODO: [REF] we should combine this with normal drawing tbh
-
                 let rect = Layout::default()
                     .margin(1)
                     .constraints([Constraint::Percentage(100)].as_ref())

src/canvas/dialogs.rs

@@ -0,0 +1,5 @@
+pub mod dd_dialog;
+pub mod help_dialog;
+
+pub use dd_dialog::KillDialog;
+pub use help_dialog::HelpDialog;

src/canvas/dialogs/dd_dialog.rs

@@ -0,0 +1,123 @@
+use std::cmp::max;
+
+use tui::{
+    backend::Backend,
+    layout::{Alignment, Rect},
+    terminal::Frame,
+    widgets::{Block, Borders, Paragraph, Text, Widget},
+};
+
+use crate::{app::App, canvas::Painter};
+
+const DD_BASE: &str = " Confirm Kill Process ── Esc to close ";
+const DD_ERROR_BASE: &str = " Error ── Esc to close ";
+
+pub trait KillDialog {
+    fn draw_dd_dialog<B: Backend>(
+        &self, f: &mut Frame<'_, B>, app_state: &mut App, draw_loc: Rect,
+    ) -> bool;
+
+    fn draw_dd_error_dialog<B: Backend>(&self, f: &mut Frame<'_, B>, dd_err: &str, draw_loc: Rect);
+}
+
+impl KillDialog for Painter {
+    fn draw_dd_dialog<B: Backend>(
+        &self, f: &mut Frame<'_, B>, app_state: &mut App, draw_loc: Rect,
+    ) -> bool {
+        if let Some(to_kill_processes) = app_state.get_to_delete_processes() {
+            if let Some(first_pid) = to_kill_processes.1.first() {
+                let dd_text = vec![
+                    if app_state.is_grouped() {
+                        if to_kill_processes.1.len() != 1 {
+                            Text::raw(format!(
+                                "\nKill {} processes with the name {}?",
+                                to_kill_processes.1.len(),
+                                to_kill_processes.0
+                            ))
+                        } else {
+                            Text::raw(format!(
+                                "\nKill {} process with the name {}?",
+                                to_kill_processes.1.len(),
+                                to_kill_processes.0
+                            ))
+                        }
+                    } else {
+                        Text::raw(format!(
+                            "\nKill process {} with PID {}?",
+                            to_kill_processes.0, first_pid
+                        ))
+                    },
+                    Text::raw("\n\n"),
+                    if app_state.delete_dialog_state.is_on_yes {
+                        Text::styled("Yes", self.colours.currently_selected_text_style)
+                    } else {
+                        Text::raw("Yes")
+                    },
+                    Text::raw("                 "),
+                    if app_state.delete_dialog_state.is_on_yes {
+                        Text::raw("No")
+                    } else {
+                        Text::styled("No", self.colours.currently_selected_text_style)
+                    },
+                ];
+
+                let repeat_num = max(
+                    0,
+                    draw_loc.width as i32 - DD_BASE.chars().count() as i32 - 2,
+                );
+                let dd_title = format!(
+                    " Confirm Kill Process ─{}─ Esc to close ",
+                    "─".repeat(repeat_num as usize)
+                );
+
+                Paragraph::new(dd_text.iter())
+                    .block(
+                        Block::default()
+                            .title(&dd_title)
+                            .title_style(self.colours.border_style)
+                            .style(self.colours.border_style)
+                            .borders(Borders::ALL)
+                            .border_style(self.colours.border_style),
+                    )
+                    .style(self.colours.text_style)
+                    .alignment(Alignment::Center)
+                    .wrap(true)
+                    .render(f, draw_loc);
+
+                return true;
+            }
+        }
+
+        // Currently we just return "false" if things go wrong finding
+        // the process or a first PID (if an error arises it should be caught).
+        // I don't really like this, and I find it ugly, but it works for now.
+        false
+    }
+
+    fn draw_dd_error_dialog<B: Backend>(&self, f: &mut Frame<'_, B>, dd_err: &str, draw_loc: Rect) {
+        let dd_text = [Text::raw(format!(
+            "\nFailure to properly kill the process - {}",
+            dd_err
+        ))];
+
+        let repeat_num = max(
+            0,
+            draw_loc.width as i32 - DD_ERROR_BASE.chars().count() as i32 - 2,
+        );
+        let error_title = format!(" Error ─{}─ Esc to close ", "─".repeat(repeat_num as usize));
+
+        Paragraph::new(dd_text.iter())
+            .block(
+                Block::default()
+                    .title(&error_title)
+                    .title_style(self.colours.border_style)
+                    .style(self.colours.border_style)
+                    .borders(Borders::ALL)
+                    .border_style(self.colours.border_style),
+            )
+            .style(self.colours.text_style)
+            .alignment(Alignment::Center)
+            .wrap(true)
+            .render(f, draw_loc);
+    }
+}

src/canvas/dialogs/help_dialog.rs

@@ -0,0 +1,57 @@
+use std::cmp::max;
+
+use tui::{
+    backend::Backend,
+    layout::{Alignment, Rect},
+    terminal::Frame,
+    widgets::{Block, Borders, Paragraph, Widget},
+};
+
+use crate::{
+    app::{App, AppHelpCategory},
+    canvas::Painter,
+};
+
+const HELP_BASE: &str = " Help ── 1: General ─── 2: Processes ─── 3: Search ─── Esc to close ";
+
+pub trait HelpDialog {
+    fn draw_help_dialog<B: Backend>(
+        &self, f: &mut Frame<'_, B>, app_state: &mut App, draw_loc: Rect,
+    );
+}
+
+impl HelpDialog for Painter {
+    fn draw_help_dialog<B: Backend>(
+        &self, f: &mut Frame<'_, B>, app_state: &mut App, draw_loc: Rect,
+    ) {
+        let repeat_num = max(
+            0,
+            draw_loc.width as i32 - HELP_BASE.chars().count() as i32 - 2,
+        );
+        let help_title = format!(
+            " Help ─{}─ 1: General ─── 2: Processes ─── 3: Search ─── Esc to close ",
+            "─".repeat(repeat_num as usize)
+        );
+
+        Paragraph::new(
+            match app_state.help_dialog_state.current_category {
+                AppHelpCategory::General => &self.styled_general_help_text,
+                AppHelpCategory::Process => &self.styled_process_help_text,
+                AppHelpCategory::Search => &self.styled_search_help_text,
+            }
+            .iter(),
+        )
+        .block(
+            Block::default()
+                .title(&help_title)
+                .title_style(self.colours.border_style)
+                .style(self.colours.border_style)
+                .borders(Borders::ALL)
+                .border_style(self.colours.border_style),
+        )
+        .style(self.colours.text_style)
+        .alignment(Alignment::Left)
+        .wrap(true)
+        .render(f, draw_loc);
+    }
+}

src/utils/error.rs

@@ -20,6 +20,8 @@ pub enum BottomError {
     FernError(String),
     /// An error to represent errors with the config.
     ConfigError(String),
+    /// An error to represent errors with converting between data types.
+    ConversionError(String),
 }
 
 impl std::fmt::Display for BottomError {
@@ -42,6 +44,9 @@ impl std::fmt::Display for BottomError {
             BottomError::ConfigError(ref message) => {
                 write!(f, "Invalid config file error: {}", message)
             }
+            BottomError::ConversionError(ref message) => {
+                write!(f, "Unable to convert: {}", message)
+            }
         }
     }
 }
@@ -87,3 +92,9 @@ impl From<fern::InitError> for BottomError {
         BottomError::FernError(err.to_string())
     }
 }
+
+impl From<std::str::Utf8Error> for BottomError {
+    fn from(err: std::str::Utf8Error) -> Self {
+        BottomError::ConversionError(err.to_string())
+    }
+}