From fcd585bb6dc27bbf5bf74b422b36614241b59eb4 Mon Sep 17 00:00:00 2001 From: Benedikt Mandelkow Date: Fri, 16 Feb 2024 08:35:11 +0100 Subject: [PATCH 1/7] remove tabled --- Cargo.lock | 134 +-------------------------------------- Cargo.toml | 1 - src/plumbing/progress.rs | 75 +++++++++++----------- 3 files changed, 39 insertions(+), 171 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6c1c509b974..88bf22c1918 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -51,15 +51,6 @@ version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4b46cbb362ab8752921c97e041f5e366ee6297bd428a31275b9fcf1e380f7299" -[[package]] -name = "ansi-str" -version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1cf4578926a981ab0ca955dc023541d19de37112bc24c1a197bd806d3d86ad1d" -dependencies = [ - "ansitok", -] - [[package]] name = "ansiterm" version = "0.12.2" @@ -69,16 +60,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "ansitok" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "220044e6a1bb31ddee4e3db724d29767f352de47445a6cd75e1a173142136c83" -dependencies = [ - "nom", - "vte", -] - [[package]] name = "anstream" version = "0.6.5" @@ -139,12 +120,6 @@ version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bddcadddf5e9015d310179a59bb28c4d4b9920ad0f11e8e14dbadf654890c9a6" -[[package]] -name = "arrayvec" -version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "23b62fc65de8e4e7f52534fb52b0f3ed04746ae267519eef2a83941e8085068b" - [[package]] name = "arrayvec" version = "0.7.4" @@ -448,12 +423,6 @@ version = "3.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f30e7476521f6f8af1a1c4c0b8cc94f0bee37d91763d0ca2665f299b6cd8aec" -[[package]] -name = "bytecount" -version = "0.6.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e1e5f035d16fc623ae5f74981db80a439803888314e3a555fd6f04acd51a3205" - [[package]] name = "byteorder" version = "1.5.0" @@ -1279,7 +1248,6 @@ dependencies = [ "owo-colors", "prodash 28.0.0", "serde_derive", - "tabled", "terminal_size", "time", "tracing", @@ -3486,12 +3454,6 @@ version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" -[[package]] -name = "minimal-lexical" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" - [[package]] name = "miniz_oxide" version = "0.7.1" @@ -3544,16 +3506,6 @@ dependencies = [ "pin-utils", ] -[[package]] -name = "nom" -version = "7.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d273983c5a657a70a3e8f2a01329822f3b8c8172b73826411a55751e404a0a4a" -dependencies = [ - "memchr", - "minimal-lexical", -] - [[package]] name = "ntapi" version = "0.4.1" @@ -3710,19 +3662,6 @@ version = "4.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "caff54706df99d2a78a5a4e3455ff45448d81ef1bb63c22cd14052ca0e993a3f" -[[package]] -name = "papergrid" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9ad43c07024ef767f9160710b3a6773976194758c7919b17e63b863db0bdf7fb" -dependencies = [ - "ansi-str", - "ansitok", - "bytecount", - "fnv", - "unicode-width", -] - [[package]] name = "parking" version = "2.2.0" @@ -3889,30 +3828,6 @@ dependencies = [ "toml_edit", ] -[[package]] -name = "proc-macro-error" -version = "1.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c" -dependencies = [ - "proc-macro-error-attr", - "proc-macro2", - "quote", - "syn 1.0.109", - "version_check", -] - -[[package]] -name = "proc-macro-error-attr" -version = "1.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869" -dependencies = [ - "proc-macro2", - "quote", - "version_check", -] - [[package]] name = "proc-macro2" version = "1.0.75" @@ -4646,32 +4561,6 @@ dependencies = [ "libc", ] -[[package]] -name = "tabled" -version = "0.15.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4c998b0c8b921495196a48aabaf1901ff28be0760136e31604f7967b0792050e" -dependencies = [ - "ansi-str", - "ansitok", - "papergrid", - "tabled_derive", - "unicode-width", -] - -[[package]] -name = "tabled_derive" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4c138f99377e5d653a371cdad263615634cfc8467685dfe8e73e2b8e98f44b17" -dependencies = [ - "heck", - "proc-macro-error", - "proc-macro2", - "quote", - "syn 1.0.109", -] - [[package]] name = "tar" version = "0.4.40" @@ -5034,7 +4923,7 @@ version = "3.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "794a32261a1f5eb6a4462c81b59cec87b5c27d5deea7dd1ac8fc781c41d226db" dependencies = [ - "arrayvec 0.7.4", + "arrayvec", ] [[package]] @@ -5129,27 +5018,6 @@ version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" -[[package]] -name = "vte" -version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6cbce692ab4ca2f1f3047fcf732430249c0e971bfdd2b234cf2c47ad93af5983" -dependencies = [ - "arrayvec 0.5.2", - "utf8parse", - "vte_generate_state_changes", -] - -[[package]] -name = "vte_generate_state_changes" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d257817081c7dffcdbab24b9e62d2def62e2ff7d00b1c20062551e6cccc145ff" -dependencies = [ - "proc-macro2", - "quote", -] - [[package]] name = "waker-fn" version = "1.1.1" diff --git a/Cargo.toml b/Cargo.toml index 5d767ba2042..af632079bc5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -190,7 +190,6 @@ tracing = { version = "0.1.37", optional = true } owo-colors = "4.0.0" # for config-tree -tabled = { version = "0.15.0", features = ["ansi"] } terminal_size = "0.3.0" # Avoid pre-compiled binaries, see https://github.com/serde-rs/serde/issues/2538 and https://github.com/serde-rs/serde/pull/2590 diff --git a/src/plumbing/progress.rs b/src/plumbing/progress.rs index f381e48fd22..3f3cfa8f5be 100644 --- a/src/plumbing/progress.rs +++ b/src/plumbing/progress.rs @@ -5,9 +5,6 @@ use std::{ use crosstermion::crossterm::style::Stylize; use owo_colors::OwoColorize; -use tabled::settings::peaker::PriorityMax; -use tabled::settings::{Extract, Style, Width}; -use tabled::Tabled; #[derive(Clone)] enum Usage { @@ -73,33 +70,35 @@ struct Record { usage: Usage, } -impl Tabled for Record { - const LENGTH: usize = 3; +// TODO implement this without table, how hard is it? - fn fields(&self) -> Vec> { - let mut tokens = self.config.split('.'); - let mut buf = vec![{ - let name = tokens.next().expect("present"); - if name == "gitoxide" { - name.bold().green() - } else { - name.bold() - } - .to_string() - }]; - buf.extend(tokens.map(ToOwned::to_owned)); +// impl Tabled for Record { +// const LENGTH: usize = 3; - vec![ - Cow::Borrowed(self.usage.icon()), - buf.join(".").into(), - self.usage.to_string().into(), - ] - } +// fn fields(&self) -> Vec> { +// let mut tokens = self.config.split('.'); +// let mut buf = vec![{ +// let name = tokens.next().expect("present"); +// if name == "gitoxide" { +// name.bold().green() +// } else { +// name.bold() +// } +// .to_string() +// }]; +// buf.extend(tokens.map(ToOwned::to_owned)); - fn headers() -> Vec> { - vec!["icon".into(), "key".into(), "info".into()] - } -} +// vec![ +// Cow::Borrowed(self.usage.icon()), +// buf.join(".").into(), +// self.usage.to_string().into(), +// ] +// } + +// fn headers() -> Vec> { +// vec!["icon".into(), "key".into(), "info".into()] +// } +// } static GIT_CONFIG: &[Record] = &[ Record { @@ -575,16 +574,18 @@ pub fn show_progress() -> anyhow::Result<()> { .count() )?; - let mut table = tabled::Table::new(sorted); - let table = table.with(Style::blank()).with(Extract::rows(1..)); - println!( - "{}", - if let Some((terminal_size::Width(w), _)) = terminal_size::terminal_size() { - table.with(Width::wrap(w as usize).keep_words().priority::()) - } else { - table - } - ); + // TODO implement this without table, how hard is it? + + // let mut table = tabled::Table::new(sorted); + // let table = table.with(Style::blank()).with(Extract::rows(1..)); + // println!( + // "{}", + // if let Some((terminal_size::Width(w), _)) = terminal_size::terminal_size() { + // table.with(Width::wrap(w as usize).keep_words().priority::()) + // } else { + // table + // } + // ); println!("{buf}"); Ok(()) } From 1ecc96d319bc60cb9dbabcf17211aa7057134fd6 Mon Sep 17 00:00:00 2001 From: Benedikt Mandelkow Date: Fri, 16 Feb 2024 08:56:13 +0100 Subject: [PATCH 2/7] bring back old implementation from https://github.com/Byron/gitoxide/commit/65e64964c7cd151e53e5a7d4b9ba8fabda1c0e16 this also removes terminal_size, although its not large --- Cargo.lock | 11 ----------- Cargo.toml | 2 +- src/plumbing/progress.rs | 11 +++++++---- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 88bf22c1918..924218e761f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1248,7 +1248,6 @@ dependencies = [ "owo-colors", "prodash 28.0.0", "serde_derive", - "terminal_size", "time", "tracing", "tracing-forest", @@ -4593,16 +4592,6 @@ dependencies = [ "winapi-util", ] -[[package]] -name = "terminal_size" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "21bebf2b7c9e0a515f6e0f8c51dc0f8e4696391e6f1ff30379559f8365fb0df7" -dependencies = [ - "rustix 0.38.31", - "windows-sys 0.48.0", -] - [[package]] name = "thiserror" version = "1.0.56" diff --git a/Cargo.toml b/Cargo.toml index af632079bc5..bb6d8e29c50 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -190,7 +190,7 @@ tracing = { version = "0.1.37", optional = true } owo-colors = "4.0.0" # for config-tree -terminal_size = "0.3.0" +# terminal_size = "0.3.0" # Avoid pre-compiled binaries, see https://github.com/serde-rs/serde/issues/2538 and https://github.com/serde-rs/serde/pull/2590 serde_derive = ">=1.0.185" diff --git a/src/plumbing/progress.rs b/src/plumbing/progress.rs index 3f3cfa8f5be..884b9280917 100644 --- a/src/plumbing/progress.rs +++ b/src/plumbing/progress.rs @@ -1,7 +1,4 @@ -use std::{ - borrow::Cow, - fmt::{Display, Formatter}, -}; +use std::fmt::{Display, Formatter}; use crosstermion::crossterm::style::Stylize; use owo_colors::OwoColorize; @@ -586,6 +583,12 @@ pub fn show_progress() -> anyhow::Result<()> { // table // } // ); + + // noted: reverted from https://github.com/Byron/gitoxide/commit/65e64964c7cd151e53e5a7d4b9ba8fabda1c0e16 + for Record { config, usage } in sorted { + println!("{} {}: {usage}", usage.icon(), config.bold(),); + } + println!("{buf}"); Ok(()) } From cc8d008843a4e4567301c14ce5f25855417cf898 Mon Sep 17 00:00:00 2001 From: Benedikt Mandelkow Date: Fri, 16 Feb 2024 09:45:23 +0100 Subject: [PATCH 3/7] reserve 50 characters for the config name --- Cargo.toml | 2 -- src/plumbing/progress.rs | 55 ++++++++-------------------------------- 2 files changed, 10 insertions(+), 47 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bb6d8e29c50..41fd0118ee3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -189,8 +189,6 @@ tracing = { version = "0.1.37", optional = true } # for progress owo-colors = "4.0.0" -# for config-tree -# terminal_size = "0.3.0" # Avoid pre-compiled binaries, see https://github.com/serde-rs/serde/issues/2538 and https://github.com/serde-rs/serde/pull/2590 serde_derive = ">=1.0.185" diff --git a/src/plumbing/progress.rs b/src/plumbing/progress.rs index 884b9280917..e3311beb094 100644 --- a/src/plumbing/progress.rs +++ b/src/plumbing/progress.rs @@ -67,36 +67,6 @@ struct Record { usage: Usage, } -// TODO implement this without table, how hard is it? - -// impl Tabled for Record { -// const LENGTH: usize = 3; - -// fn fields(&self) -> Vec> { -// let mut tokens = self.config.split('.'); -// let mut buf = vec![{ -// let name = tokens.next().expect("present"); -// if name == "gitoxide" { -// name.bold().green() -// } else { -// name.bold() -// } -// .to_string() -// }]; -// buf.extend(tokens.map(ToOwned::to_owned)); - -// vec![ -// Cow::Borrowed(self.usage.icon()), -// buf.join(".").into(), -// self.usage.to_string().into(), -// ] -// } - -// fn headers() -> Vec> { -// vec!["icon".into(), "key".into(), "info".into()] -// } -// } - static GIT_CONFIG: &[Record] = &[ Record { config: "core.symlinks", @@ -571,22 +541,17 @@ pub fn show_progress() -> anyhow::Result<()> { .count() )?; - // TODO implement this without table, how hard is it? - - // let mut table = tabled::Table::new(sorted); - // let table = table.with(Style::blank()).with(Extract::rows(1..)); - // println!( - // "{}", - // if let Some((terminal_size::Width(w), _)) = terminal_size::terminal_size() { - // table.with(Width::wrap(w as usize).keep_words().priority::()) - // } else { - // table - // } - // ); - - // noted: reverted from https://github.com/Byron/gitoxide/commit/65e64964c7cd151e53e5a7d4b9ba8fabda1c0e16 for Record { config, usage } in sorted { - println!("{} {}: {usage}", usage.icon(), config.bold(),); + println!( + "{icon} {config: <50}: {usage}", + icon = usage.icon(), + config = if let Some(config) = config.strip_prefix("gitoxide.") { + format!("{gitoxide}{config}", gitoxide = "gitoxide.".green()) + } else { + config.to_string() + } + .bold(), + ); } println!("{buf}"); From c0c4d59bbae688074184b186e08c6bf8e23c5705 Mon Sep 17 00:00:00 2001 From: Benedikt Mandelkow Date: Fri, 16 Feb 2024 11:25:36 +0100 Subject: [PATCH 4/7] fix indenting by removing formatting on the config str --- src/plumbing/progress.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/plumbing/progress.rs b/src/plumbing/progress.rs index e3311beb094..1668038de6f 100644 --- a/src/plumbing/progress.rs +++ b/src/plumbing/progress.rs @@ -1,6 +1,5 @@ use std::fmt::{Display, Formatter}; -use crosstermion::crossterm::style::Stylize; use owo_colors::OwoColorize; #[derive(Clone)] @@ -46,7 +45,7 @@ impl Display for Usage { impl Usage { pub fn icon(&self) -> &'static str { match self { - Puzzled => "?", + Puzzled => "❓", NotApplicable { .. } => "❌", Planned { .. } => "🕒", NotPlanned { .. } => "🤔", @@ -546,11 +545,10 @@ pub fn show_progress() -> anyhow::Result<()> { "{icon} {config: <50}: {usage}", icon = usage.icon(), config = if let Some(config) = config.strip_prefix("gitoxide.") { - format!("{gitoxide}{config}", gitoxide = "gitoxide.".green()) + format!("{gitoxide}{config}", gitoxide = "gitoxide.") } else { config.to_string() } - .bold(), ); } From 4ac597a0e9ccb8ed1e1864afcffdc55abc37260c Mon Sep 17 00:00:00 2001 From: Benedikt Mandelkow Date: Fri, 16 Feb 2024 19:25:25 +0100 Subject: [PATCH 5/7] add back line wrapping simplify Usage Variants --- Cargo.lock | 18 +- Cargo.toml | 5 +- src/plumbing/progress.rs | 384 +++++++++++++++++++-------------------- 3 files changed, 205 insertions(+), 202 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 924218e761f..1e59cd56517 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1245,9 +1245,9 @@ dependencies = [ "gix-features 0.38.0", "is-terminal", "once_cell", - "owo-colors", "prodash 28.0.0", "serde_derive", + "terminal_size", "time", "tracing", "tracing-forest", @@ -3655,12 +3655,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" -[[package]] -name = "owo-colors" -version = "4.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "caff54706df99d2a78a5a4e3455ff45448d81ef1bb63c22cd14052ca0e993a3f" - [[package]] name = "parking" version = "2.2.0" @@ -4592,6 +4586,16 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "terminal_size" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "21bebf2b7c9e0a515f6e0f8c51dc0f8e4696391e6f1ff30379559f8365fb0df7" +dependencies = [ + "rustix 0.38.31", + "windows-sys 0.48.0", +] + [[package]] name = "thiserror" version = "1.0.56" diff --git a/Cargo.toml b/Cargo.toml index 41fd0118ee3..4116d7bd39e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -186,9 +186,8 @@ tracing-forest = { version = "0.1.5", features = ["serde"], optional = true } tracing-subscriber = { version = "0.3.17", optional = true } tracing = { version = "0.1.37", optional = true } -# for progress -owo-colors = "4.0.0" - +# for config-tree +terminal_size = "0.3.0" # Avoid pre-compiled binaries, see https://github.com/serde-rs/serde/issues/2538 and https://github.com/serde-rs/serde/pull/2590 serde_derive = ">=1.0.185" diff --git a/src/plumbing/progress.rs b/src/plumbing/progress.rs index 1668038de6f..cb00154cdf6 100644 --- a/src/plumbing/progress.rs +++ b/src/plumbing/progress.rs @@ -1,17 +1,15 @@ use std::fmt::{Display, Formatter}; -use owo_colors::OwoColorize; - #[derive(Clone)] enum Usage { /// It's not reasonable to implement it as the prerequisites don't apply. - NotApplicable { reason: &'static str }, + NotApplicable(&'static str), /// We have no intention to implement it, but that can change if there is demand. - NotPlanned { reason: &'static str }, + NotPlanned(&'static str), /// We definitely want to implement this configuration value. - Planned { note: Option<&'static str> }, + Planned(&'static str), /// The configuration is already used, possibly with a given `deviation`. - InUse { deviation: Option<&'static str> }, + InUse(&'static str), /// Needs analysis, unclear how it works or what it does. Puzzled, } @@ -21,20 +19,16 @@ impl Display for Usage { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { Puzzled => f.write_str("❓")?, - NotApplicable { reason } => write!(f, "not applicable: {reason}")?, - NotPlanned { reason } => { - write!(f, "{}", "not planned".blink())?; - write!(f, " ℹ {} ℹ", reason.bright_white())?; - } - Planned { note } => { - write!(f, "{}", "planned".blink())?; - if let Some(note) = note { - write!(f, " ℹ {} ℹ", note.bright_white())?; + NotApplicable(reason) => write!(f, "not applicable because {reason}")?, + NotPlanned(reason) => write!(f, "not planned || {reason}")?, + Planned(note) => { + if !note.is_empty() { + write!(f, "planned || {note}")? } } - InUse { deviation } => { - if let Some(deviation) = deviation { - write!(f, "{}", format!("❗️{deviation}❗️").bright_white())? + InUse(deviation) => { + if !deviation.is_empty() { + write!(f, "❗️❗️❗️{deviation}")? } } } @@ -43,17 +37,17 @@ impl Display for Usage { } impl Usage { - pub fn icon(&self) -> &'static str { + pub const fn icon(&self) -> &'static str { match self { Puzzled => "❓", - NotApplicable { .. } => "❌", - Planned { .. } => "🕒", - NotPlanned { .. } => "🤔", - InUse { deviation, .. } => { - if deviation.is_some() { - "👌️" - } else { + NotApplicable(_) => "❌", + Planned(_) => "🕒", + NotPlanned(_) => "🤔", + InUse(deviation) => { + if deviation.is_empty() { "✅" + } else { + "👌️" } } } @@ -69,403 +63,387 @@ struct Record { static GIT_CONFIG: &[Record] = &[ Record { config: "core.symlinks", - usage: Planned {note: Some("needed to handle checkouts faithfully")} + usage: Planned("needed to handle checkouts faithfully") }, Record { config: "core.hideDotFiles", - usage: Planned {note: Some("Seems useful, but needs demand from windows users")} + usage: Planned("Seems useful, but needs demand from windows users") }, Record { config: "core.packedGitWindowSize", - usage: NotPlanned { reason: "an optimization for handling many large packs more efficiently seems unnecessary" } + usage: NotPlanned("an optimization for handling many large packs more efficiently seems unnecessary") }, Record { config: "core.packedGitLimit", - usage: NotApplicable { reason: "we target 32bit systems only and don't use a windowing mechanism" } + usage: NotApplicable("we target 32bit systems only and don't use a windowing mechanism") }, Record { config: "core.alternateRefsCommand", - usage: NotPlanned { reason: "there is no need as we can perform the required operation in-binary. This could happen though if there is a use-case and demand." } + usage: NotPlanned("there is no need as we can perform the required operation in-binary. This could happen though if there is a use-case and demand.") }, Record { config: "core.alternateRefsPrefixes", - usage: NotPlanned { reason: "seems like a niche feature, but can be implemented if there is demand" } + usage: NotPlanned("seems like a niche feature, but can be implemented if there is demand") }, Record { config: "core.compression", - usage: Planned { note: Some("Allow to remove similar hardcoded value - passing it through will be some effort") }, + usage: Planned("Allow to remove similar hardcoded value - passing it through will be some effort") }, Record { config: "core.loosecompression", - usage: Planned { note: None }, + usage: Planned("") }, Record { config: "core.protectHFS", - usage: Planned { note: Some("relevant for checkout on MacOS") }, + usage: Planned("relevant for checkout on MacOS") }, Record { config: "core.protectNTFS", - usage: NotPlanned { reason: "lack of demand"}, + usage: NotPlanned("lack of demand") }, Record { config: "core.sparseCheckout", - usage: Planned { note: Some("we want to support huge repos and be the fastest in doing so") }, + usage: Planned("we want to support huge repos and be the fastest in doing so") }, Record { config: "core.sparseCheckoutCone", - usage: Planned { note: Some("this is a nice improvement over spareCheckout alone and should one day be available too") }, + usage: Planned("this is a nice improvement over spareCheckout alone and should one day be available too") }, Record { config: "core.gitProxy", - usage: NotPlanned { reason: "the transport mechanism works differently enough to not support it for now, but of course it's possible to add support if there is demand" }, + usage: NotPlanned("the transport mechanism works differently enough to not support it for now, but of course it's possible to add support if there is demand") }, Record { config: "checkout.defaultRemote", - usage: Planned { note: Some("needed for correct checkout behaviour, similar to what git does") }, + usage: Planned("needed for correct checkout behaviour, similar to what git does") }, Record { config: "core.untrackedCache", - usage: Planned { note: Some("needed for fast worktree operation") }, + usage: Planned("needed for fast worktree operation") }, Record { config: "checkout.guess", - usage: Planned { note: None }, + usage: Planned("") }, Record { config: "checkout.thresholdForParallelism", - usage: NotApplicable {reason: "parallelism is efficient enough to always run with benefit"}, + usage: NotApplicable("parallelism is efficient enough to always run with benefit") }, Record { config: "feature.manyFiles", - usage: Planned {note: Some("big repositories are on the roadmap")}, + usage: Planned("big repositories are on the roadmap") }, Record { config: "core.preloadIndex", - usage: Planned {note: Some("it's enabled by default and allows parallel stat checks - it's using a lot of CPU for just minor performance boosts though")}, + usage: Planned("it's enabled by default and allows parallel stat checks - it's using a lot of CPU for just minor performance boosts though") }, Record { config: "commitGraph.generationVersion", - usage: NotPlanned { reason: "couldn't find a test that would require corrected generation numbers, even `git` has no test for this." }, + usage: NotPlanned("couldn't find a test that would require corrected generation numbers, even `git` has no test for this.") }, Record { config: "commitGraph.maxNewFilters", - usage: NotPlanned { reason: "can be considered when the underlying feature is actually used or needed" }, + usage: NotPlanned("can be considered when the underlying feature is actually used or needed") }, Record { config: "commitGraph.readChangedPaths", - usage: NotPlanned { reason: "can be considered when the underlying feature is actually used or needed" }, + usage: NotPlanned("can be considered when the underlying feature is actually used or needed") }, Record { config: "index.sparse", - usage: Planned {note: Some("we can read sparse indices and support for it will be added early on")}, + usage: Planned("we can read sparse indices and support for it will be added early on") }, Record { config: "merge.renormalize", - usage: Planned {note: Some("once merging is being implemented, renormalization should be respected")}, + usage: Planned("once merging is being implemented, renormalization should be respected") }, Record { config: "sparse.expectFilesOutsideOfPatterns", - usage: Planned {note: Some("a feature definitely worth having")}, + usage: Planned("a feature definitely worth having") }, Record { config: "submodule.recurse", - usage: Planned {note: Some("very relevant for doing the right thing during checkouts. Note that 'clone' isnt' affected by it, even though we could make it so for good measure.")}, + usage: Planned("very relevant for doing the right thing during checkouts. Note that 'clone' isnt' affected by it, even though we could make it so for good measure.") }, Record { config: "submodule.propagateBranches", - usage: NotPlanned {reason: "it is experimental, let's see how it pans out"} + usage: NotPlanned("it is experimental, let's see how it pans out") }, Record { config: "submodule.alternateLocation", - usage: NotPlanned {reason: "not currently supported when we clone either"} + usage: NotPlanned("not currently supported when we clone either") }, Record { config: "submodule.alternateErrorStrategy", - usage: NotPlanned {reason: "not currently supported when we clone either"} + usage: NotPlanned("not currently supported when we clone either") }, Record { config: "submodule.fetchJobs", - usage: Planned {note: Some("relevant for fetching")}, + usage: Planned("relevant for fetching") }, Record { config: "branch.autoSetupRebase", - usage: Planned { - note: Some("for when we allow setting up upstream branches") - }, + usage: Planned("for when we allow setting up upstream branches") }, Record { config: "branch..rebase", - usage: Planned { - note: Some("for when we can merge, rebase should be supported") - }, + usage: Planned("for when we can merge, rebase should be supported") }, Record { config: "branch..description", - usage: NotPlanned { - reason: "no plan to implement format-patch or request-pull summary" - }, + usage: NotPlanned("no plan to implement format-patch or request-pull summary") + }, Record { config: "core.fsync", - usage: Planned {note: Some("more safety for disk write operations is a good thing, definitely on the server")} + usage: Planned("more safety for disk write operations is a good thing, definitely on the server") }, Record { config: "core.fsyncMethod", - usage: Planned {note: Some("needed to support `core.fsync`")} + usage: Planned("needed to support `core.fsync`") }, Record { config: "core.sharedRepository", - usage: NotPlanned {reason: "on demand"} + usage: NotPlanned("on demand") }, Record { config: "core.createObject", - usage: NotPlanned {reason: "it's valuable not to do writes unless needed on the lowest level, but we hope to avoid issues by not writing duplicate objects in the first place"} + usage: NotPlanned("it's valuable not to do writes unless needed on the lowest level, but we hope to avoid issues by not writing duplicate objects in the first place") }, Record { - config: "clone.filterSubmodules,", - usage: Planned { - note: Some("currently object filtering isn't support, a prerequisite for this, see --filter=blob:none for more"), - }, + config: "clone.filterSubmodules,", + usage: Planned("currently object filtering isn't support, a prerequisite for this, see --filter=blob:none for more"), + }, Record { config: "clone.rejectShallow", - usage: Planned { - note: Some("probably trivial to implement once there is protocol support for shallow clones"), - }, + usage: Planned("probably trivial to implement once there is protocol support for shallow clones") }, Record { config: "receive.shallowUpdate", - usage: NotPlanned { - reason: "it looks like a server-only setting that allows boundaries to change if refs are pushed that are outside of the boundary.", - }, + usage: NotPlanned("it looks like a server-only setting that allows boundaries to change if refs are pushed that are outside of the boundary.") }, Record { config: "fetch.recurseSubmodules", - usage: Planned { - note: Some("Seems useful for cargo as well"), - }, + usage: Planned("Seems useful for cargo as well"), + }, Record { config: "fetch.fsckObjects", - usage: Puzzled, + usage: Puzzled }, Record { config: "fetch.fsck.", - usage: Puzzled, + usage: Puzzled }, Record { config: "fetch.fsck.skipList", - usage: Puzzled, + usage: Puzzled }, Record { config: "fetch.unpackLimit", - usage: Planned { note: None }, + usage: Planned("") }, Record { config: "fetch.prune", - usage: Planned { note: None }, + usage: Planned("") }, Record { config: "fetch.pruneTags", - usage: Planned { note: None }, + usage: Planned("") }, Record { config: "fetch.writeCommitGraph", - usage: Planned { note: None }, + usage: Planned("") }, Record { config: "fetch.parallel", - usage: Planned { note: None }, + usage: Planned("") }, Record { config: "fetch.showForcedUpdates", - usage: NotApplicable {reason: "we don't support advices"}, + usage: NotApplicable("we don't support advices") }, Record { config: "fetch.output", - usage: NotPlanned {reason: "'gix' might support it, but there is no intention on copying the 'git' CLI"}, + usage: NotPlanned("'gix' might support it, but there is no intention on copying the 'git' CLI") }, Record { config: "remotes.", - usage: Planned { - note: Some("useful for multi-remote fetches as part of the standard API, maybe just `group(name) -> Option>`"), - }, + usage: Planned("useful for multi-remote fetches as part of the standard API, maybe just `group(name) -> Option>`") + }, Record { config: "advice.updateSparsePath", - usage: NotApplicable { reason: "gitoxide does not yet have an 'advice' system" }, + usage: NotApplicable("gitoxide does not yet have an 'advice' system") }, Record { config: "core.sparseCheckout", - usage: Planned { note: Some("together with 'index.sparse' and 'core.sparseCheckoutCone', configures if the index should be written sparse or not") }, + usage: Planned("together with 'index.sparse' and 'core.sparseCheckoutCone', configures if the index should be written sparse or not") }, Record { config: "core.sparseCheckoutCone", - usage: Planned { note: Some("non-cone mode is deprecated but should still fail gracefully if encountered") }, + usage: Planned("non-cone mode is deprecated but should still fail gracefully if encountered") }, Record { config: "core.splitIndex", - usage: NotPlanned { reason: "we don't want to be able to create split indices, but we will read them. It's (somewhat) superseded by sparse indices" }, + usage: NotPlanned("we don't want to be able to create split indices, but we will read them. It's (somewhat) superseded by sparse indices") }, Record { config: "splitIndex.maxPercentageChange", - usage: NotPlanned { reason: "seems like it's superseded by sparse indices" }, + usage: NotPlanned("seems like it's superseded by sparse indices") }, Record { config: "splitIndex.sharedIndexExpire", - usage: NotPlanned { reason: "seems like it's superseded by sparse indices" }, + usage: NotPlanned("seems like it's superseded by sparse indices") }, Record { config: "index.sparse", - usage: Planned { note: Some("together with 'core.sparseCheckout' and 'core.sparseCheckoutCone', configures if the index should be written sparse or not") }, + usage: Planned("together with 'core.sparseCheckout' and 'core.sparseCheckoutCone', configures if the index should be written sparse or not") }, Record { config: "index.version", - usage: Planned { note: Some("once V4 indices can be written, we need to be able to set a desired version. For now we write the smallest possible index version only.") }, + usage: Planned("once V4 indices can be written, we need to be able to set a desired version. For now we write the smallest possible index version only.") }, Record { config: "http..*", - usage: Planned { note: Some("definitely needed for correctness, testing against baseline is a must") } + usage: Planned("definitely needed for correctness, testing against baseline is a must") }, Record { config: "http.proxySSLCert", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.proxySSLKey", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.proxySSLCertPasswordProtected", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.proxySSLCAInfo", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.emptyAuth", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.delegation", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.cookieFile", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.saveCookies", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.curloptResolve", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.sslCipherList", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.sslCipherList", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.sslCert", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.sslKey", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.sslCertPasswordProtected", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.sslCertPasswordProtected", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.sslCAPath", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.sslBackend", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.pinnedPubkey", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.sslTry", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.maxRequests", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.minSessions", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http.postBuffer", - usage: Planned { note: Some("relevant when implementing push, we should understand how memory allocation works when streaming") } + usage: Planned("relevant when implementing push, we should understand how memory allocation works when streaming") }, Record { config: "http.noEPSV", - usage: NotPlanned { reason: "on demand" } + usage: NotPlanned("on demand") }, Record { config: "http..*", - usage: Planned { note: Some("it's a vital part of git configuration. It's unclear how to get a baseline from git for this one.") } + usage: Planned("it's a vital part of git configuration. It's unclear how to get a baseline from git for this one.") }, Record { config: "init.templateDir", - usage: NotPlanned { reason: "git expects this dir to be a valid git dir - I'd find additive template dirs more interesting, or changes done afterwards procedurally. Maybe this needs a 'init_or_open' semantic to be really useful" } + usage: NotPlanned("git expects this dir to be a valid git dir - I'd find additive template dirs more interesting, or changes done afterwards procedurally. Maybe this needs a 'init_or_open' semantic to be really useful") }, Record { config: "sparse.expectFilesOutsideOfPatterns", - usage: NotPlanned { reason: "todo" }, + usage: NotPlanned("todo") }, Record { config: "remote..promisor", - usage: Planned { - note: Some("required for big monorepos, and typically used in conjunction with sparse indices") - } + usage: Planned("required for big monorepos, and typically used in conjunction with sparse indices") }, Record { config: "remote..partialCloneFilter", - usage: Planned { - note: Some("required for big monorepos, and typically used in conjunction with sparse indices") - } + usage: Planned("required for big monorepos, and typically used in conjunction with sparse indices") }, Record { config: "merge.renameLimit", - usage: Planned { note: Some("The same as diff.renameLimit") } + usage: Planned("The same as diff.renameLimit") }, Record { config: "merge.renames", - usage: Planned { note: Some("The same as diff.renames") } + usage: Planned("The same as diff.renames") }, Record { config: "status.renameLimit", - usage: Planned { note: Some("definitely needed to do status properly, even though it doesn't have to be there for day one. The same as diff.renameLimit") } + usage: Planned("definitely needed to do status properly, even though it doesn't have to be there for day one. The same as diff.renameLimit") }, Record { config: "status.renames", - usage: Planned { note: Some("the same as diff.renames") } + usage: Planned("the same as diff.renames") }, Record { config: "transfer.credentialsInUrl", - usage: Planned { note: Some("currently we are likely to expose passwords in errors or in other places, and it's better to by default not do that") } + usage: Planned("currently we are likely to expose passwords in errors or in other places, and it's better to by default not do that") }, Record { config: "diff.*.cachetextconv", - usage: NotPlanned {reason: "It seems to slow to do that, and persisting results to save a relatively cheap computation doesn't seem right"} + usage: NotPlanned("It seems to slow to do that, and persisting results to save a relatively cheap computation doesn't seem right") }, ]; @@ -480,18 +458,22 @@ pub fn show_progress() -> anyhow::Result<()> { gix::config::tree::Note::Deviation(n) | gix::config::tree::Note::Informative(n) => n.to_string(), }); let link = key.link().map(|link| match link { - gix::config::tree::Link::FallbackKey(key) => format!("fallback is '{}'", key.logical_name()), + gix::config::tree::Link::FallbackKey(key) => { + format!("fallback is '{fallback}'", fallback = key.logical_name()) + } gix::config::tree::Link::EnvironmentOverride(name) => format!("overridden by '{name}'"), }); + let deviation = match (note, link) { - (Some(n), Some(l)) => Some(format!("{n}. {l}")), - (Some(n), None) | (None, Some(n)) => Some(n), - (None, None) => None, - } - .map(|d| &*Box::leak(d.into_boxed_str())); + (Some(n), Some(l)) => format!("{n}. {l}"), + (Some(n), None) | (None, Some(n)) => n, + (None, None) => "".to_string(), + }; + + let deviation = &*Box::leak(deviation.into_boxed_str()); Record { config: Box::leak(config.into_boxed_str()), - usage: InUse { deviation }, + usage: InUse(deviation), } } section @@ -505,53 +487,71 @@ pub fn show_progress() -> anyhow::Result<()> { v }; - let mut buf = String::new(); - use std::fmt::Write; - writeln!(&mut buf, - "\nTotal records: {} ({perfect_icon} = {perfect}, {deviation_icon} = {deviation}, {planned_icon} = {planned}, {ondemand_icon} = {ondemand}, {not_applicable_icon} = {not_applicable})", - sorted.len(), - perfect_icon = InUse { - deviation: None - } - .icon(), - deviation_icon = InUse { - deviation: Some("") + let mut perfect = 0; + let mut deviation = 0; + let mut notplanned = 0; + let mut not_applicable = 0; + let mut planned = 0; + + for s in &sorted { + match s.usage { + NotApplicable(_) => not_applicable += 1, + NotPlanned(_) => notplanned += 1, + Planned(_) => planned += 1, + InUse(dev) => { + if dev.is_empty() { + perfect += 1; + } else { + deviation += 1; + } + } + Puzzled => {} } - .icon(), - planned_icon = Planned { note: None }.icon(), - planned = sorted.iter().filter(|e| matches!(e.usage, Planned { .. })).count(), - ondemand_icon = NotPlanned { reason: "" }.icon(), - not_applicable_icon = NotApplicable { reason: "" }.icon(), - perfect = sorted - .iter() - .filter(|e| matches!(e.usage, InUse { deviation, .. } if deviation.is_none())) - .count(), - deviation = sorted - .iter() - .filter(|e| matches!(e.usage, InUse { deviation, .. } if deviation.is_some())) - .count(), - ondemand = sorted - .iter() - .filter(|e| matches!(e.usage, NotPlanned { .. })) - .count(), - not_applicable = sorted - .iter() - .filter(|e| matches!(e.usage, NotApplicable { .. })) - .count() - )?; + } - for Record { config, usage } in sorted { - println!( - "{icon} {config: <50}: {usage}", - icon = usage.icon(), - config = if let Some(config) = config.strip_prefix("gitoxide.") { - format!("{gitoxide}{config}", gitoxide = "gitoxide.") - } else { - config.to_string() + let width: Option = terminal_size::terminal_size() + .map(|(width, _height)| width.0) + .map(|w| w.into()); + + let mut stdout = std::io::stdout().lock(); + for Record { config, usage } in &sorted { + use std::io::Write; + write!(stdout, "{icon} {config: <50}: ", icon = usage.icon())?; + + if let Some(width) = width { + let icon_and_config_width = 55; + let width_after_config = width - icon_and_config_width; + let usage = usage.to_string(); + let mut usage = usage.split(" "); + let mut idx = 0; + while let Some(word) = usage.next() { + // +1 for the space after each word + let word_len = word.chars().count() + 1; + + if idx + word_len > width_after_config { + write!(stdout, "\n")?; + for _ in 0..icon_and_config_width { + write!(stdout, " ")?; + } + idx = 0; + } + + write!(stdout, "{word} ")?; + idx += word_len; } - ); + writeln!(stdout, "")?; + } else { + writeln!(stdout, "{usage}")?; + } } - println!("{buf}"); + println!("\nTotal records: {nr_sorted} ({perfect_icon} = {perfect}, {deviation_icon} = {deviation}, {planned_icon} = {planned}, {ondemand_icon} = {notplanned}, {not_applicable_icon} = {not_applicable})", + nr_sorted = sorted.len(), + perfect_icon = InUse("").icon(), + deviation_icon = InUse("dev").icon(), + planned_icon = Planned("").icon(), + ondemand_icon = NotPlanned("").icon(), + not_applicable_icon = NotApplicable("").icon(), + ); Ok(()) } From 454e6b9de86def27c811cc93fd80f4eb169dda52 Mon Sep 17 00:00:00 2001 From: Benedikt Mandelkow Date: Fri, 16 Feb 2024 20:42:00 +0100 Subject: [PATCH 6/7] fix clippy lints --- src/plumbing/progress.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/plumbing/progress.rs b/src/plumbing/progress.rs index cb00154cdf6..b0a217b3f07 100644 --- a/src/plumbing/progress.rs +++ b/src/plumbing/progress.rs @@ -511,7 +511,7 @@ pub fn show_progress() -> anyhow::Result<()> { let width: Option = terminal_size::terminal_size() .map(|(width, _height)| width.0) - .map(|w| w.into()); + .map(std::convert::Into::into); let mut stdout = std::io::stdout().lock(); for Record { config, usage } in &sorted { @@ -522,14 +522,13 @@ pub fn show_progress() -> anyhow::Result<()> { let icon_and_config_width = 55; let width_after_config = width - icon_and_config_width; let usage = usage.to_string(); - let mut usage = usage.split(" "); let mut idx = 0; - while let Some(word) = usage.next() { + for word in usage.split(' ') { // +1 for the space after each word let word_len = word.chars().count() + 1; if idx + word_len > width_after_config { - write!(stdout, "\n")?; + writeln!(stdout)?; for _ in 0..icon_and_config_width { write!(stdout, " ")?; } @@ -539,7 +538,7 @@ pub fn show_progress() -> anyhow::Result<()> { write!(stdout, "{word} ")?; idx += word_len; } - writeln!(stdout, "")?; + writeln!(stdout)?; } else { writeln!(stdout, "{usage}")?; } From e25d7eb443d9e1fde8d16422251ee0d288ff4a51 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 16 Feb 2024 21:14:08 +0100 Subject: [PATCH 7/7] refactor - fix subtract with overflow - extract line wrapping into function --- src/plumbing/progress.rs | 46 +++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/src/plumbing/progress.rs b/src/plumbing/progress.rs index b0a217b3f07..4444370b795 100644 --- a/src/plumbing/progress.rs +++ b/src/plumbing/progress.rs @@ -1,4 +1,5 @@ use std::fmt::{Display, Formatter}; +use std::io::StdoutLock; #[derive(Clone)] enum Usage { @@ -519,26 +520,7 @@ pub fn show_progress() -> anyhow::Result<()> { write!(stdout, "{icon} {config: <50}: ", icon = usage.icon())?; if let Some(width) = width { - let icon_and_config_width = 55; - let width_after_config = width - icon_and_config_width; - let usage = usage.to_string(); - let mut idx = 0; - for word in usage.split(' ') { - // +1 for the space after each word - let word_len = word.chars().count() + 1; - - if idx + word_len > width_after_config { - writeln!(stdout)?; - for _ in 0..icon_and_config_width { - write!(stdout, " ")?; - } - idx = 0; - } - - write!(stdout, "{word} ")?; - idx += word_len; - } - writeln!(stdout)?; + write_with_linewrap(&mut stdout, &usage.to_string(), width)?; } else { writeln!(stdout, "{usage}")?; } @@ -554,3 +536,27 @@ pub fn show_progress() -> anyhow::Result<()> { ); Ok(()) } + +fn write_with_linewrap(stdout: &mut StdoutLock<'_>, text: &str, width: usize) -> Result<(), std::io::Error> { + use std::io::Write; + let icon_and_config_width = 55; + let width_after_config = width.saturating_sub(icon_and_config_width); + let mut idx = 0; + for word in text.split(' ') { + // +1 for the space after each word + let word_len = word.chars().count() + 1; + + if idx + word_len > width_after_config { + writeln!(stdout)?; + for _ in 0..icon_and_config_width { + write!(stdout, " ")?; + } + idx = 0; + } + + write!(stdout, "{word} ")?; + idx += word_len; + } + writeln!(stdout)?; + Ok(()) +}