explore: consolidate padding config, handle ByteStream, tweak naming+comments (#12915)

Some minor changes to `explore`, continuing on my mission to simplify
the command in preparation for a larger UX overhaul:

1. Consolidate padding configuration. I don't think we need separate
config points for the (optional) index column and regular data columns
in the normal pager, they can share padding configuration. Likewise, in
the binary viewer all 3 columns (index, data, ASCII) had their
left+right padding configured independently.
2. Update `explore` so we use the binary viewer for the new `ByteStream`
type. `cat foo.txt | into binary | explore` was not using the binary
viewer after the `ByteStream` changes.
3. Tweak the naming of a few helper functions, add a comment

I've put the changes in separate commits to make them easier to review.

---------

Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com>

crates/nu-explore/src/commands/table.rs

@@ -25,8 +25,6 @@ struct TableSettings {
     selected_column_s: Option<Style>,
     padding_column_left: Option<usize>,
     padding_column_right: Option<usize>,
-    padding_index_left: Option<usize>,
-    padding_index_right: Option<usize>,
     turn_on_cursor_mode: bool,
 }
 
@@ -102,16 +100,6 @@ impl ViewCommand for TableCmd {
             view.set_padding_column((c.0, p))
         }
 
-        if let Some(p) = self.settings.padding_index_left {
-            let c = view.get_padding_index();
-            view.set_padding_index((p, c.1))
-        }
-
-        if let Some(p) = self.settings.padding_index_right {
-            let c = view.get_padding_index();
-            view.set_padding_index((c.0, p))
-        }
-
         if self.settings.turn_on_cursor_mode {
             view.set_cursor_mode();
         }

crates/nu-explore/src/lib.rs

@@ -35,13 +35,16 @@ fn run_pager(
     let commands = create_command_registry();
 
     let is_record = matches!(input, PipelineData::Value(Value::Record { .. }, ..));
-    let is_binary = matches!(input, PipelineData::Value(Value::Binary { .. }, ..));
+    let is_binary = matches!(
+        input,
+        PipelineData::Value(Value::Binary { .. }, ..) | PipelineData::ByteStream(..)
+    );
 
     if is_binary {
         p.show_message("For help type :help");
 
-        let view = binary_view(input);
-        return p.run(engine_state, stack, ctrlc, view, commands);
+        let view = binary_view(input)?;
+        return p.run(engine_state, stack, ctrlc, Some(view), commands);
     }
 
     let (columns, data) = collect_pipeline(input)?;
@@ -88,15 +91,16 @@ fn help_view() -> Option<Page> {
     Some(Page::new(HelpCmd::view(), false))
 }
 
-fn binary_view(input: PipelineData) -> Option<Page> {
+fn binary_view(input: PipelineData) -> Result<Page> {
     let data = match input {
         PipelineData::Value(Value::Binary { val, .. }, _) => val,
+        PipelineData::ByteStream(bs, _) => bs.into_bytes()?,
         _ => unreachable!("checked beforehand"),
     };
 
     let view = BinaryView::new(data);
 
-    Some(Page::new(view, true))
+    Ok(Page::new(view, true))
 }
 
 fn create_command_registry() -> CommandRegistry {

crates/nu-explore/src/views/binary/binary_widget.rs

@@ -12,6 +12,9 @@ use crate::{
     views::util::{nu_style_to_tui, text_style_to_tui_style},
 };
 
+/// Padding between segments in the hex view
+const SEGMENT_PADDING: u16 = 1;
+
 #[derive(Debug, Clone)]
 pub struct BinaryWidget<'a> {
     data: &'a [u8],
@@ -70,45 +73,27 @@ impl BinarySettings {
 #[derive(Debug, Default, Clone)]
 pub struct BinaryStyle {
     color_index: Option<NuStyle>,
-    indent_index: Indent,
-    indent_data: Indent,
-    indent_ascii: Indent,
-    indent_segment: usize,
+    column_padding_left: u16,
+    column_padding_right: u16,
     show_split: bool,
 }
 
 impl BinaryStyle {
     pub fn new(
         color_index: Option<NuStyle>,
-        indent_index: Indent,
-        indent_data: Indent,
-        indent_ascii: Indent,
-        indent_segment: usize,
+        column_padding_left: u16,
+        column_padding_right: u16,
         show_split: bool,
     ) -> Self {
         Self {
             color_index,
-            indent_index,
-            indent_data,
-            indent_ascii,
-            indent_segment,
+            column_padding_left,
+            column_padding_right,
             show_split,
         }
     }
 }
 
-#[derive(Debug, Default, Clone)]
-pub struct Indent {
-    left: u16,
-    right: u16,
-}
-
-impl Indent {
-    pub fn new(left: u16, right: u16) -> Self {
-        Self { left, right }
-    }
-}
-
 impl Widget for BinaryWidget<'_> {
     fn render(self, area: Rect, buf: &mut Buffer) {
         let min_width = get_widget_width(&self);
@@ -153,9 +138,9 @@ fn render_hexdump(area: Rect, buf: &mut Buffer, w: BinaryWidget) {
         let line = &w.data[start_index..];
 
         if show_index {
-            x += render_space(buf, x, y, 1, w.style.indent_index.left);
+            x += render_space(buf, x, y, 1, w.style.column_padding_left);
             x += render_hex_usize(buf, x, y, address, index_width, get_index_style(&w));
-            x += render_space(buf, x, y, 1, w.style.indent_index.right);
+            x += render_space(buf, x, y, 1, w.style.column_padding_right);
         }
 
         if show_split {
@@ -163,9 +148,9 @@ fn render_hexdump(area: Rect, buf: &mut Buffer, w: BinaryWidget) {
         }
 
         if show_data {
-            x += render_space(buf, x, y, 1, w.style.indent_data.left);
+            x += render_space(buf, x, y, 1, w.style.column_padding_left);
             x += render_data_line(buf, x, y, line, &w);
-            x += render_space(buf, x, y, 1, w.style.indent_data.right);
+            x += render_space(buf, x, y, 1, w.style.column_padding_right);
         }
 
         if show_split {
@@ -173,9 +158,9 @@ fn render_hexdump(area: Rect, buf: &mut Buffer, w: BinaryWidget) {
         }
 
         if show_ascii {
-            x += render_space(buf, x, y, 1, w.style.indent_ascii.left);
+            x += render_space(buf, x, y, 1, w.style.column_padding_left);
             x += render_ascii_line(buf, x, y, line, &w);
-            render_space(buf, x, y, 1, w.style.indent_ascii.right);
+            render_space(buf, x, y, 1, w.style.column_padding_right);
         }
     }
 
@@ -193,9 +178,9 @@ fn render_hexdump(area: Rect, buf: &mut Buffer, w: BinaryWidget) {
             let y = line;
 
             if show_index {
-                x += render_space(buf, x, y, 1, w.style.indent_index.left);
+                x += render_space(buf, x, y, 1, w.style.column_padding_left);
                 x += render_hex_usize(buf, x, y, address, index_width, get_index_style(&w));
-                x += render_space(buf, x, y, 1, w.style.indent_index.right);
+                x += render_space(buf, x, y, 1, w.style.column_padding_right);
             }
 
             if show_split {
@@ -203,9 +188,9 @@ fn render_hexdump(area: Rect, buf: &mut Buffer, w: BinaryWidget) {
             }
 
             if show_data {
-                x += render_space(buf, x, y, 1, w.style.indent_data.left);
+                x += render_space(buf, x, y, 1, w.style.column_padding_left);
                 x += render_space(buf, x, y, 1, data_line_size);
-                x += render_space(buf, x, y, 1, w.style.indent_data.right);
+                x += render_space(buf, x, y, 1, w.style.column_padding_right);
             }
 
             if show_split {
@@ -213,9 +198,9 @@ fn render_hexdump(area: Rect, buf: &mut Buffer, w: BinaryWidget) {
             }
 
             if show_ascii {
-                x += render_space(buf, x, y, 1, w.style.indent_ascii.left);
+                x += render_space(buf, x, y, 1, w.style.column_padding_left);
                 x += render_space(buf, x, y, 1, ascii_line_size);
-                render_space(buf, x, y, 1, w.style.indent_ascii.right);
+                render_space(buf, x, y, 1, w.style.column_padding_right);
             }
         }
     }
@@ -232,13 +217,13 @@ fn render_data_line(buf: &mut Buffer, x: u16, y: u16, line: &[u8], w: &BinaryWid
 
     while count != count_max && count * segment_size < line.len() {
         let data = &line[count * segment_size..];
-        size += render_space(buf, x + size, y, 1, w.style.indent_segment as u16);
+        size += render_space(buf, x + size, y, 1, SEGMENT_PADDING);
         size += render_segment(buf, x + size, y, data, w);
         count += 1;
     }
 
     while count != count_max {
-        size += render_space(buf, x + size, y, 1, w.style.indent_segment as u16);
+        size += render_space(buf, x + size, y, 1, SEGMENT_PADDING);
         size += render_space(buf, x + size, y, 1, w.opts.segment_size as u16 * 2);
         count += 1;
     }
@@ -398,7 +383,7 @@ fn get_widget_width(w: &BinaryWidget) -> usize {
     let index_size = usize_to_hex(max_index, 0).len();
     let index_size = index_size.max(MIN_INDEX_SIZE);
 
-    let data_split_size = w.opts.count_segments.saturating_sub(1) * w.style.indent_segment;
+    let data_split_size = w.opts.count_segments.saturating_sub(1) * (SEGMENT_PADDING as usize);
     let data_size = line_size + data_split_size;
 
     let ascii_size = w.opts.count_segments * w.opts.segment_size;
@@ -406,17 +391,17 @@ fn get_widget_width(w: &BinaryWidget) -> usize {
     let split = w.style.show_split as usize;
     #[allow(clippy::identity_op)]
     let min_width = 0
-        + w.style.indent_index.left as usize
+        + w.style.column_padding_left as usize
         + index_size
-        + w.style.indent_index.right as usize
+        + w.style.column_padding_right as usize
         + split
-        + w.style.indent_data.left as usize
+        + w.style.column_padding_left as usize
         + data_size
-        + w.style.indent_data.right as usize
+        + w.style.column_padding_right as usize
         + split
-        + w.style.indent_ascii.left as usize
+        + w.style.column_padding_left as usize
         + ascii_size
-        + w.style.indent_ascii.right as usize;
+        + w.style.column_padding_right as usize;
 
     min_width
 }

crates/nu-explore/src/views/binary/mod.rs

@@ -19,7 +19,7 @@ use crate::{
     util::create_map,
 };
 
-use self::binary_widget::{BinarySettings, BinaryStyle, BinaryWidget, Indent};
+use self::binary_widget::{BinarySettings, BinaryStyle, BinaryWidget};
 
 use super::{cursor::XYCursor, Layout, View, ViewConfig};
 
@@ -193,19 +193,8 @@ fn settings_from_config(config: &ConfigMap) -> Settings {
         ),
         style: BinaryStyle::new(
             colors.get("color_index").cloned(),
-            Indent::new(
-                config_get_usize(config, "padding_index_left", 2) as u16,
-                config_get_usize(config, "padding_index_right", 2) as u16,
-            ),
-            Indent::new(
-                config_get_usize(config, "padding_data_left", 2) as u16,
-                config_get_usize(config, "padding_data_right", 2) as u16,
-            ),
-            Indent::new(
-                config_get_usize(config, "padding_ascii_left", 2) as u16,
-                config_get_usize(config, "padding_ascii_right", 2) as u16,
-            ),
-            config_get_usize(config, "padding_segment", 1),
+            config_get_usize(config, "column_padding_left", 1) as u16,
+            config_get_usize(config, "column_padding_right", 1) as u16,
             config_get_bool(config, "split", false),
         ),
     }

crates/nu-explore/src/views/record/mod.rs

@@ -71,26 +71,14 @@ impl<'a> RecordView<'a> {
     }
 
     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;
-    }
-
-    pub fn set_padding_index(&mut self, (left, right): (usize, usize)) {
-        self.theme.table.padding_index_left = left;
-        self.theme.table.padding_index_right = right;
+        self.theme.table.column_padding_left = left;
+        self.theme.table.column_padding_right = right;
     }
 
     pub fn get_padding_column(&self) -> (usize, usize) {
         (
-            self.theme.table.padding_column_left,
-            self.theme.table.padding_column_right,
-        )
-    }
-
-    pub fn get_padding_index(&self) -> (usize, usize) {
-        (
-            self.theme.table.padding_index_left,
-            self.theme.table.padding_index_right,
+            self.theme.table.column_padding_left,
+            self.theme.table.column_padding_right,
         )
     }
 
@@ -853,10 +841,8 @@ fn theme_from_config(config: &ConfigMap) -> TableTheme {
     theme.table.show_header = config_get_bool(config, "show_head", true);
     theme.table.show_index = config_get_bool(config, "show_index", false);
 
-    theme.table.padding_index_left = config_get_usize(config, "padding_index_left", 2);
-    theme.table.padding_index_right = config_get_usize(config, "padding_index_right", 1);
-    theme.table.padding_column_left = config_get_usize(config, "padding_column_left", 2);
-    theme.table.padding_column_right = config_get_usize(config, "padding_column_right", 2);
+    theme.table.column_padding_left = config_get_usize(config, "column_padding_left", 1);
+    theme.table.column_padding_right = config_get_usize(config, "column_padding_right", 1);
 
     theme
 }

crates/nu-explore/src/views/record/tablew.rs

@@ -41,10 +41,8 @@ pub struct TableStyle {
     pub shift_line_style: NuStyle,
     pub show_index: bool,
     pub show_header: bool,
-    pub padding_index_left: usize,
-    pub padding_index_right: usize,
-    pub padding_column_left: usize,
-    pub padding_column_right: usize,
+    pub column_padding_left: usize,
+    pub column_padding_right: usize,
 }
 
 impl<'a> TableW<'a> {
@@ -103,10 +101,8 @@ impl StatefulWidget for TableW<'_> {
 // todo: refactoring these to methods as they have quite a bit in common.
 impl<'a> TableW<'a> {
     fn render_table_horizontal(self, area: Rect, buf: &mut Buffer, state: &mut TableWState) {
-        let padding_cell_l = self.style.padding_column_left as u16;
-        let padding_cell_r = self.style.padding_column_right as u16;
-        let padding_index_l = self.style.padding_index_left as u16;
-        let padding_index_r = self.style.padding_index_right as u16;
+        let padding_l = self.style.column_padding_left as u16;
+        let padding_r = self.style.column_padding_right as u16;
 
         let show_index = self.style.show_index;
         let show_head = self.style.show_header;
@@ -153,11 +149,11 @@ impl<'a> TableW<'a> {
                 area,
                 self.style_computer,
                 self.index_row,
-                padding_index_l,
-                padding_index_r,
+                padding_l,
+                padding_r,
             );
 
-            width += render_vertical(
+            width += render_vertical_line_with_split(
                 buf,
                 width,
                 data_y,
@@ -193,7 +189,7 @@ impl<'a> TableW<'a> {
                 let is_last = col + 1 == self.columns.len();
                 let space = area.width - width;
 
-                let pad = padding_cell_l + padding_cell_r;
+                let pad = padding_l + padding_r;
                 let head = show_head.then_some(&mut head);
                 let (w, ok, shift) =
                     truncate_column_width(space, 1, use_space, pad, is_last, &mut column, head);
@@ -216,20 +212,20 @@ impl<'a> TableW<'a> {
                 }
 
                 let mut w = width;
-                w += render_space(buf, w, head_y, 1, padding_cell_l);
+                w += render_space(buf, w, head_y, 1, padding_l);
                 w += render_column(buf, w, head_y, use_space, &header);
-                w += render_space(buf, w, head_y, 1, padding_cell_r);
+                w += render_space(buf, w, head_y, 1, padding_r);
 
-                let x = w - padding_cell_r - use_space;
+                let x = w - padding_r - use_space;
                 state.layout.push(&header[0].0, x, head_y, use_space, 1);
             }
 
-            width += render_space(buf, width, data_y, data_height, padding_cell_l);
+            width += render_space(buf, width, data_y, data_height, padding_l);
             width += render_column(buf, width, data_y, use_space, &column);
-            width += render_space(buf, width, data_y, data_height, padding_cell_r);
+            width += render_space(buf, width, data_y, data_height, padding_r);
 
             for (row, (text, _)) in column.iter().enumerate() {
-                let x = width - padding_cell_r - use_space;
+                let x = width - padding_r - use_space;
                 let y = data_y + row as u16;
                 state.layout.push(text, x, y, use_space, 1);
             }
@@ -242,13 +238,13 @@ impl<'a> TableW<'a> {
         }
 
         if do_render_shift_column && show_head {
-            width += render_space(buf, width, data_y, data_height, padding_cell_l);
+            width += render_space(buf, width, data_y, data_height, padding_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);
+            width += render_space(buf, width, data_y, data_height, padding_r);
         }
 
         if width < area.width {
-            width += render_vertical(
+            width += render_vertical_line_with_split(
                 buf,
                 width,
                 data_y,
@@ -273,10 +269,8 @@ impl<'a> TableW<'a> {
             return;
         }
 
-        let padding_cell_l = self.style.padding_column_left as u16;
-        let padding_cell_r = self.style.padding_column_right as u16;
-        let padding_index_l = self.style.padding_index_left as u16;
-        let padding_index_r = self.style.padding_index_right as u16;
+        let padding_l = self.style.column_padding_left as u16;
+        let padding_r = self.style.column_padding_right as u16;
 
         let show_index = self.style.show_index;
         let show_head = self.style.show_header;
@@ -292,11 +286,11 @@ impl<'a> TableW<'a> {
                 area,
                 self.style_computer,
                 self.index_row,
-                padding_index_l,
-                padding_index_r,
+                padding_l,
+                padding_r,
             );
 
-            left_w += render_vertical(
+            left_w += render_vertical_line_with_split(
                 buf,
                 area.x + left_w,
                 area.y,
@@ -316,7 +310,7 @@ impl<'a> TableW<'a> {
             let columns_width = columns.iter().map(|s| string_width(s)).max().unwrap_or(0);
 
             let will_use_space =
-                padding_cell_l as usize + padding_cell_r as usize + columns_width + left_w as usize;
+                padding_l as usize + padding_r as usize + columns_width + left_w as usize;
             if will_use_space > area.width as usize {
                 return;
             }
@@ -328,24 +322,32 @@ impl<'a> TableW<'a> {
 
             if !show_index {
                 let x = area.x + left_w;
-                left_w += render_vertical(buf, x, area.y, area.height, false, false, splitline_s);
+                left_w += render_vertical_line_with_split(
+                    buf,
+                    x,
+                    area.y,
+                    area.height,
+                    false,
+                    false,
+                    splitline_s,
+                );
             }
 
             let x = area.x + left_w;
-            left_w += render_space(buf, x, area.y, 1, padding_cell_l);
+            left_w += render_space(buf, x, area.y, 1, padding_l);
             let x = area.x + left_w;
             left_w += render_column(buf, x, area.y, columns_width as u16, &columns);
             let x = area.x + left_w;
-            left_w += render_space(buf, x, area.y, 1, padding_cell_r);
+            left_w += render_space(buf, x, area.y, 1, padding_r);
 
-            let layout_x = left_w - padding_cell_r - columns_width as u16;
+            let layout_x = left_w - padding_r - columns_width as u16;
             for (i, (text, _)) in columns.iter().enumerate() {
                 state
                     .layout
                     .push(text, layout_x, area.y + i as u16, columns_width as u16, 1);
             }
 
-            left_w += render_vertical(
+            left_w += render_vertical_line_with_split(
                 buf,
                 area.x + left_w,
                 area.y,
@@ -372,7 +374,7 @@ impl<'a> TableW<'a> {
             let column_width = column_width as u16;
             let available = area.width - left_w;
             let is_last = col + 1 == self.data.len();
-            let pad = padding_cell_l + padding_cell_r;
+            let pad = padding_l + padding_r;
             let (column_width, ok, shift) =
                 truncate_column_width(available, 1, column_width, pad, is_last, &mut column, None);
 
@@ -385,15 +387,15 @@ impl<'a> TableW<'a> {
             }
 
             let x = area.x + left_w;
-            left_w += render_space(buf, x, area.y, area.height, padding_cell_l);
+            left_w += render_space(buf, x, area.y, area.height, padding_l);
             let x = area.x + left_w;
             left_w += render_column(buf, x, area.y, column_width, &column);
             let x = area.x + left_w;
-            left_w += render_space(buf, x, area.y, area.height, padding_cell_r);
+            left_w += render_space(buf, x, area.y, area.height, padding_r);
 
             {
                 for (row, (text, _)) in column.iter().enumerate() {
-                    let x = left_w - padding_cell_r - column_width;
+                    let x = left_w - padding_r - column_width;
                     let y = area.y + row as u16;
                     state.layout.push(text, x, y, column_width, 1);
                 }
@@ -408,11 +410,11 @@ impl<'a> TableW<'a> {
 
         if do_render_shift_column {
             let x = area.x + left_w;
-            left_w += render_space(buf, x, area.y, area.height, padding_cell_l);
+            left_w += render_space(buf, x, area.y, area.height, padding_l);
             let x = area.x + left_w;
             left_w += render_shift_column(buf, x, area.y, area.height, shift_column_s);
             let x = area.x + left_w;
-            left_w += render_space(buf, x, area.y, area.height, padding_cell_r);
+            left_w += render_space(buf, x, area.y, area.height, padding_r);
         }
 
         _ = left_w;
@@ -561,7 +563,7 @@ fn render_index(
     width
 }
 
-fn render_vertical(
+fn render_vertical_line_with_split(
     buf: &mut Buffer,
     x: u16,
     y: u16,
@@ -570,7 +572,7 @@ fn render_vertical(
     bottom_slit: bool,
     style: NuStyle,
 ) -> u16 {
-    render_vertical_split(buf, x, y, height, style);
+    render_vertical_line(buf, x, y, height, style);
 
     if top_slit && y > 0 {
         render_top_connector(buf, x, y - 1, style);
@@ -583,7 +585,7 @@ fn render_vertical(
     1
 }
 
-fn render_vertical_split(buf: &mut Buffer, x: u16, y: u16, height: u16, style: NuStyle) {
+fn render_vertical_line(buf: &mut Buffer, x: u16, y: u16, height: u16, style: NuStyle) {
     let style = TextStyle {
         alignment: Alignment::Left,
         color_style: Some(style),
@@ -615,10 +617,12 @@ fn create_column(data: &[Vec<NuText>], col: usize) -> Vec<NuText> {
     column
 }
 
+/// Starting at cell [x, y]: paint `width` characters of `c` (left to right), move 1 row down, repeat
+/// Repeat this `height` times
 fn repeat_vertical(
     buf: &mut ratatui::buffer::Buffer,
-    x_offset: u16,
-    y_offset: u16,
+    x: u16,
+    y: u16,
     width: u16,
     height: u16,
     c: char,
@@ -631,7 +635,7 @@ fn repeat_vertical(
     let span = Span::styled(text, style);
 
     for row in 0..height {
-        buf.set_span(x_offset, y_offset + row, &span, width);
+        buf.set_span(x, y + row, &span, width);
     }
 }