From 8b193db0cbc9eb93f8c7d3fb514b59be92e0fa30 Mon Sep 17 00:00:00 2001 From: Tomas Koutsky Date: Thu, 3 Jun 2021 08:25:28 +0200 Subject: [PATCH] Fix VCS markers not showing up in textview (#3530) Changes: * Bug fix - bat adds markers only when a file path is passed and it can use git2 on it. It doesn't add markers when bytes are passed. Hence, the code is adjusted accordingly. The sideeffect is files being opened multiple times and its content being unnecessarily loaded in memory. * Refactoring of the crate - Config is extracted to its struct file. Repetitive blocks of code are dried and nested conditionals are flattened. --- Cargo.lock | 85 ++++++-- crates/nu_plugin_textview/Cargo.toml | 2 +- crates/nu_plugin_textview/src/config.rs | 110 +++++++++++ crates/nu_plugin_textview/src/lib.rs | 1 + crates/nu_plugin_textview/src/textview.rs | 227 ++++++---------------- 5 files changed, 236 insertions(+), 189 deletions(-) create mode 100644 crates/nu_plugin_textview/src/config.rs diff --git a/Cargo.lock b/Cargo.lock index fe2f170740..e47fce8204 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -446,17 +446,19 @@ checksum = "904dfeac50f3cdaba28fc6f57fdcddb75f49ed61346676a78c4ffe55877802fd" [[package]] name = "bat" -version = "0.17.1" +version = "0.18.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "df1e36a14d9603f9799def62aebb9dfece7e1b364aaaa8eba856e64bce1141c5" +checksum = "cc7224a2b5bdebb4762c36c25841694ab90328fe0c59fc8e685577795b9b785b" dependencies = [ "ansi_colours", "ansi_term 0.12.1", "atty", + "bugreport", "clap", + "clircle", "console", "content_inspector", - "dirs 3.0.2", + "dirs-next", "encoding", "error-chain", "git2", @@ -616,6 +618,17 @@ dependencies = [ "serde 1.0.126", ] +[[package]] +name = "bugreport" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a0e97e538864a7c95d33accbf64c8d354018ba3b6e032502fd0fe7259cf1aa3d" +dependencies = [ + "git-version", + "shell-escape", + "sys-info", +] + [[package]] name = "bumpalo" version = "3.7.0" @@ -806,6 +819,18 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "clircle" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e68bbd985a63de680ab4d1ad77b6306611a8f961b282c8b5ab513e6de934e396" +dependencies = [ + "cfg-if 1.0.0", + "libc", + "serde 1.0.125", + "winapi 0.3.9", +] + [[package]] name = "cloudabi" version = "0.0.3" @@ -885,9 +910,9 @@ dependencies = [ [[package]] name = "console" -version = "0.13.0" +version = "0.14.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a50aab2529019abfabfa93f1e6c41ef392f91fbf179b347a7e96abb524884a08" +checksum = "3993e6445baa160675931ec041a5e03ca84b9c6e32a056150d3aa2bdda0a1f45" dependencies = [ "encode_unicode", "lazy_static 1.4.0", @@ -896,7 +921,6 @@ dependencies = [ "terminal_size", "unicode-width", "winapi 0.3.9", - "winapi-util", ] [[package]] @@ -1411,15 +1435,6 @@ dependencies = [ "winapi 0.3.9", ] -[[package]] -name = "dirs" -version = "3.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30baa043103c9d0c2a57cf537cc2f35623889dc0d405e6c3cccfadbc81c71309" -dependencies = [ - "dirs-sys", -] - [[package]] name = "dirs-next" version = "2.0.0" @@ -2103,6 +2118,28 @@ version = "0.24.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0e4075386626662786ddb0ec9081e7c7eeb1ba31951f447ca780ef9f5d568189" +[[package]] +name = "git-version" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94918e83f1e01dedc2e361d00ce9487b14c58c7f40bab148026fa39d42cb41e2" +dependencies = [ + "git-version-macro", + "proc-macro-hack", +] + +[[package]] +name = "git-version-macro" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34a97a52fdee1870a34fa6e4b77570cba531b27d1838874fef4429a791a3d657" +dependencies = [ + "proc-macro-hack", + "proc-macro2", + "quote 1.0.9", + "syn 1.0.71", +] + [[package]] name = "git2" version = "0.13.20" @@ -5722,6 +5759,12 @@ dependencies = [ "git2", ] +[[package]] +name = "shell-escape" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "45bb67a18fa91266cc7807181f62f9178a6873bfad7dc788c42e6430db40184f" + [[package]] name = "shell-words" version = "1.0.0" @@ -6091,6 +6134,16 @@ dependencies = [ "yaml-rust", ] +[[package]] +name = "sys-info" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33fcecee49339531cf6bd84ecf3ed94f9c8ef4a7e700f2a1cac9cc1ca485383a" +dependencies = [ + "cc", + "libc", +] + [[package]] name = "sys-locale" version = "0.1.0" @@ -6158,7 +6211,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "edd106a334b7657c10b7c540a0106114feadeb4dc314513e97df481d5d966f42" dependencies = [ "byteorder", - "dirs 1.0.5", + "dirs", "winapi 0.3.9", ] diff --git a/crates/nu_plugin_textview/Cargo.toml b/crates/nu_plugin_textview/Cargo.toml index f67a3ca7d6..f8727d4112 100644 --- a/crates/nu_plugin_textview/Cargo.toml +++ b/crates/nu_plugin_textview/Cargo.toml @@ -17,7 +17,7 @@ nu-protocol = { path = "../nu-protocol", version = "0.32.0" } nu-source = { path = "../nu-source", version = "0.32.0" } nu-ansi-term = { version = "0.32.0", path = "../nu-ansi-term" } -bat = { version = "0.17.1", features = ["regex-fancy", "paging"] } +bat = { version = "0.18", features = ["regex-fancy", "paging", "git"] } term_size = "0.3.2" url = "2.2.0" diff --git a/crates/nu_plugin_textview/src/config.rs b/crates/nu_plugin_textview/src/config.rs new file mode 100644 index 0000000000..1bd74c0324 --- /dev/null +++ b/crates/nu_plugin_textview/src/config.rs @@ -0,0 +1,110 @@ +use nu_protocol::Value; + +pub struct Config { + pub term_width: usize, + pub tab_width: u64, + pub colored_output: bool, + pub true_color: bool, + pub header: bool, + pub line_numbers: bool, + pub grid: bool, + pub vcs_modification_markers: bool, + pub snip: bool, + pub wrapping_mode: bat::WrappingMode, + pub use_italics: bool, + pub paging_mode: bat::PagingMode, + pub pager: String, + pub line_ranges: bat::line_range::LineRanges, + // TODO: Not configurable + highlight_range: String, + // TODO: Not configurable + pub highlight_range_from: u64, + // TODO: Not configurable + pub highlight_range_to: u64, + pub theme: String, +} + +impl From<&Value> for Config { + fn from(value: &Value) -> Self { + let mut config = Config::default(); + + for (idx, entry) in value.row_entries() { + match idx.as_ref() { + "term_width" => { + config.term_width = entry.as_u64().unwrap_or(config.term_width as u64) as usize; + } + "tab_width" => { + config.tab_width = entry.as_u64().unwrap_or(4_u64); + } + "colored_output" => config.colored_output = entry.as_bool().unwrap_or(true), + "true_color" => config.true_color = entry.as_bool().unwrap_or(true), + "header" => config.header = entry.as_bool().unwrap_or(true), + "line_numbers" => config.line_numbers = entry.as_bool().unwrap_or(true), + "grid" => config.grid = entry.as_bool().unwrap_or(true), + "vcs_modification_markers" => { + config.vcs_modification_markers = entry.as_bool().unwrap_or(true) + } + "snip" => config.snip = entry.as_bool().unwrap_or(true), + "wrapping_mode" => { + config.wrapping_mode = match entry.as_string() { + Ok(s) if s.to_lowercase() == "nowrapping" => { + bat::WrappingMode::NoWrapping(true) + } + Ok(s) if s.to_lowercase() == "character" => bat::WrappingMode::Character, + _ => bat::WrappingMode::NoWrapping(true), + } + } + "use_italics" => config.use_italics = entry.as_bool().unwrap_or(true), + "paging_mode" => { + config.paging_mode = match entry.as_string() { + Ok(s) if s.to_lowercase() == "always" => bat::PagingMode::Always, + Ok(s) if s.to_lowercase() == "never" => bat::PagingMode::Never, + Ok(s) if s.to_lowercase() == "quitifonescreen" => { + bat::PagingMode::QuitIfOneScreen + } + _ => bat::PagingMode::QuitIfOneScreen, + } + } + "pager" => config.pager = entry.as_string().unwrap_or_else(|_| "less".to_string()), + // TODO: not real sure what to do with this + "line_ranges" => config.line_ranges = bat::line_range::LineRanges::all(), + "highlight_range" => config.highlight_range = "0,0".into(), + "theme" => { + config.theme = value + .as_string() + .unwrap_or_else(|_| "OneDarkHalf".to_string()) + } + _ => (), + } + } + + config + } +} + +impl Default for Config { + fn default() -> Self { + let (term_width, _) = term_size::dimensions().unwrap_or((80, 20)); + + Self { + term_width, + tab_width: 4, + colored_output: true, + true_color: true, + header: true, + line_numbers: true, + grid: true, + vcs_modification_markers: true, + snip: true, + wrapping_mode: bat::WrappingMode::NoWrapping(true), + use_italics: true, + paging_mode: bat::PagingMode::QuitIfOneScreen, + pager: "less".into(), + line_ranges: bat::line_range::LineRanges::all(), + highlight_range: "0.0".into(), + highlight_range_from: 0, + highlight_range_to: 0, + theme: "OneHalfDark".into(), + } + } +} diff --git a/crates/nu_plugin_textview/src/lib.rs b/crates/nu_plugin_textview/src/lib.rs index 3f6ec8b2af..9fc844450b 100644 --- a/crates/nu_plugin_textview/src/lib.rs +++ b/crates/nu_plugin_textview/src/lib.rs @@ -1,3 +1,4 @@ +mod config; mod nu; mod textview; diff --git a/crates/nu_plugin_textview/src/textview.rs b/crates/nu_plugin_textview/src/textview.rs index 98ec1ee60e..2d03a8fb11 100644 --- a/crates/nu_plugin_textview/src/textview.rs +++ b/crates/nu_plugin_textview/src/textview.rs @@ -1,3 +1,4 @@ +use crate::config::Config; use nu_protocol::{Primitive, UntaggedValue, Value}; use nu_source::{AnchorLocation, Tag}; use std::path::Path; @@ -11,179 +12,61 @@ impl TextView { } } -#[allow(clippy::cognitive_complexity)] -pub fn view_text_value(value: &Value) { - let (mut term_width, _) = term_size::dimensions().unwrap_or((80, 20)); - let mut tab_width: u64 = 4; - let mut colored_output = true; - let mut true_color = true; - let mut header = true; - let mut line_numbers = true; - let mut grid = true; - let mut vcs_modification_markers = true; - let mut snip = true; - let mut wrapping_mode = bat::WrappingMode::NoWrapping; - let mut use_italics = true; - let mut paging_mode = bat::PagingMode::QuitIfOneScreen; - let mut pager = "less".to_string(); - let mut line_ranges = bat::line_range::LineRanges::all(); - let mut _highlight_range = "0,0"; - let highlight_range_from: u64 = 0; - let highlight_range_to: u64 = 0; - let mut theme = "OneHalfDark".to_string(); +fn get_file_path(source: AnchorLocation) -> Option { + match source { + AnchorLocation::File(file) => { + let path = Path::new(&file); - if let Ok(config) = nu_data::config::config(Tag::unknown()) { - if let Some(batvars) = config.get("textview") { - for (idx, value) in batvars.row_entries() { - match idx.as_ref() { - "term_width" => { - term_width = value.as_u64().unwrap_or(term_width as u64) as usize; - } - "tab_width" => { - tab_width = value.as_u64().unwrap_or(4_u64); - } - "colored_output" => colored_output = value.as_bool().unwrap_or(true), - "true_color" => true_color = value.as_bool().unwrap_or(true), - "header" => header = value.as_bool().unwrap_or(true), - "line_numbers" => line_numbers = value.as_bool().unwrap_or(true), - "grid" => grid = value.as_bool().unwrap_or(true), - "vcs_modification_markers" => { - vcs_modification_markers = value.as_bool().unwrap_or(true) - } - "snip" => snip = value.as_bool().unwrap_or(true), - "wrapping_mode" => { - wrapping_mode = match value.as_string() { - Ok(s) if s.to_lowercase() == "nowrapping" => { - bat::WrappingMode::NoWrapping - } - Ok(s) if s.to_lowercase() == "character" => { - bat::WrappingMode::Character - } - _ => bat::WrappingMode::NoWrapping, - } - } - "use_italics" => use_italics = value.as_bool().unwrap_or(true), - "paging_mode" => { - paging_mode = match value.as_string() { - Ok(s) if s.to_lowercase() == "always" => bat::PagingMode::Always, - Ok(s) if s.to_lowercase() == "never" => bat::PagingMode::Never, - Ok(s) if s.to_lowercase() == "quitifonescreen" => { - bat::PagingMode::QuitIfOneScreen - } - _ => bat::PagingMode::QuitIfOneScreen, - } - } - "pager" => pager = value.as_string().unwrap_or_else(|_| "less".to_string()), - "line_ranges" => line_ranges = bat::line_range::LineRanges::all(), // not real sure what to do with this - "highlight_range" => _highlight_range = "0,0", //ignore config value for now - "theme" => { - theme = value - .as_string() - .unwrap_or_else(|_| "OneDarkHalf".to_string()) - } - _ => (), - } - } - } - } - - let value_anchor = value.anchor(); - if let UntaggedValue::Primitive(Primitive::String(ref s)) = &value.value { - if let Some(source) = value_anchor { - let file_path: Option = match source { - AnchorLocation::File(file) => { - let path = Path::new(&file); - Some(path.to_string_lossy().to_string()) - } - AnchorLocation::Url(url) => { - let url = url::Url::parse(&url); - if let Ok(url) = url { - if let Some(mut segments) = url.path_segments() { - if let Some(file) = segments.next_back() { - let path = Path::new(file); - Some(path.to_string_lossy().to_string()) - } else { - None - } - } else { - None - } - } else { - None - } - } - //FIXME: this probably isn't correct - AnchorLocation::Source(_source) => None, - }; - - match file_path { - Some(file_path) => { - // Let bat do it's thing - bat::PrettyPrinter::new() - .input(bat::Input::from_bytes(s.as_bytes()).name(file_path)) - .term_width(term_width as usize) - .tab_width(Some(tab_width as usize)) - .colored_output(colored_output) - .true_color(true_color) - .header(header) - .line_numbers(line_numbers) - .grid(grid) - .vcs_modification_markers(vcs_modification_markers) - .snip(snip) - .wrapping_mode(wrapping_mode) - .use_italics(use_italics) - .paging_mode(paging_mode) - .pager(&pager) - .line_ranges(line_ranges) - .highlight_range(highlight_range_from as usize, highlight_range_to as usize) - .theme(&theme) - .print() - .expect("Error with bat PrettyPrint"); - } - _ => { - bat::PrettyPrinter::new() - .input_from_bytes(s.as_bytes()) - .term_width(term_width as usize) - .tab_width(Some(tab_width as usize)) - .colored_output(colored_output) - .true_color(true_color) - .header(header) - .line_numbers(line_numbers) - .grid(grid) - .vcs_modification_markers(vcs_modification_markers) - .snip(snip) - .wrapping_mode(wrapping_mode) - .use_italics(use_italics) - .paging_mode(paging_mode) - .pager(&pager) - .line_ranges(line_ranges) - .highlight_range(highlight_range_from as usize, highlight_range_to as usize) - .theme(&theme) - .print() - .expect("Error with bat PrettyPrint"); - } - } - } else { - bat::PrettyPrinter::new() - .input_from_bytes(s.as_bytes()) - .term_width(term_width as usize) - .tab_width(Some(tab_width as usize)) - .colored_output(colored_output) - .true_color(true_color) - .header(header) - .line_numbers(line_numbers) - .grid(grid) - .vcs_modification_markers(vcs_modification_markers) - .snip(snip) - .wrapping_mode(wrapping_mode) - .use_italics(use_italics) - .paging_mode(paging_mode) - .pager(&pager) - .line_ranges(line_ranges) - .highlight_range(highlight_range_from as usize, highlight_range_to as usize) - .theme(&theme) - .print() - .expect("Error with bat PrettyPrint"); + Some(path.to_string_lossy().to_string()) } + AnchorLocation::Url(url) => url::Url::parse(&url).ok().and_then(|url| { + url.path_segments().and_then(|mut segments| { + segments + .next_back() + .map(|segment| Path::new(segment).to_string_lossy().to_string()) + }) + }), + //FIXME: this probably isn't correct + AnchorLocation::Source(_source) => None, + } +} + +#[allow(clippy::cognitive_complexity)] +pub fn view_text_value(value: &Value) { + let config = nu_data::config::config(Tag::unknown()) + .ok() + .and_then(|config| config.get("textview").map(Config::from)) + .unwrap_or_else(Config::default); + + if let UntaggedValue::Primitive(Primitive::String(ref s)) = &value.value { + let mut printer = bat::PrettyPrinter::new(); + + printer + .term_width(config.term_width) + .tab_width(Some(config.tab_width as usize)) + .colored_output(config.colored_output) + .true_color(config.true_color) + .header(config.header) + .line_numbers(config.line_numbers) + .grid(config.grid) + .vcs_modification_markers(config.vcs_modification_markers) + .snip(config.snip) + .wrapping_mode(config.wrapping_mode) + .use_italics(config.use_italics) + .paging_mode(config.paging_mode) + .pager(&config.pager) + .line_ranges(config.line_ranges) + .highlight_range( + config.highlight_range_from as usize, + config.highlight_range_to as usize, + ) + .theme(&config.theme); + + match value.anchor().and_then(get_file_path) { + Some(file_path) => printer.input_file(file_path), + None => printer.input_from_bytes(s.as_bytes()), + }; + + printer.print().expect("Error with bat PrettyPrint"); } }