refactor: update process widget code #729

ClementTsang · 2022-05-15T08:41:45Z

Description

A description of the change and what it does. If relevant (such as any change that modifies the UI), please provide screenshots of the change:

Refactors how the process widget to work with the (maybe better) consolidated tables code, as well as refactoring a bunch of old logic.

Testing

If relevant, please state how this was tested. All changes must be tested to work:

Please also indicate which platforms were tested. All platforms directly affected by the change must be tested:

Windows
macOS
Linux

Checklist

If relevant, ensure the following have been met:

Areas your change affects have been linted using rustfmt (cargo fmt)
The change has been tested and doesn't appear to cause any unintended breakage
Documentation has been added/updated if needed (README.md, help menu, etc.)
The pull request passes the provided CI pipeline
There are no merge conflicts

codecov-commenter · 2022-05-15T08:47:25Z

Codecov Report

Merging #729 (cc6d7b8) into consolidate_tables (69ec526) will increase coverage by 2.95%.
The diff coverage is 29.51%.

@@                  Coverage Diff                   @@
##           consolidate_tables     #729      +/-   ##
======================================================
+ Coverage               21.23%   24.18%   +2.95%     
======================================================
  Files                      54       56       +2     
  Lines                   13606    12918     -688     
======================================================
+ Hits                     2889     3124     +235     
+ Misses                  10717     9794     -923

Impacted Files	Coverage Δ
src/app.rs	`0.04% <0.00%> (+<0.01%)`	⬆️
src/app/data_harvester.rs	`0.00% <0.00%> (ø)`
src/app/data_harvester/network/heim.rs	`0.00% <ø> (ø)`
src/app/data_harvester/processes/linux.rs	`19.83% <0.00%> (-0.68%)`	⬇️
src/app/data_harvester/processes/mod.rs	`0.00% <0.00%> (-83.34%)`	⬇️
src/app/data_harvester/processes/unix.rs	`6.66% <ø> (ø)`
src/app/query.rs	`0.00% <0.00%> (ø)`
src/canvas/components/time_graph.rs	`69.03% <0.00%> (ø)`
src/canvas/dialogs/dd_dialog.rs	`0.00% <0.00%> (ø)`
src/canvas/widgets/battery_display.rs	`0.00% <0.00%> (ø)`
... and 33 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69ec526...cc6d7b8. Read the comment docs.

src/canvas.rs

src/utils/gen_util.rs

src/data_conversion.rs

src/lib.rs

src/app/query.rs

src/canvas/drawing_utils.rs

ClementTsang · 2022-05-15T22:56:54Z

src/canvas/dialogs/dd_dialog.rs

+                    if app_state
+                        .proc_state
+                        .widget_states
+                        .get(&app_state.current_widget.widget_id)
+                        .map(|p| matches!(p.mode, ProcWidgetMode::Grouped))
+                        .unwrap_or(false)
+                    {


Another example of how its a bit annoying how its currently done with a mapping, where it needs an unwrap in the case it "fails". Ideally bind this to the PWS.

src/canvas/drawing_utils.rs

ClementTsang · 2022-05-15T23:09:00Z

src/canvas/widgets/process_table.rs

+            // TODO: [Refactor] This is an ugly hack to add the disabled style...
+            // this could be solved by storing style locally to the widget.
+            for row in &mut proc_widget_state.table_data.data {
+                if let TableRow::Styled(_, style) = row {
+                    *style = style.patch(self.colours.disabled_text_style);


Not a fan of this but its fine for now...

ClementTsang · 2022-05-15T23:10:56Z

src/canvas/components/text_table.rs

@@ -24,7 +30,7 @@ pub struct TextTableTitle<'a> {

 pub struct TextTable<'a> {
    pub table_gap: u16,
-    pub is_force_redraw: bool,
+    pub is_force_redraw: bool, // TODO: Is this force redraw thing needed? Or is there a better way?


Not a fan of this bit. Fix in the future!

ClementTsang · 2022-05-15T23:26:29Z

src/app/widgets/process_widget.rs

+    const CPU_PERCENT: CellContent = CellContent::Simple(Cow::Borrowed("CPU%"));
+    const MEM_PERCENT: CellContent = CellContent::Simple(Cow::Borrowed("Mem%"));
+    const MEM: CellContent = CellContent::Simple(Cow::Borrowed("Mem"));
+    const READS_PER_SECOND: CellContent = CellContent::Simple(Cow::Borrowed("R/s"));
+    const WRITES_PER_SECOND: CellContent = CellContent::Simple(Cow::Borrowed("W/s"));
+    const TOTAL_READ: CellContent = CellContent::Simple(Cow::Borrowed("T.Read"));
+    const TOTAL_WRITE: CellContent = CellContent::Simple(Cow::Borrowed("T.Write"));
+    const STATE: CellContent = CellContent::Simple(Cow::Borrowed("State"));
+    const PROCESS_NAME: CellContent = CellContent::Simple(Cow::Borrowed("Name"));
+    const COMMAND: CellContent = CellContent::Simple(Cow::Borrowed("Command"));
+    const PID: CellContent = CellContent::Simple(Cow::Borrowed("PID"));
+    const COUNT: CellContent = CellContent::Simple(Cow::Borrowed("Count"));
+    const USER: CellContent = CellContent::Simple(Cow::Borrowed("User"));
+
+    const SHORTCUT_CPU_PERCENT: CellContent = CellContent::Simple(Cow::Borrowed("CPU%(c)"));
+    const SHORTCUT_MEM_PERCENT: CellContent = CellContent::Simple(Cow::Borrowed("Mem%(m)"));
+    const SHORTCUT_MEM: CellContent = CellContent::Simple(Cow::Borrowed("Mem(m)"));
+    const SHORTCUT_PROCESS_NAME: CellContent = CellContent::Simple(Cow::Borrowed("Name(n)"));
+    const SHORTCUT_COMMAND: CellContent = CellContent::Simple(Cow::Borrowed("Command(n)"));
+    const SHORTCUT_PID: CellContent = CellContent::Simple(Cow::Borrowed("PID(p)"));


Kinda gross. Feel like there's a better way to do this...

src/app/widgets/process_widget.rs

ClementTsang · 2022-05-15T23:30:02Z

src/app/widgets/process_widget.rs

+        if is_disabled {
+            TableRow::Styled(contents, tui::style::Style::default())
+        } else {
+            TableRow::Raw(contents)


Part of the ugly hack for disabled entries.

src/app/widgets/process_widget.rs

src/app/states.rs

A bunch of work towards also refactoring how the process widget gathers and converts data.

Bugs squashed: - Incorrect column sizing for flex cases - Case where the sort menu bounds were still existing despite being hidden - Proc widget not actually taking into account the calculated row widths in some cases during data conversion.

ClementTsang force-pushed the refactor_process_widget_into_tables branch from 385f205 to 0e41747 Compare May 15, 2022 08:42

ClementTsang force-pushed the consolidate_tables branch from bcf6cc5 to 69ec526 Compare May 15, 2022 09:03

ClementTsang force-pushed the refactor_process_widget_into_tables branch from eecf396 to 129eb97 Compare May 15, 2022 09:09

ClementTsang mentioned this pull request May 15, 2022

refactor: consolidate tables #730

Merged

15 tasks

ClementTsang force-pushed the refactor_process_widget_into_tables branch 5 times, most recently from 79cec5d to c1d3b4e Compare May 15, 2022 22:31