From c7097ca93761ea2062fef2271ebbdc4a10d67bce Mon Sep 17 00:00:00 2001 From: Reilly Wood <26268125+rgwood@users.noreply.github.com> Date: Wed, 22 May 2024 20:06:14 -0700 Subject: [PATCH] `explore` cleanup: remove+move binary viewer config (#12920) Small change, removing 4 more configuration options from `explore`'s binary viewer: 1. `show_index` 2. `show_data` 3. `show_ascii` 4. `show_split` These controlled whether the 3 columns in the binary viewer (index, hex data, ASCII) and the pipe separator (`|`) in between them are shown. I don't think we need this level of configurability until the `explore` command is more mature, and maybe even not then; we can just show them all. I think it's very unlikely that anyone is using these configuration points. Also, the row offset (e.g. how many rows we have scrolled down) was being stored in config/settings when it's arguably not config; more like internal state of the binary viewer. I moved it to a more appropriate location and renamed it. --- .../src/views/binary/binary_widget.rs | 135 +++++++----------- crates/nu-explore/src/views/binary/mod.rs | 14 +- 2 files changed, 52 insertions(+), 97 deletions(-) diff --git a/crates/nu-explore/src/views/binary/binary_widget.rs b/crates/nu-explore/src/views/binary/binary_widget.rs index bee4e9d854..d100774be5 100644 --- a/crates/nu-explore/src/views/binary/binary_widget.rs +++ b/crates/nu-explore/src/views/binary/binary_widget.rs @@ -20,11 +20,17 @@ pub struct BinaryWidget<'a> { data: &'a [u8], opts: BinarySettings, style: BinaryStyle, + row_offset: usize, } impl<'a> BinaryWidget<'a> { pub fn new(data: &'a [u8], opts: BinarySettings, style: BinaryStyle) -> Self { - Self { data, opts, style } + Self { + data, + opts, + style, + row_offset: 0, + } } pub fn count_lines(&self) -> usize { @@ -35,37 +41,22 @@ impl<'a> BinaryWidget<'a> { self.opts.count_segments * self.opts.segment_size } - pub fn set_index_offset(&mut self, offset: usize) { - self.opts.index_offset = offset; + pub fn set_row_offset(&mut self, offset: usize) { + self.row_offset = offset; } } #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] pub struct BinarySettings { - disable_index: bool, - disable_ascii: bool, - disable_data: bool, segment_size: usize, count_segments: usize, - index_offset: usize, } impl BinarySettings { - pub fn new( - disable_index: bool, - disable_ascii: bool, - disable_data: bool, - segment_size: usize, - count_segments: usize, - index_offset: usize, - ) -> Self { + pub fn new(segment_size: usize, count_segments: usize) -> Self { Self { - disable_index, - disable_ascii, - disable_data, segment_size, count_segments, - index_offset, } } } @@ -75,7 +66,6 @@ pub struct BinaryStyle { color_index: Option, column_padding_left: u16, column_padding_right: u16, - show_split: bool, } impl BinaryStyle { @@ -83,13 +73,11 @@ impl BinaryStyle { color_index: Option, column_padding_left: u16, column_padding_right: u16, - show_split: bool, ) -> Self { Self { color_index, column_padding_left, column_padding_right, - show_split, } } } @@ -102,10 +90,6 @@ impl Widget for BinaryWidget<'_> { return; } - if self.opts.disable_index && self.opts.disable_data && self.opts.disable_ascii { - return; - } - render_hexdump(area, buf, self); } } @@ -114,11 +98,6 @@ impl Widget for BinaryWidget<'_> { fn render_hexdump(area: Rect, buf: &mut Buffer, w: BinaryWidget) { const MIN_INDEX_SIZE: usize = 8; - let show_index = !w.opts.disable_index; - let show_data = !w.opts.disable_data; - let show_ascii = !w.opts.disable_ascii; - let show_split = w.style.show_split; - let index_width = get_max_index_size(&w).max(MIN_INDEX_SIZE) as u16; // safe as it's checked before hand that we have enough space let mut last_line = None; @@ -126,7 +105,7 @@ fn render_hexdump(area: Rect, buf: &mut Buffer, w: BinaryWidget) { for line in 0..area.height { let data_line_length = w.opts.count_segments * w.opts.segment_size; let start_index = line as usize * data_line_length; - let address = w.opts.index_offset + start_index; + let address = w.row_offset + start_index; if start_index > w.data.len() { last_line = Some(line); @@ -137,31 +116,24 @@ fn render_hexdump(area: Rect, buf: &mut Buffer, w: BinaryWidget) { let y = line; let line = &w.data[start_index..]; - if show_index { - 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.column_padding_right); - } + // index column + 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.column_padding_right); - if show_split { - x += render_split(buf, x, y); - } + x += render_vertical_split(buf, x, y); - if show_data { - 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.column_padding_right); - } + // data/hex column + 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.column_padding_right); - if show_split { - x += render_split(buf, x, y); - } + x += render_vertical_split(buf, x, y); - if show_ascii { - 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.column_padding_right); - } + // ASCII column + 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.column_padding_right); } let data_line_size = (w.opts.count_segments * (w.opts.segment_size * 2) @@ -172,36 +144,29 @@ fn render_hexdump(area: Rect, buf: &mut Buffer, w: BinaryWidget) { for line in last_line..area.height { let data_line_length = w.opts.count_segments * w.opts.segment_size; let start_index = line as usize * data_line_length; - let address = w.opts.index_offset + start_index; + let address = w.row_offset + start_index; let mut x = 0; let y = line; - if show_index { - 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.column_padding_right); - } + // index column + 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.column_padding_right); - if show_split { - x += render_split(buf, x, y); - } + x += render_vertical_split(buf, x, y); - if show_data { - 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.column_padding_right); - } + // data/hex column + 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.column_padding_right); - if show_split { - x += render_split(buf, x, y); - } + x += render_vertical_split(buf, x, y); - if show_ascii { - 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.column_padding_right); - } + // ASCII column + 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.column_padding_right); } } } @@ -336,12 +301,15 @@ fn get_index_style(w: &BinaryWidget) -> Option { w.style.color_index } -fn render_space(buf: &mut Buffer, x: u16, y: u16, height: u16, padding: u16) -> u16 { - repeat_vertical(buf, x, y, padding, height, ' ', TextStyle::default()); - padding +/// Render blank characters starting at the given position, going right `width` characters and down `height` characters. +/// Returns `width` for convenience. +fn render_space(buf: &mut Buffer, x: u16, y: u16, height: u16, width: u16) -> u16 { + repeat_vertical(buf, x, y, width, height, ' ', TextStyle::default()); + width } -fn render_split(buf: &mut Buffer, x: u16, y: u16) -> u16 { +/// Render a vertical split (│) at the given position. Returns the width of the split (always 1) for convenience. +fn render_vertical_split(buf: &mut Buffer, x: u16, y: u16) -> u16 { repeat_vertical(buf, x, y, 1, 1, '│', TextStyle::default()); 1 } @@ -369,7 +337,7 @@ fn repeat_vertical( fn get_max_index_size(w: &BinaryWidget) -> usize { let line_size = w.opts.count_segments * (w.opts.segment_size * 2); let count_lines = w.data.len() / line_size; - let max_index = w.opts.index_offset + count_lines * line_size; + let max_index = w.row_offset + count_lines * line_size; usize_to_hex(max_index, 0).len() } @@ -379,7 +347,7 @@ fn get_widget_width(w: &BinaryWidget) -> usize { let line_size = w.opts.count_segments * (w.opts.segment_size * 2); let count_lines = w.data.len() / line_size; - let max_index = w.opts.index_offset + count_lines * line_size; + let max_index = w.row_offset + count_lines * line_size; let index_size = usize_to_hex(max_index, 0).len(); let index_size = index_size.max(MIN_INDEX_SIZE); @@ -388,17 +356,16 @@ fn get_widget_width(w: &BinaryWidget) -> usize { let ascii_size = w.opts.count_segments * w.opts.segment_size; - let split = w.style.show_split as usize; #[allow(clippy::identity_op)] let min_width = 0 + w.style.column_padding_left as usize + index_size + w.style.column_padding_right as usize - + split + + 1 // split + w.style.column_padding_left as usize + data_size + w.style.column_padding_right as usize - + split + + 1 //split + w.style.column_padding_left as usize + ascii_size + w.style.column_padding_right as usize; diff --git a/crates/nu-explore/src/views/binary/mod.rs b/crates/nu-explore/src/views/binary/mod.rs index 31ea0d6eaf..7e44a3985e 100644 --- a/crates/nu-explore/src/views/binary/mod.rs +++ b/crates/nu-explore/src/views/binary/mod.rs @@ -107,7 +107,7 @@ fn create_binary_widget(v: &BinaryView) -> BinaryWidget<'_> { let data = &v.data[index..]; let mut w = BinaryWidget::new(data, v.settings.opts, v.settings.style.clone()); - w.set_index_offset(index); + w.set_row_offset(index); w } @@ -184,29 +184,17 @@ fn settings_from_config(config: &ConfigMap) -> Settings { Settings { opts: BinarySettings::new( - !config_get_bool(config, "show_index", true), - !config_get_bool(config, "show_ascii", true), - !config_get_bool(config, "show_data", true), config_get_usize(config, "segment_size", 2), config_get_usize(config, "count_segments", 8), - 0, ), style: BinaryStyle::new( colors.get("color_index").cloned(), 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), ), } } -fn config_get_bool(config: &ConfigMap, key: &str, default: bool) -> bool { - config - .get(key) - .and_then(|v| v.as_bool().ok()) - .unwrap_or(default) -} - fn config_get_usize(config: &ConfigMap, key: &str, default: usize) -> usize { config .get(key)