From 78d0e1d0b8d0145310ae7cc73cf2f9322c4243c2 Mon Sep 17 00:00:00 2001
From: Reilly Wood <26268125+rgwood@users.noreply.github.com>
Date: Thu, 28 Sep 2023 18:17:56 -0700
Subject: [PATCH] `explore`: highlight selected cell using background colour
 instead of cursor (#10533)

More incremental `explore` improvements!

This PR removes the `show_cursor` config from the `explore` command, in
favour of always using the background colour to highlight the selected
cell. I believe this is a better default and I'd like to remove the
`show_cursor` functionality entirely as part of the effort to simplify
`explore`.

The style for selected cells is still configurable. I went with light
blue for the default background colour, it looks OK to me.

## Before:

![Screenshot from 2023-09-27
08-51-03](https://github.com/nushell/nushell/assets/26268125/798636be-a4ea-467f-b852-c0e929e4aa9d)


## After:

![Screenshot from 2023-09-27
08-50-59](https://github.com/nushell/nushell/assets/26268125/c88662e7-05b5-42a7-bf30-b03c70fba79d)
---
 crates/nu-explore/src/commands/table.rs       |  7 -----
 crates/nu-explore/src/explore.rs              |  3 --
 crates/nu-explore/src/views/record/mod.rs     | 30 +++++++++----------
 .../src/sample_config/default_config.nu       |  3 +-
 4 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/crates/nu-explore/src/commands/table.rs b/crates/nu-explore/src/commands/table.rs
index 7ebcde3eb5..21a2237499 100644
--- a/crates/nu-explore/src/commands/table.rs
+++ b/crates/nu-explore/src/commands/table.rs
@@ -33,7 +33,6 @@ struct TableSettings {
     selected_cell_s: Option<Style>,
     selected_row_s: Option<Style>,
     selected_column_s: Option<Style>,
-    show_cursor: Option<bool>,
     padding_column_left: Option<usize>,
     padding_column_right: Option<usize>,
     padding_index_left: Option<usize>,
@@ -97,8 +96,6 @@ impl ViewCommand for TableCmd {
             ConfigOption::boolean(":table group", "Lines are lines", "table.line_shift"),
             ConfigOption::boolean(":table group", "Lines are lines", "table.line_index"),
 
-            ConfigOption::boolean(":table group", "Show cursor", "table.show_cursor"),
-
             ConfigOption::new(":table group", "Color of selected cell", "table.selected_cell", default_color_list()),
             ConfigOption::new(":table group", "Color of selected row", "table.selected_row", default_color_list()),
             ConfigOption::new(":table group", "Color of selected column", "table.selected_column", default_color_list()),
@@ -164,10 +161,6 @@ impl ViewCommand for TableCmd {
             view.set_line_trailing(true);
         }
 
-        if self.settings.show_cursor.unwrap_or(false) {
-            view.show_cursor(true);
-        }
-
         if let Some(style) = self.settings.selected_cell_s {
             view.set_style_selected_cell(style);
         }
diff --git a/crates/nu-explore/src/explore.rs b/crates/nu-explore/src/explore.rs
index 988227e3cc..eacebf4da0 100644
--- a/crates/nu-explore/src/explore.rs
+++ b/crates/nu-explore/src/explore.rs
@@ -211,8 +211,6 @@ fn prepare_default_config(config: &mut HashMap<String, Value>) {
 
     const TABLE_LINE_SHIFT: bool = true;
 
-    const TABLE_SELECT_CURSOR: bool = true;
-
     const TABLE_SELECT_CELL: Style = color(None, None);
 
     const TABLE_SELECT_ROW: Style = color(None, None);
@@ -251,7 +249,6 @@ fn prepare_default_config(config: &mut HashMap<String, Value>) {
         insert_style(&mut hm, "selected_cell", TABLE_SELECT_CELL);
         insert_style(&mut hm, "selected_row", TABLE_SELECT_ROW);
         insert_style(&mut hm, "selected_column", TABLE_SELECT_COLUMN);
-        insert_bool(&mut hm, "cursor", TABLE_SELECT_CURSOR);
         insert_bool(&mut hm, "line_head_top", TABLE_LINE_HEADER_TOP);
         insert_bool(&mut hm, "line_head_bottom", TABLE_LINE_HEADER_BOTTOM);
         insert_bool(&mut hm, "line_shift", TABLE_LINE_SHIFT);
diff --git a/crates/nu-explore/src/views/record/mod.rs b/crates/nu-explore/src/views/record/mod.rs
index 80c125d785..a6cf19589b 100644
--- a/crates/nu-explore/src/views/record/mod.rs
+++ b/crates/nu-explore/src/views/record/mod.rs
@@ -75,10 +75,6 @@ impl<'a> RecordView<'a> {
         self.theme.cursor.selected_column = Some(style)
     }
 
-    pub fn show_cursor(&mut self, b: bool) {
-        self.theme.cursor.show_cursor = b;
-    }
-
     pub fn set_line_head_top(&mut self, b: bool) {
         self.theme.table.header_top = b;
     }
@@ -682,27 +678,33 @@ fn convert_records_to_string(
 }
 
 fn highlight_cell(f: &mut Frame, area: Rect, info: ElementInfo, theme: &CursorStyle) {
+    // highlight selected column
     if let Some(style) = theme.selected_column {
         let highlight_block = Block::default().style(nu_style_to_tui(style));
         let area = Rect::new(info.area.x, area.y, info.area.width, area.height);
         f.render_widget(highlight_block.clone(), area);
     }
 
+    // highlight selected row
     if let Some(style) = theme.selected_row {
         let highlight_block = Block::default().style(nu_style_to_tui(style));
         let area = Rect::new(area.x, info.area.y, area.width, 1);
         f.render_widget(highlight_block.clone(), area);
     }
 
-    if let Some(style) = theme.selected_cell {
-        let highlight_block = Block::default().style(nu_style_to_tui(style));
-        let area = Rect::new(info.area.x, info.area.y, info.area.width, 1);
-        f.render_widget(highlight_block.clone(), area);
-    }
-
-    if theme.show_cursor {
-        f.set_cursor(info.area.x, info.area.y);
-    }
+    // highlight selected cell
+    let cell_style = match theme.selected_cell {
+        Some(s) => s,
+        None => {
+            let mut style = nu_ansi_term::Style::new();
+            // light blue chosen somewhat arbitrarily, looks OK but I'm not set on it
+            style.background = Some(nu_ansi_term::Color::LightBlue);
+            style
+        }
+    };
+    let highlight_block = Block::default().style(nu_style_to_tui(cell_style));
+    let area = Rect::new(info.area.x, info.area.y, info.area.width, 1);
+    f.render_widget(highlight_block.clone(), area)
 }
 
 fn build_last_value(v: &RecordView) -> Value {
@@ -852,7 +854,6 @@ fn theme_from_config(config: &ConfigMap) -> TableTheme {
     theme.cursor.selected_cell = colors.get("selected_cell").cloned();
     theme.cursor.selected_row = colors.get("selected_row").cloned();
     theme.cursor.selected_column = colors.get("selected_column").cloned();
-    theme.cursor.show_cursor = config_get_bool(config, "show_cursor", true);
 
     theme.table.header_top = config_get_bool(config, "line_head_top", true);
     theme.table.header_bottom = config_get_bool(config, "line_head_bottom", true);
@@ -896,5 +897,4 @@ struct CursorStyle {
     selected_cell: Option<NuStyle>,
     selected_column: Option<NuStyle>,
     selected_row: Option<NuStyle>,
-    show_cursor: bool,
 }
diff --git a/crates/nu-utils/src/sample_config/default_config.nu b/crates/nu-utils/src/sample_config/default_config.nu
index 820ea4c652..e7e2e4e07f 100644
--- a/crates/nu-utils/src/sample_config/default_config.nu
+++ b/crates/nu-utils/src/sample_config/default_config.nu
@@ -189,10 +189,9 @@ $env.config = {
         },
         table: {
             split_line: {fg: "#404040"},
-            selected_cell: {},
+            selected_cell: {bg: light_blue},
             selected_row: {},
             selected_column: {},
-            show_cursor: true,
             line_head_top: true,
             line_head_bottom: true,
             line_shift: true,