From 7c274ad4d8a9dad40f6b6f2aa2d75c157f687208 Mon Sep 17 00:00:00 2001
From: Reilly Wood <26268125+rgwood@users.noreply.github.com>
Date: Sat, 30 Sep 2023 19:26:43 -0500
Subject: [PATCH] `explore`: remove 4 line config options (#10562)

This PR removes the `line_head_top`, `line_head_bottom`, `line_shift`,
and `line_index` configuration options from `explore`. These were
previously used to control whether the horizontal+vertical lines in this
`ls | explore -i` screenshot would be displayed:


![image](https://github.com/nushell/nushell/assets/26268125/b705e8a0-935c-40ff-be4a-f119dbae3080)

Now, all lines are displayed (same as the previous default config
values) and this is no longer configurable.

## Context

I'm continuing to chip away at `explore` when I have time. I have a
long-term goal to make `explore` simpler for users+developers. For now
I'm mostly making small incremental changes where I find underused
functionality+configuration and remove it; hopefully eventually this
will make it easier to make larger changes.

I found these specific config options a little hard to understand when
reading `explore` code, and when reading `config.nu` as a user their
behaviour+naming is not obvious. I also think that in the long term,
`explore` styling should inherit most styling from `table` instead of
having its own styling system.
---
 crates/nu-explore/src/commands/table.rs       |  25 ----
 crates/nu-explore/src/explore.rs              |  12 --
 crates/nu-explore/src/views/record/mod.rs     |  21 ---
 crates/nu-explore/src/views/record/tablew.rs  | 122 ++++++++----------
 .../src/sample_config/default_config.nu       |   4 -
 5 files changed, 53 insertions(+), 131 deletions(-)

diff --git a/crates/nu-explore/src/commands/table.rs b/crates/nu-explore/src/commands/table.rs
index 21a2237499..6ef36697ef 100644
--- a/crates/nu-explore/src/commands/table.rs
+++ b/crates/nu-explore/src/commands/table.rs
@@ -25,10 +25,6 @@ pub struct TableCmd {
 #[derive(Debug, Default, Clone)]
 struct TableSettings {
     orientation: Option<Orientation>,
-    line_head_top: Option<bool>,
-    line_head_bottom: Option<bool>,
-    line_shift: Option<bool>,
-    line_index: Option<bool>,
     split_line_s: Option<Style>,
     selected_cell_s: Option<Style>,
     selected_row_s: Option<Style>,
@@ -91,11 +87,6 @@ impl ViewCommand for TableCmd {
             ConfigOption::boolean(":table group", "Show index", "table.show_index"),
             ConfigOption::boolean(":table group", "Show header", "table.show_head"),
 
-            ConfigOption::boolean(":table group", "Lines are lines", "table.line_head_top"),
-            ConfigOption::boolean(":table group", "Lines are lines", "table.line_head_bottom"),
-            ConfigOption::boolean(":table group", "Lines are lines", "table.line_shift"),
-            ConfigOption::boolean(":table group", "Lines are lines", "table.line_index"),
-
             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()),
@@ -145,22 +136,6 @@ impl ViewCommand for TableCmd {
             view.set_orientation_current(o);
         }
 
-        if self.settings.line_head_bottom.unwrap_or(false) {
-            view.set_line_head_bottom(true);
-        }
-
-        if self.settings.line_head_top.unwrap_or(false) {
-            view.set_line_head_top(true);
-        }
-
-        if self.settings.line_index.unwrap_or(false) {
-            view.set_line_index(true);
-        }
-
-        if self.settings.line_shift.unwrap_or(false) {
-            view.set_line_trailing(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 eacebf4da0..56e227716a 100644
--- a/crates/nu-explore/src/explore.rs
+++ b/crates/nu-explore/src/explore.rs
@@ -203,14 +203,6 @@ fn prepare_default_config(config: &mut HashMap<String, Value>) {
 
     const TABLE_SPLIT_LINE: Style = color(Some(Color::Rgb(64, 64, 64)), None);
 
-    const TABLE_LINE_HEADER_TOP: bool = true;
-
-    const TABLE_LINE_HEADER_BOTTOM: bool = true;
-
-    const TABLE_LINE_INDEX: bool = true;
-
-    const TABLE_LINE_SHIFT: bool = true;
-
     const TABLE_SELECT_CELL: Style = color(None, None);
 
     const TABLE_SELECT_ROW: Style = color(None, None);
@@ -249,10 +241,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, "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);
-        insert_bool(&mut hm, "line_index", TABLE_LINE_INDEX);
 
         config.insert(String::from("table"), map_into_value(hm));
     }
diff --git a/crates/nu-explore/src/views/record/mod.rs b/crates/nu-explore/src/views/record/mod.rs
index 115fdaee39..d908202080 100644
--- a/crates/nu-explore/src/views/record/mod.rs
+++ b/crates/nu-explore/src/views/record/mod.rs
@@ -75,22 +75,6 @@ impl<'a> RecordView<'a> {
         self.theme.cursor.selected_column = Some(style)
     }
 
-    pub fn set_line_head_top(&mut self, b: bool) {
-        self.theme.table.header_top = b;
-    }
-
-    pub fn set_line_head_bottom(&mut self, b: bool) {
-        self.theme.table.header_bottom = b;
-    }
-
-    pub fn set_line_trailing(&mut self, b: bool) {
-        self.theme.table.shift_line = b;
-    }
-
-    pub fn set_line_index(&mut self, b: bool) {
-        self.theme.table.index_line = b;
-    }
-
     pub fn set_padding_column(&mut self, (left, right): (usize, usize)) {
         self.theme.table.padding_column_left = left;
         self.theme.table.padding_column_right = right;
@@ -852,11 +836,6 @@ fn theme_from_config(config: &ConfigMap) -> TableTheme {
     theme.cursor.selected_row = colors.get("selected_row").cloned();
     theme.cursor.selected_column = colors.get("selected_column").cloned();
 
-    theme.table.header_top = config_get_bool(config, "line_head_top", true);
-    theme.table.header_bottom = config_get_bool(config, "line_head_bottom", true);
-    theme.table.shift_line = config_get_bool(config, "line_shift", true);
-    theme.table.index_line = config_get_bool(config, "line_index", true);
-
     theme.table.show_header = config_get_bool(config, "show_head", true);
     theme.table.show_index = config_get_bool(config, "show_index", false);
 
diff --git a/crates/nu-explore/src/views/record/tablew.rs b/crates/nu-explore/src/views/record/tablew.rs
index d83c0dfaa2..269f4f9d6e 100644
--- a/crates/nu-explore/src/views/record/tablew.rs
+++ b/crates/nu-explore/src/views/record/tablew.rs
@@ -44,10 +44,6 @@ pub struct TableStyle {
     pub shift_line_style: NuStyle,
     pub show_index: bool,
     pub show_header: bool,
-    pub header_top: bool,
-    pub header_bottom: bool,
-    pub shift_line: bool,
-    pub index_line: bool,
     pub padding_index_left: usize,
     pub padding_index_right: usize,
     pub padding_column_left: usize,
@@ -129,16 +125,14 @@ impl<'a> TableW<'a> {
             data_y += 1;
             data_height -= 1;
 
-            if self.style.header_top {
-                data_y += 1;
-                data_height -= 1;
-                head_y += 1
-            }
+            // top line
+            data_y += 1;
+            data_height -= 1;
+            head_y += 1;
 
-            if self.style.header_bottom {
-                data_y += 1;
-                data_height -= 1;
-            }
+            // bottom line
+            data_y += 1;
+            data_height -= 1;
         }
 
         if area.width == 0 || area.height == 0 {
@@ -152,13 +146,7 @@ impl<'a> TableW<'a> {
         }
 
         if show_head {
-            // fixme: color from config
-            let top = self.style.header_top;
-            let bottom = self.style.header_bottom;
-
-            if top || bottom {
-                render_header_borders(buf, area, 1, splitline_s, top, bottom);
-            }
+            render_header_borders(buf, area, 1, splitline_s);
         }
 
         if show_index {
@@ -172,11 +160,15 @@ impl<'a> TableW<'a> {
                 padding_index_r,
             );
 
-            if self.style.index_line {
-                let head_t = show_head && self.style.header_bottom;
-                width +=
-                    render_vertical(buf, width, data_y, data_height, head_t, false, splitline_s);
-            }
+            width += render_vertical(
+                buf,
+                width,
+                data_y,
+                data_height,
+                show_head,
+                false,
+                splitline_s,
+            );
         }
 
         let mut do_render_shift_column = false;
@@ -252,21 +244,22 @@ impl<'a> TableW<'a> {
             }
         }
 
-        if do_render_shift_column {
-            // we actually want to show a shift only in header.
-            //
-            // render_shift_column(buf, used_width, head_offset, available_height);
-
-            if show_head {
-                width += render_space(buf, width, data_y, data_height, padding_cell_l);
-                width += render_shift_column(buf, width, head_y, 1, shift_column_s);
-                width += render_space(buf, width, data_y, data_height, padding_cell_r);
-            }
+        if do_render_shift_column && show_head {
+            width += render_space(buf, width, data_y, data_height, padding_cell_l);
+            width += render_shift_column(buf, width, head_y, 1, shift_column_s);
+            width += render_space(buf, width, data_y, data_height, padding_cell_r);
         }
 
-        if self.style.shift_line && width < area.width {
-            let head_t = show_head && self.style.header_bottom;
-            width += render_vertical(buf, width, data_y, data_height, head_t, false, splitline_s);
+        if width < area.width {
+            width += render_vertical(
+                buf,
+                width,
+                data_y,
+                data_height,
+                show_head,
+                false,
+                splitline_s,
+            );
         }
 
         let rest = area.width.saturating_sub(width);
@@ -306,10 +299,15 @@ impl<'a> TableW<'a> {
                 padding_index_r,
             );
 
-            if self.style.index_line {
-                let x = area.x + left_w;
-                left_w += render_vertical(buf, x, area.y, area.height, false, false, splitline_s);
-            }
+            left_w += render_vertical(
+                buf,
+                area.x + left_w,
+                area.y,
+                area.height,
+                false,
+                false,
+                splitline_s,
+            );
         }
 
         let mut columns = &self.columns[self.index_row..];
@@ -331,8 +329,7 @@ impl<'a> TableW<'a> {
                 .map(|s| head_row_text(s, self.style_computer))
                 .collect::<Vec<_>>();
 
-            let have_index_line = show_index && self.style.index_line;
-            if !have_index_line && self.style.header_top {
+            if !show_index {
                 let x = area.x + left_w;
                 left_w += render_vertical(buf, x, area.y, area.height, false, false, splitline_s);
             }
@@ -351,10 +348,15 @@ impl<'a> TableW<'a> {
                     .push(text, layout_x, area.y + i as u16, columns_width as u16, 1);
             }
 
-            if self.style.header_bottom {
-                let x = area.x + left_w;
-                left_w += render_vertical(buf, x, area.y, area.height, false, false, splitline_s);
-            }
+            left_w += render_vertical(
+                buf,
+                area.x + left_w,
+                area.y,
+                area.height,
+                false,
+                false,
+                splitline_s,
+            );
         }
 
         let mut do_render_shift_column = false;
@@ -525,30 +527,12 @@ impl Widget for IndexColumn<'_> {
     }
 }
 
-fn render_header_borders(
-    buf: &mut Buffer,
-    area: Rect,
-    span: u16,
-    style: NuStyle,
-    top: bool,
-    bottom: bool,
-) -> (u16, u16) {
-    let mut i = 0;
-    let mut borders = Borders::NONE;
-    if top {
-        borders |= Borders::TOP;
-        i += 1;
-    }
-
-    if bottom {
-        borders |= Borders::BOTTOM;
-        i += 1;
-    }
-
+fn render_header_borders(buf: &mut Buffer, area: Rect, span: u16, style: NuStyle) -> (u16, u16) {
+    let borders = Borders::TOP | Borders::BOTTOM;
     let block = Block::default()
         .borders(borders)
         .border_style(nu_style_to_tui(style));
-    let height = i + span;
+    let height = span + 2;
     let area = Rect::new(area.x, area.y, area.width, height);
     block.render(area, buf);
 
diff --git a/crates/nu-utils/src/sample_config/default_config.nu b/crates/nu-utils/src/sample_config/default_config.nu
index 077a09a56c..55d7f4f849 100644
--- a/crates/nu-utils/src/sample_config/default_config.nu
+++ b/crates/nu-utils/src/sample_config/default_config.nu
@@ -192,10 +192,6 @@ $env.config = {
             selected_cell: {bg: light_blue},
             selected_row: {},
             selected_column: {},
-            line_head_top: true,
-            line_head_bottom: true,
-            line_shift: true,
-            line_index: true,
         },
     }