From f29237cbcaf0532b34017951e581db1ddf4473f8 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 18 Nov 2025 22:55:06 +0100 Subject: [PATCH 01/21] Split fn format_commit Rewrite format_commit to use a formatter struct. Use push_str instead of write macro to simplfy further. --- src/print/format.rs | 536 +++++++++++++++++++++----------------------- 1 file changed, 253 insertions(+), 283 deletions(-) diff --git a/src/print/format.rs b/src/print/format.rs index bc44e51..9dda684 100644 --- a/src/print/format.rs +++ b/src/print/format.rs @@ -83,14 +83,14 @@ pub fn format_commit( wrapping: &Option, hash_color: Option, ) -> Result, String> { + // Find replacements let mut replacements = vec![]; - for (idx, arr) in PLACEHOLDERS.iter().enumerate() { let mut curr = 0; loop { let mut found = false; for (mode, str) in arr.iter().enumerate() { - if let Some(start) = &format[curr..format.len()].find(str) { + if let Some(start) = &format[curr..].find(str) { replacements.push((curr + start, str.len(), idx, mode)); curr += start + str.len(); found = true; @@ -102,296 +102,266 @@ pub fn format_commit( } } } - replacements.sort_by_key(|p| p.0); + // Generate formatted lines let mut lines = vec![]; let mut out = String::new(); - if replacements.is_empty() { - write!(out, "{}", format).unwrap(); - add_line(&mut lines, &mut out, wrapping); - } else { - let mut curr = 0; - for (start, len, idx, mode) in replacements { - if idx == NEW_LINE { - write!(out, "{}", &format[curr..start]).unwrap(); - add_line(&mut lines, &mut out, wrapping); - } else { - write!(out, "{}", &format[curr..start]).unwrap(); - let id = commit.id(); - match idx { - HASH => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - if let Some(color) = hash_color { - write!(out, "{}", id.to_string().fixed(color)) - } else { - write!(out, "{}", id) - } - } - HASH_ABBREV => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - if let Some(color) = hash_color { - write!(out, "{}", id.to_string()[..7].fixed(color)) - } else { - write!(out, "{}", &id.to_string()[..7]) - } - } - PARENT_HASHES => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - for i in 0..commit.parent_count() { - write!(out, "{}", commit.parent_id(i).unwrap()).unwrap(); - if i < commit.parent_count() - 1 { - write!(out, " ").unwrap(); - } - } - Ok(()) - } - PARENT_HASHES_ABBREV => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - for i in 0..commit.parent_count() { - write!( - out, - "{}", - &commit - .parent_id(i) - .map_err(|err| err.to_string())? - .to_string()[..7] - ) - .unwrap(); - if i < commit.parent_count() - 1 { - write!(out, " ").unwrap(); - } - } - Ok(()) - } - REFS => { - match mode { - MODE_SPACE => { - if !branches.is_empty() { - write!(out, " ").unwrap() - } - } - MODE_PLUS => { - if !branches.is_empty() { - add_line(&mut lines, &mut out, wrapping) - } - } - MODE_MINUS => { - if branches.is_empty() { - out = remove_empty_lines(&mut lines, out) - } - } - _ => {} - } - write!(out, "{}", branches) - } - SUBJECT => { - let summary = commit.summary().unwrap_or(""); - match mode { - MODE_SPACE => { - if !summary.is_empty() { - write!(out, " ").unwrap() - } - } - MODE_PLUS => { - if !summary.is_empty() { - add_line(&mut lines, &mut out, wrapping) - } - } - MODE_MINUS => { - if summary.is_empty() { - out = remove_empty_lines(&mut lines, out) - } - } - _ => {} - } - write!(out, "{}", summary) - } - AUTHOR => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - write!(out, "{}", &commit.author().name().unwrap_or("")) - } - AUTHOR_EMAIL => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - write!(out, "{}", &commit.author().email().unwrap_or("")) - } - AUTHOR_DATE => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - write!( - out, - "{}", - format_date(commit.author().when(), "%a %b %e %H:%M:%S %Y %z") - ) - } - AUTHOR_DATE_SHORT => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - write!(out, "{}", format_date(commit.author().when(), "%F")) - } - AUTHOR_DATE_RELATIVE => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - write!(out, "{}", format_relative_time(commit.author().when())) - } - COMMITTER => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - write!(out, "{}", &commit.committer().name().unwrap_or("")) - } - COMMITTER_EMAIL => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - write!(out, "{}", &commit.committer().email().unwrap_or("")) - } - COMMITTER_DATE => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - write!( - out, - "{}", - format_date(commit.committer().when(), "%a %b %e %H:%M:%S %Y %z") - ) - } - COMMITTER_DATE_SHORT => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - write!(out, "{}", format_date(commit.committer().when(), "%F")) - } - COMMITTER_DATE_RELATIVE => { - match mode { - MODE_SPACE => write!(out, " ").unwrap(), - MODE_PLUS => add_line(&mut lines, &mut out, wrapping), - _ => {} - } - write!(out, "{}", format_relative_time(commit.committer().when())) - } - BODY => { - let message = commit - .message() - .unwrap_or("") - .lines() - .collect::>(); - - let num_parts = message.len(); - match mode { - MODE_SPACE => { - if num_parts > 2 { - write!(out, " ").unwrap() - } - } - MODE_PLUS => { - if num_parts > 2 { - add_line(&mut lines, &mut out, wrapping) - } - } - MODE_MINUS => { - if num_parts <= 2 { - out = remove_empty_lines(&mut lines, out) - } - } - _ => {} - } - for (cnt, line) in message.iter().enumerate() { - if cnt > 1 && (cnt < num_parts - 1 || !line.is_empty()) { - write!(out, "{}", line).unwrap(); - add_line(&mut lines, &mut out, wrapping); - } - } - Ok(()) - } - BODY_RAW => { - let message = commit - .message() - .unwrap_or("") - .lines() - .collect::>(); - - let num_parts = message.len(); - - match mode { - MODE_SPACE => { - if !message.is_empty() { - write!(out, " ").unwrap() - } - } - MODE_PLUS => { - if !message.is_empty() { - add_line(&mut lines, &mut out, wrapping) - } - } - MODE_MINUS => { - if message.is_empty() { - out = remove_empty_lines(&mut lines, out) - } - } - _ => {} - } - for (cnt, line) in message.iter().enumerate() { - if cnt < num_parts - 1 || !line.is_empty() { - write!(out, "{}", line).unwrap(); - add_line(&mut lines, &mut out, wrapping); - } - } - Ok(()) - } - x => return Err(format!("No commit field at index {}", x)), + let mut formatter = CommitFieldFormatter::new( + commit, &branches, hash_color, wrapping, &mut out, &mut lines, + ); + + let mut curr = 0; + for &(start, len, idx, mode) in &replacements { + formatter.out.push_str(&format[curr..start]); + formatter.format_field(idx, mode)?; + curr = start + len; + } + formatter.out.push_str(&format[curr..]); + formatter.finalize_tail(); + Ok(lines) +} + +struct CommitFieldFormatter<'a> { + commit: &'a Commit<'a>, + branches: &'a str, + hash_color: Option, + wrapping: &'a Option>, + out: &'a mut String, + lines: &'a mut Vec, +} + +impl<'a> CommitFieldFormatter<'a> { + fn new( + commit: &'a Commit, + branches: &'a str, + hash_color: Option, + wrapping: &'a Option, + out: &'a mut String, + lines: &'a mut Vec, + ) -> Self { + Self { + commit, + branches, + hash_color, + wrapping, + out, + lines, + } + } + + fn handle_mode(&mut self, mode: usize, field_has_content: bool) { + match mode { + MODE_SPACE => { + if field_has_content { + self.out.push(' '); } - .unwrap(); } - curr = start + len; + MODE_PLUS => { + if field_has_content { + self.add_line(); + } + } + MODE_MINUS => { + if !field_has_content { + *self.out = remove_empty_lines(self.lines, self.out.clone()); + } + } + _ => {} } - write!(out, "{}", &format[curr..(format.len())]).unwrap(); - if !out.is_empty() { - add_line(&mut lines, &mut out, wrapping); + } + + pub fn format_field(&mut self, idx: usize, mode: usize) -> Result<(), String> { + match idx { + NEW_LINE => self.add_line(), + HASH => self.format_hash(mode), + HASH_ABBREV => self.format_hash_abbrev(mode), + PARENT_HASHES => self.format_parent_hashes(mode)?, + PARENT_HASHES_ABBREV => self.format_parent_hashes_abbrev(mode)?, + REFS => self.format_refs(mode), + SUBJECT => self.format_subject(mode)?, + AUTHOR => self.format_author(mode), + AUTHOR_EMAIL => self.format_author_email(mode), + AUTHOR_DATE => self.format_author_date(mode), + AUTHOR_DATE_SHORT => self.format_author_date_short(mode), + AUTHOR_DATE_RELATIVE => self.format_author_date_relative(mode), + COMMITTER => self.format_committer(mode), + COMMITTER_EMAIL => self.format_committer_email(mode), + COMMITTER_DATE => self.format_committer_date(mode), + COMMITTER_DATE_SHORT => self.format_committer_date_short(mode), + COMMITTER_DATE_RELATIVE => self.format_committer_date_relative(mode), + BODY => self.format_body(mode)?, + BODY_RAW => self.format_body_raw(mode)?, + x => return Err(format!("No commit field at index {}", x)), } + Ok(()) + } + + fn add_line(&mut self) { + if !self.out.is_empty() { + add_line(self.lines, self.out, self.wrapping); + } + } + + pub fn finalize_tail(&mut self) { + self.add_line(); + } + + fn format_hash(&mut self, mode: usize) { + self.handle_mode(mode, true); + let id = self.commit.id(); + if let Some(color) = self.hash_color { + self.out.push_str(&id.to_string().fixed(color).to_string()); + } else { + self.out.push_str(&id.to_string()); + } + } + + fn format_hash_abbrev(&mut self, mode: usize) { + self.handle_mode(mode, true); + let id = self.commit.id().to_string(); + let s = &id[..7]; + if let Some(color) = self.hash_color { + self.out.push_str(&s.fixed(color).to_string()); + } else { + self.out.push_str(s); + } + } + + fn format_parent_hashes(&mut self, mode: usize) -> Result<(), String> { + self.handle_mode(mode, true); + for i in 0..self.commit.parent_count() { + self.out + .push_str(&self.commit.parent_id(i).unwrap().to_string()); + if i < self.commit.parent_count() - 1 { + self.out.push(' '); + } + } + Ok(()) + } + + fn format_parent_hashes_abbrev(&mut self, mode: usize) -> Result<(), String> { + self.handle_mode(mode, true); + for i in 0..self.commit.parent_count() { + let parent_id = self.commit.parent_id(i).map_err(|err| err.to_string())?; + self.out.push_str(&parent_id.to_string()[..7]); + if i < self.commit.parent_count() - 1 { + self.out.push(' '); + } + } + Ok(()) + } + + fn format_refs(&mut self, mode: usize) { + self.handle_mode(mode, !self.branches.is_empty()); + self.out.push_str(self.branches); + } + + fn format_subject(&mut self, mode: usize) -> Result<(), String> { + let summary = self.commit.summary().unwrap_or(""); + self.handle_mode(mode, !summary.is_empty()); + self.out.push_str(summary); + Ok(()) + } + + fn format_author(&mut self, mode: usize) { + self.handle_mode(mode, true); + self.out.push_str(self.commit.author().name().unwrap_or("")); + } + + fn format_author_email(&mut self, mode: usize) { + self.handle_mode(mode, true); + self.out + .push_str(self.commit.author().email().unwrap_or("")); + } + + fn format_author_date(&mut self, mode: usize) { + self.handle_mode(mode, true); + self.out.push_str(&format_date( + self.commit.author().when(), + "%a %b %e %H:%M:%S %Y %z", + )); + } + + fn format_author_date_short(&mut self, mode: usize) { + self.handle_mode(mode, true); + self.out + .push_str(&format_date(self.commit.author().when(), "%F")); + } + + fn format_author_date_relative(&mut self, mode: usize) { + self.handle_mode(mode, true); + self.out + .push_str(&format_relative_time(self.commit.author().when())); + } + + fn format_committer(&mut self, mode: usize) { + self.handle_mode(mode, true); + self.out + .push_str(self.commit.committer().name().unwrap_or("")); + } + + fn format_committer_email(&mut self, mode: usize) { + self.handle_mode(mode, true); + self.out + .push_str(self.commit.committer().email().unwrap_or("")); + } + + fn format_committer_date(&mut self, mode: usize) { + self.handle_mode(mode, true); + self.out.push_str(&format_date( + self.commit.committer().when(), + "%a %b %e %H:%M:%S %Y %z", + )); + } + + fn format_committer_date_short(&mut self, mode: usize) { + self.handle_mode(mode, true); + self.out + .push_str(&format_date(self.commit.committer().when(), "%F")); + } + + fn format_committer_date_relative(&mut self, mode: usize) { + self.handle_mode(mode, true); + self.out + .push_str(&format_relative_time(self.commit.committer().when())); + } + + fn format_body(&mut self, mode: usize) -> Result<(), String> { + let message = self + .commit + .message() + .unwrap_or("") + .lines() + .collect::>(); + let num_parts = message.len(); + self.handle_mode(mode, num_parts > 2); + for (cnt, line) in message.iter().enumerate() { + if cnt > 1 && (cnt < num_parts - 1 || !line.is_empty()) { + self.out.push_str(line); + self.add_line(); + } + } + Ok(()) + } + + fn format_body_raw(&mut self, mode: usize) -> Result<(), String> { + let message = self + .commit + .message() + .unwrap_or("") + .lines() + .collect::>(); + let num_parts = message.len(); + self.handle_mode(mode, !message.is_empty()); + for (cnt, line) in message.iter().enumerate() { + if cnt < num_parts - 1 || !line.is_empty() { + self.out.push_str(line); + self.add_line(); + } + } + Ok(()) } - Ok(lines) } /// Format a commit for `CommitFormat::OneLine`. From 185c46b426a4ea746122c5ce8f7885928bb428a6 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 25 Nov 2025 11:18:59 +0100 Subject: [PATCH 02/21] define Sessions struct This holds all data needed to run the application. --- src/main.rs | 42 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index 1146205..93d141e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -34,6 +34,8 @@ fn from_args() -> Result<(), String> { create_config(&models_dir)?; + let mut ses = Session::new(); + let app = Command::new("git-graph") .version(crate_version!()) .about( @@ -270,7 +272,7 @@ fn from_args() -> Result<(), String> { return Ok(()); } - let commit_limit = match matches.get_one::("max-count") { + ses.commit_limit = match matches.get_one::("max-count") { None => None, Some(str) => match str.parse::() { Ok(val) => Some(val), @@ -287,7 +289,7 @@ fn from_args() -> Result<(), String> { let reverse_commit_order = matches.get_flag("reverse"); - let svg = matches.get_flag("svg"); + ses.svg = matches.get_flag("svg"); let compact = !matches.get_flag("sparse"); let debug = matches.get_flag("debug"); let style = matches @@ -390,6 +392,8 @@ fn from_args() -> Result<(), String> { Some((None, Some(0), Some(8))) }; + ses.repository = Some(repository); + let settings = Settings { reverse_commit_order, debug, @@ -404,7 +408,39 @@ fn from_args() -> Result<(), String> { merge_patterns: MergePatterns::default(), }; - run(repository, &settings, svg, commit_limit) + run( + ses.repository.unwrap(), + &settings, + ses.svg, + ses.commit_limit, + ) +} + +struct Session { + // Settings related fields + // TODO + + // Other fields + pub repository: Option, + pub svg: bool, + pub commit_limit: Option, +} + +impl Session { + pub fn new() -> Self { + Self { + // models related fields + // TODO + + // Settings related fields + // TODO + + // Other fields + repository: None, + svg: false, + commit_limit: None, + } + } } fn run( From 79ac33b153f3fe9e3453317ff14a3750529b0923 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Wed, 26 Nov 2025 11:43:57 +0100 Subject: [PATCH 03/21] Configure clippy to warn on complex functions Make sure CI will prevent a function from growing by enabling the relevant clippy warning. --- src/lib.rs | 5 ----- src/main.rs | 4 ++++ 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1e044dc..b14c59f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,14 +7,9 @@ //! 2. Lay out the graph structure according to the branching model (See [graph]) //! 3. Render the layout to text or SVG (See [mod@print]) -/* TODO git-graph has some complex functions, which make it hard to -understand and modify the code. The code should be made simpler -so the following warnings can be enabled without triggering. - // Configure clippy to look for complex functions #![warn(clippy::cognitive_complexity)] #![warn(clippy::too_many_lines)] -*/ use git2::Repository; use std::path::Path; diff --git a/src/main.rs b/src/main.rs index 1146205..e1d6b75 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,5 +1,9 @@ //! Command line tool to show clear git graphs arranged for your branching model. +// Configure clippy to look for complex functions +#![warn(clippy::cognitive_complexity)] +#![warn(clippy::too_many_lines)] + use clap::{crate_version, Arg, Command}; use git2::Repository; use git_graph::config::{ From c733155465ca1bcc8e8b7baebb3ad704e2953cad Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 25 Nov 2025 10:58:05 +0100 Subject: [PATCH 04/21] split from_args - repo --- src/main.rs | 82 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 47 insertions(+), 35 deletions(-) diff --git a/src/main.rs b/src/main.rs index 93d141e..f1f4377 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,5 +1,6 @@ //! Command line tool to show clear git graphs arranged for your branching model. +use clap::ArgMatches; use clap::{crate_version, Arg, Command}; use git2::Repository; use git_graph::config::{ @@ -36,10 +37,8 @@ fn from_args() -> Result<(), String> { let mut ses = Session::new(); - let app = Command::new("git-graph") - .version(crate_version!()) - .about( - "Structured Git graphs for your branching model.\n \ + let app = Command::new("git-graph").version(crate_version!()).about( + "Structured Git graphs for your branching model.\n \ https://github.com/mlange-42/git-graph\n\ \n\ EXAMPES:\n \ @@ -49,7 +48,9 @@ fn from_args() -> Result<(), String> { git-graph model --list -> List available branching models\n \ git-graph model -> Show repo's current branching models\n \ git-graph model -> Permanently set model for this repo", - ) + ); + let app = add_repo_args(app); + let app = app .arg( Arg::new("reverse") .long("reverse") @@ -58,14 +59,6 @@ fn from_args() -> Result<(), String> { .required(false) .num_args(0), ) - .arg( - Arg::new("path") - .long("path") - .short('p') - .help("Open repository from this path or above. Default '.'") - .required(false) - .num_args(1), - ) .arg( Arg::new("max-count") .long("max-count") @@ -206,15 +199,6 @@ fn from_args() -> Result<(), String> { .required(false) .num_args(1), ) - .arg( - Arg::new("skip-repo-owner-validation") - .long("skip-repo-owner-validation") - .help("Skip owner validation for the repository.\n\ - This will turn off libgit2's owner validation, which may increase security risks.\n\ - Please do not disable this validation for repositories you do not trust.") - .required(false) - .num_args(0) - ) .subcommand(Command::new("model") .about("Prints or permanently sets the branching model for a repository.") .arg( @@ -246,26 +230,20 @@ fn from_args() -> Result<(), String> { } } - let skip_repo_owner_validation = matches.get_flag("skip-repo-owner-validation"); - if skip_repo_owner_validation { - println!("Warning: skip-repo-owner-validation is set! "); - } - let dot = ".".to_string(); - let path = matches.get_one::("path").unwrap_or(&dot); - let repository = get_repo(path, skip_repo_owner_validation) - .map_err(|err| format!("ERROR: {}\n Navigate into a repository before running git-graph, or use option --path", err.message()))?; + match_repo_args(&mut ses, &matches)?; if let Some(matches) = matches.subcommand_matches("model") { + let repository = ses.repository.as_ref().unwrap(); match matches.get_one::("model") { None => { - let curr_model = get_model_name(&repository, REPO_CONFIG_FILE)?; + let curr_model = get_model_name(repository, REPO_CONFIG_FILE)?; match curr_model { None => print!("No branching model set"), Some(model) => print!("{}", model), } } Some(model) => { - set_model(&repository, model, REPO_CONFIG_FILE, &models_dir)?; + set_model(repository, model, REPO_CONFIG_FILE, &models_dir)?; eprint!("Branching model set to '{}'", model); } }; @@ -304,7 +282,7 @@ fn from_args() -> Result<(), String> { }; let model = get_model( - &repository, + ses.repository.as_ref().unwrap(), matches.get_one::("model").map(|s| &s[..]), REPO_CONFIG_FILE, &models_dir, @@ -392,8 +370,6 @@ fn from_args() -> Result<(), String> { Some((None, Some(0), Some(8))) }; - ses.repository = Some(repository); - let settings = Settings { reverse_commit_order, debug, @@ -443,6 +419,42 @@ impl Session { } } +fn add_repo_args(app: Command) -> Command { + app.arg( + Arg::new("path") + .long("path") + .short('p') + .help("Open repository from this path or above. Default '.'") + .required(false) + .num_args(1), + ) + .arg( + Arg::new("skip-repo-owner-validation") + .long("skip-repo-owner-validation") + .help( + "Skip owner validation for the repository.\n\ + This will turn off libgit2's owner validation, which may increase security risks.\n\ + Please do not disable this validation for repositories you do not trust.", + ) + .required(false) + .num_args(0), + ) +} + +fn match_repo_args(ses: &mut Session, matches: &ArgMatches) -> Result<(), String> { + let skip_repo_owner_validation = matches.get_flag("skip-repo-owner-validation"); + if skip_repo_owner_validation { + println!("Warning: skip-repo-owner-validation is set! "); + } + let default_path = ".".to_string(); + let path = matches.get_one::("path").unwrap_or(&default_path); + let repository = get_repo(path, skip_repo_owner_validation) + .map_err(|err| format!("ERROR: {}\n Navigate into a repository before running git-graph, or use option --path", err.message()))?; + + ses.repository = Some(repository); + Ok(()) +} + fn run( repository: Repository, settings: &Settings, From b532f71c94d45e4938e1a9240fdd27cd442ff678 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 25 Nov 2025 11:58:04 +0100 Subject: [PATCH 05/21] split from_args - model subcommand --- src/main.rs | 181 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 114 insertions(+), 67 deletions(-) diff --git a/src/main.rs b/src/main.rs index f1f4377..7fc028a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,8 +11,11 @@ use git_graph::graph::GitGraph; use git_graph::print::format::CommitFormat; use git_graph::print::svg::print_svg; use git_graph::print::unicode::print_unicode; -use git_graph::settings::{BranchOrder, BranchSettings, Characters, MergePatterns, Settings}; +use git_graph::settings::{ + BranchOrder, BranchSettings, BranchSettingsDef, Characters, MergePatterns, Settings, +}; use platform_dirs::AppDirs; +use std::path::PathBuf; use std::str::FromStr; use std::time::Instant; @@ -29,13 +32,8 @@ fn main() { } fn from_args() -> Result<(), String> { - let app_dir = AppDirs::new(Some("git-graph"), false).unwrap().config_dir; - let mut models_dir = app_dir; - models_dir.push("models"); - - create_config(&models_dir)?; - let mut ses = Session::new(); + store_default_models(&mut ses)?; let app = Command::new("git-graph").version(crate_version!()).about( "Structured Git graphs for your branching model.\n \ @@ -50,6 +48,8 @@ fn from_args() -> Result<(), String> { git-graph model -> Permanently set model for this repo", ); let app = add_repo_args(app); + let app = add_model_args(app); + let app = app .arg( Arg::new("reverse") @@ -68,17 +68,6 @@ fn from_args() -> Result<(), String> { .num_args(1) .value_name("n"), ) - .arg( - Arg::new("model") - .long("model") - .short('m') - .help("Branching model. Available presets are [simple|git-flow|none].\n\ - Default: git-flow. \n\ - Permanently set the model for a repository with\n\ - > git-graph model ") - .required(false) - .num_args(1), - ) .arg( Arg::new("local") .long("local") @@ -199,55 +188,18 @@ fn from_args() -> Result<(), String> { .required(false) .num_args(1), ) - .subcommand(Command::new("model") - .about("Prints or permanently sets the branching model for a repository.") - .arg( - Arg::new("model") - .help("The branching model to be used. Available presets are [simple|git-flow|none].\n\ - When not given, prints the currently set model.") - .value_name("model") - .num_args(1) - .required(false) - .index(1)) - .arg( - Arg::new("list") - .long("list") - .short('l') - .help("List all available branching models.") - .required(false) - .num_args(0), - )); + ; let matches = app.get_matches(); - if let Some(matches) = matches.subcommand_matches("model") { - if matches.get_flag("list") { - println!( - "{}", - itertools::join(get_available_models(&models_dir)?, "\n") - ); - return Ok(()); - } + if match_model_list(&mut ses, &matches)? { + return Ok(()); // Exit after showing model list } match_repo_args(&mut ses, &matches)?; - if let Some(matches) = matches.subcommand_matches("model") { - let repository = ses.repository.as_ref().unwrap(); - match matches.get_one::("model") { - None => { - let curr_model = get_model_name(repository, REPO_CONFIG_FILE)?; - match curr_model { - None => print!("No branching model set"), - Some(model) => print!("{}", model), - } - } - Some(model) => { - set_model(repository, model, REPO_CONFIG_FILE, &models_dir)?; - eprint!("Branching model set to '{}'", model); - } - }; - return Ok(()); + if match_model_subcommand(&mut ses, &matches)? { + return Ok(()); // Exit after model subcommand } ses.commit_limit = match matches.get_one::("max-count") { @@ -281,12 +233,7 @@ fn from_args() -> Result<(), String> { style }; - let model = get_model( - ses.repository.as_ref().unwrap(), - matches.get_one::("model").map(|s| &s[..]), - REPO_CONFIG_FILE, - &models_dir, - )?; + let model = match_model_opt(&mut ses, &matches)?; let format = match matches.get_one::("format") { None => CommitFormat::OneLine, @@ -393,6 +340,9 @@ fn from_args() -> Result<(), String> { } struct Session { + // models related fields + models_dir: PathBuf, + // Settings related fields // TODO @@ -406,7 +356,7 @@ impl Session { pub fn new() -> Self { Self { // models related fields - // TODO + models_dir: PathBuf::new(), // Settings related fields // TODO @@ -455,6 +405,103 @@ fn match_repo_args(ses: &mut Session, matches: &ArgMatches) -> Result<(), String Ok(()) } +// +// "model" subcommand +// + +/// Fill APP_dir/git-graph folder with default models +fn store_default_models(ses: &mut Session) -> Result<(), String> { + let app_dir = AppDirs::new(Some("git-graph"), false).unwrap().config_dir; + let mut models_dir = app_dir; + models_dir.push("models"); + + create_config(&models_dir)?; + + ses.models_dir = models_dir; + Ok(()) +} + +fn add_model_args(app: Command) -> Command { + app + .arg( + Arg::new("model") + .long("model") + .short('m') + .help("Branching model. Available presets are [simple|git-flow|none].\n\ + Default: git-flow. \n\ + Permanently set the model for a repository with\n\ + > git-graph model ") + .required(false) + .num_args(1), + ) + .subcommand(Command::new("model") + .about("Prints or permanently sets the branching model for a repository.") + .arg( + Arg::new("model") + .help("The branching model to be used. Available presets are [simple|git-flow|none].\n\ + When not given, prints the currently set model.") + .value_name("model") + .num_args(1) + .required(false) + .index(1)) + .arg( + Arg::new("list") + .long("list") + .short('l') + .help("List all available branching models.") + .required(false) + .num_args(0), + )) +} + +fn match_model_list(ses: &mut Session, matches: &ArgMatches) -> Result { + if let Some(matches) = matches.subcommand_matches("model") { + if matches.get_flag("list") { + println!( + "{}", + itertools::join(get_available_models(&ses.models_dir)?, "\n") + ); + return Ok(true); + } + } + Ok(false) +} + +fn match_model_subcommand(ses: &mut Session, matches: &ArgMatches) -> Result { + if let Some(matches) = matches.subcommand_matches("model") { + let repository = ses.repository.as_ref().unwrap(); + match matches.get_one::("model") { + None => { + let curr_model = get_model_name(repository, REPO_CONFIG_FILE)?; + match curr_model { + None => print!("No branching model set"), + Some(model) => print!("{}", model), + } + } + Some(model) => { + set_model(repository, model, REPO_CONFIG_FILE, &ses.models_dir)?; + eprint!("Branching model set to '{}'", model); + } + }; + return Ok(true); + } + Ok(false) +} + +fn match_model_opt(ses: &mut Session, matches: &ArgMatches) -> Result { + let model = get_model( + ses.repository.as_ref().unwrap(), + matches.get_one::("model").map(|s| &s[..]), + REPO_CONFIG_FILE, + &ses.models_dir, + )?; + Ok(model) +} + +// +// Run application +// + fn run( repository: Repository, settings: &Settings, From 0c090b0d2e0f2f26aa10515192406aff1b6bcc04 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 25 Nov 2025 12:12:04 +0100 Subject: [PATCH 06/21] split from_args - commit_limit --- src/main.rs | 57 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/src/main.rs b/src/main.rs index 7fc028a..d140bbe 100644 --- a/src/main.rs +++ b/src/main.rs @@ -49,6 +49,7 @@ fn from_args() -> Result<(), String> { ); let app = add_repo_args(app); let app = add_model_args(app); + let app = add_commit_limit_args(app); let app = app .arg( @@ -59,15 +60,6 @@ fn from_args() -> Result<(), String> { .required(false) .num_args(0), ) - .arg( - Arg::new("max-count") - .long("max-count") - .short('n') - .help("Maximum number of commits") - .required(false) - .num_args(1) - .value_name("n"), - ) .arg( Arg::new("local") .long("local") @@ -201,19 +193,7 @@ fn from_args() -> Result<(), String> { if match_model_subcommand(&mut ses, &matches)? { return Ok(()); // Exit after model subcommand } - - ses.commit_limit = match matches.get_one::("max-count") { - None => None, - Some(str) => match str.parse::() { - Ok(val) => Some(val), - Err(_) => { - return Err(format![ - "Option max-count must be a positive number, but got '{}'", - str - ]) - } - }, - }; + match_commit_limit_args(&mut ses, &matches)?; let include_remote = !matches.get_flag("local"); @@ -498,6 +478,39 @@ fn match_model_opt(ses: &mut Session, matches: &ArgMatches) -> Result Command { + app.arg( + Arg::new("max-count") + .long("max-count") + .short('n') + .help("Maximum number of commits") + .required(false) + .num_args(1) + .value_name("n"), + ) +} + +fn match_commit_limit_args(ses: &mut Session, matches: &ArgMatches) -> Result<(), String> { + ses.commit_limit = match matches.get_one::("max-count") { + None => None, + Some(str) => match str.parse::() { + Ok(val) => Some(val), + Err(_) => { + return Err(format![ + "Option max-count must be a positive number, but got '{}'", + str + ]) + } + }, + }; + + Ok(()) +} + // // Run application // From 7d98906985ccc1d0e4e1410fbe318c06cc11789a Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 25 Nov 2025 12:19:45 +0100 Subject: [PATCH 07/21] split from_args - color --- src/main.rs | 116 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 67 insertions(+), 49 deletions(-) diff --git a/src/main.rs b/src/main.rs index d140bbe..f282228 100644 --- a/src/main.rs +++ b/src/main.rs @@ -50,6 +50,7 @@ fn from_args() -> Result<(), String> { let app = add_repo_args(app); let app = add_model_args(app); let app = add_commit_limit_args(app); + let app = add_color_args(app); let app = app .arg( @@ -92,23 +93,6 @@ fn from_args() -> Result<(), String> { .required(false) .num_args(0), ) - .arg( - Arg::new("color") - .long("color") - .help("Specify when colors should be used. One of [auto|always|never].\n\ - Default: auto.") - .required(false) - .num_args(1), - ) - .arg( - Arg::new("no-color") - .long("no-color") - .help("Print without colors. Missing color support should be detected\n\ - automatically (e.g. when piping to a file).\n\ - Overrides option '--color'") - .required(false) - .num_args(0), - ) .arg( Arg::new("style") .long("style") @@ -220,38 +204,7 @@ fn from_args() -> Result<(), String> { Some(str) => CommitFormat::from_str(str)?, }; - let colored = if matches.get_flag("no-color") { - false - } else if let Some(mode) = matches.get_one::("color") { - match &mode[..] { - "auto" => { - atty::is(atty::Stream::Stdout) - && (!cfg!(windows) || { - yansi::enable(); - yansi::is_enabled() - }) - } - "always" => { - if cfg!(windows) { - yansi::enable(); - } - true - } - "never" => false, - other => { - return Err(format!( - "Unknown color mode '{}'. Supports [auto|always|never].", - other - )) - } - } - } else { - atty::is(atty::Stream::Stdout) - && (!cfg!(windows) || { - yansi::enable(); - yansi::is_enabled() - }) - }; + let colored = match_color_args(&mut ses, &matches)?; let wrapping = if let Some(wrap_values) = matches.get_many::("wrap") { let strings = wrap_values.map(|s| s.as_str()).collect::>(); @@ -511,6 +464,71 @@ fn match_commit_limit_args(ses: &mut Session, matches: &ArgMatches) -> Result<() Ok(()) } +// +// color flag +// + +fn add_color_args(app: Command) -> Command { + app.arg( + Arg::new("color") + .long("color") + .help( + "Specify when colors should be used. One of [auto|always|never].\n\ + Default: auto.", + ) + .required(false) + .num_args(1), + ) + .arg( + Arg::new("no-color") + .long("no-color") + .help( + "Print without colors. Missing color support should be detected\n\ + automatically (e.g. when piping to a file).\n\ + Overrides option '--color'", + ) + .required(false) + .num_args(0), + ) +} + +fn match_color_args(_ses: &mut Session, matches: &ArgMatches) -> Result { + let colored = if matches.get_flag("no-color") { + false + } else if let Some(mode) = matches.get_one::("color") { + match &mode[..] { + "auto" => { + atty::is(atty::Stream::Stdout) + && (!cfg!(windows) || { + yansi::enable(); + yansi::is_enabled() + }) + } + "always" => { + if cfg!(windows) { + yansi::enable(); + } + true + } + "never" => false, + other => { + return Err(format!( + "Unknown color mode '{}'. Supports [auto|always|never].", + other + )) + } + } + } else { + atty::is(atty::Stream::Stdout) + && (!cfg!(windows) || { + yansi::enable(); + yansi::is_enabled() + }) + }; + + Ok(colored) +} + // // Run application // From 8d5513a518882f3aabcf916066e85ef6fe207933 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 25 Nov 2025 12:28:08 +0100 Subject: [PATCH 08/21] split from_args - wrapping --- src/main.rs | 143 +++++++++++++++++++++++++++++----------------------- 1 file changed, 81 insertions(+), 62 deletions(-) diff --git a/src/main.rs b/src/main.rs index f282228..0d774c1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -51,6 +51,7 @@ fn from_args() -> Result<(), String> { let app = add_model_args(app); let app = add_commit_limit_args(app); let app = add_color_args(app); + let app = add_wrap_args(app); let app = app .arg( @@ -102,25 +103,6 @@ fn from_args() -> Result<(), String> { .required(false) .num_args(1), ) - .arg( - Arg::new("wrap") - .long("wrap") - .short('w') - .help("Line wrapping for formatted commit text. Default: 'auto 0 8'\n\ - Argument format: [|auto|none[ [ ]]]\n\ - For examples, consult 'git-graph --help'") - .long_help("Line wrapping for formatted commit text. Default: 'auto 0 8'\n\ - Argument format: [|auto|none[ [ ]]]\n\ - Examples:\n \ - git-graph --wrap auto\n \ - git-graph --wrap auto 0 8\n \ - git-graph --wrap none\n \ - git-graph --wrap 80\n \ - git-graph --wrap 80 0 8\n\ - 'auto' uses the terminal's width if on a terminal.") - .required(false) - .num_args(0..=3), - ) .arg( Arg::new("format") .long("format") @@ -206,49 +188,7 @@ fn from_args() -> Result<(), String> { let colored = match_color_args(&mut ses, &matches)?; - let wrapping = if let Some(wrap_values) = matches.get_many::("wrap") { - let strings = wrap_values.map(|s| s.as_str()).collect::>(); - if strings.is_empty() { - Some((None, Some(0), Some(8))) - } else { - match strings[0] { - "none" => None, - "auto" => { - let wrap = strings - .iter() - .skip(1) - .map(|str| str.parse::()) - .collect::, _>>() - .map_err(|_| { - format!( - "ERROR: Can't parse option --wrap '{}' to integers.", - strings.join(" ") - ) - })?; - Some((None, wrap.first().cloned(), wrap.get(1).cloned())) - } - _ => { - let wrap = strings - .iter() - .map(|str| str.parse::()) - .collect::, _>>() - .map_err(|_| { - format!( - "ERROR: Can't parse option --wrap '{}' to integers.", - strings.join(" ") - ) - })?; - Some(( - wrap.first().cloned(), - wrap.get(1).cloned(), - wrap.get(2).cloned(), - )) - } - } - } - } else { - Some((None, Some(0), Some(8))) - }; + let wrapping = match_wrap_args(&mut ses, &matches)?; let settings = Settings { reverse_commit_order, @@ -529,6 +469,85 @@ fn match_color_args(_ses: &mut Session, matches: &ArgMatches) -> Result Command { + app.arg( + Arg::new("wrap") + .long("wrap") + .short('w') + .help( + "Line wrapping for formatted commit text. Default: 'auto 0 8'\n\ + Argument format: [|auto|none[ [ ]]]\n\ + For examples, consult 'git-graph --help'", + ) + .long_help( + "Line wrapping for formatted commit text. Default: 'auto 0 8'\n\ + Argument format: [|auto|none[ [ ]]]\n\ + Examples:\n \ + git-graph --wrap auto\n \ + git-graph --wrap auto 0 8\n \ + git-graph --wrap none\n \ + git-graph --wrap 80\n \ + git-graph --wrap 80 0 8\n\ + 'auto' uses the terminal's width if on a terminal.", + ) + .required(false) + .num_args(0..=3), + ) +} + +type WrapType = Option<(Option, Option, Option)>; +fn match_wrap_args(_ses: &mut Session, matches: &ArgMatches) -> Result { + let wrapping = if let Some(wrap_values) = matches.get_many::("wrap") { + let strings = wrap_values.map(|s| s.as_str()).collect::>(); + if strings.is_empty() { + Some((None, Some(0), Some(8))) + } else { + match strings[0] { + "none" => None, + "auto" => { + let wrap = strings + .iter() + .skip(1) + .map(|str| str.parse::()) + .collect::, _>>() + .map_err(|_| { + format!( + "ERROR: Can't parse option --wrap '{}' to integers.", + strings.join(" ") + ) + })?; + Some((None, wrap.first().cloned(), wrap.get(1).cloned())) + } + _ => { + let wrap = strings + .iter() + .map(|str| str.parse::()) + .collect::, _>>() + .map_err(|_| { + format!( + "ERROR: Can't parse option --wrap '{}' to integers.", + strings.join(" ") + ) + })?; + Some(( + wrap.first().cloned(), + wrap.get(1).cloned(), + wrap.get(2).cloned(), + )) + } + } + } + } else { + Some((None, Some(0), Some(8))) + }; + + Ok(wrapping) +} + // // Run application // From 535be8d7adf915b478a795c6e8ff758cddb0864d Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 25 Nov 2025 12:43:04 +0100 Subject: [PATCH 09/21] split from_args - format --- src/main.rs | 123 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 70 insertions(+), 53 deletions(-) diff --git a/src/main.rs b/src/main.rs index 0d774c1..899b9fe 100644 --- a/src/main.rs +++ b/src/main.rs @@ -52,6 +52,7 @@ fn from_args() -> Result<(), String> { let app = add_commit_limit_args(app); let app = add_color_args(app); let app = add_wrap_args(app); + let app = add_format_args(app); let app = app .arg( @@ -89,8 +90,10 @@ fn from_args() -> Result<(), String> { Arg::new("sparse") .long("sparse") .short('S') - .help("Print a less compact graph: merge lines point to target lines\n\ - rather than merge commits.") + .help( + "Print a less compact graph: merge lines point to target lines\n\ + rather than merge commits.", + ) .required(false) .num_args(0), ) @@ -98,55 +101,13 @@ fn from_args() -> Result<(), String> { Arg::new("style") .long("style") .short('s') - .help("Output style. One of [normal/thin|round|bold|double|ascii].\n \ - (First character can be used as abbreviation, e.g. '-s r')") + .help( + "Output style. One of [normal/thin|round|bold|double|ascii].\n \ + (First character can be used as abbreviation, e.g. '-s r')", + ) .required(false) .num_args(1), - ) - .arg( - Arg::new("format") - .long("format") - .short('f') - .help("Commit format. One of [oneline|short|medium|full|\"\"].\n \ - (First character can be used as abbreviation, e.g. '-f m')\n\ - Default: oneline.\n\ - For placeholders supported in \"\", consult 'git-graph --help'") - .long_help("Commit format. One of [oneline|short|medium|full|\"\"].\n \ - (First character can be used as abbreviation, e.g. '-f m')\n\ - Formatting placeholders for \"\":\n \ - %n newline\n \ - %H commit hash\n \ - %h abbreviated commit hash\n \ - %P parent commit hashes\n \ - %p abbreviated parent commit hashes\n \ - %d refs (branches, tags)\n \ - %s commit summary\n \ - %b commit message body\n \ - %B raw body (subject and body)\n \ - %an author name\n \ - %ae author email\n \ - %ad author date\n \ - %as author date in short format 'YYYY-MM-DD'\n \ - %cn committer name\n \ - %ce committer email\n \ - %cd committer date\n \ - %cs committer date in short format 'YYYY-MM-DD'\n \ - \n \ - If you add a + (plus sign) after % of a placeholder,\n \ - a line-feed is inserted immediately before the expansion if\n \ - and only if the placeholder expands to a non-empty string.\n \ - If you add a - (minus sign) after % of a placeholder, all\n \ - consecutive line-feeds immediately preceding the expansion are\n \ - deleted if and only if the placeholder expands to an empty string.\n \ - If you add a ' ' (space) after % of a placeholder, a space is\n \ - inserted immediately before the expansion if and only if\n \ - the placeholder expands to a non-empty string.\n\ - \n \ - See also the respective git help: https://git-scm.com/docs/pretty-formats\n") - .required(false) - .num_args(1), - ) - ; + ); let matches = app.get_matches(); @@ -181,10 +142,7 @@ fn from_args() -> Result<(), String> { let model = match_model_opt(&mut ses, &matches)?; - let format = match matches.get_one::("format") { - None => CommitFormat::OneLine, - Some(str) => CommitFormat::from_str(str)?, - }; + let format = match_format_args(&mut ses, &matches)?; let colored = match_color_args(&mut ses, &matches)?; @@ -548,6 +506,65 @@ fn match_wrap_args(_ses: &mut Session, matches: &ArgMatches) -> Result Command { + app + .arg( + Arg::new("format") + .long("format") + .short('f') + .help("Commit format. One of [oneline|short|medium|full|\"\"].\n \ + (First character can be used as abbreviation, e.g. '-f m')\n\ + Default: oneline.\n\ + For placeholders supported in \"\", consult 'git-graph --help'") + .long_help("Commit format. One of [oneline|short|medium|full|\"\"].\n \ + (First character can be used as abbreviation, e.g. '-f m')\n\ + Formatting placeholders for \"\":\n \ + %n newline\n \ + %H commit hash\n \ + %h abbreviated commit hash\n \ + %P parent commit hashes\n \ + %p abbreviated parent commit hashes\n \ + %d refs (branches, tags)\n \ + %s commit summary\n \ + %b commit message body\n \ + %B raw body (subject and body)\n \ + %an author name\n \ + %ae author email\n \ + %ad author date\n \ + %as author date in short format 'YYYY-MM-DD'\n \ + %cn committer name\n \ + %ce committer email\n \ + %cd committer date\n \ + %cs committer date in short format 'YYYY-MM-DD'\n \ + \n \ + If you add a + (plus sign) after % of a placeholder,\n \ + a line-feed is inserted immediately before the expansion if\n \ + and only if the placeholder expands to a non-empty string.\n \ + If you add a - (minus sign) after % of a placeholder, all\n \ + consecutive line-feeds immediately preceding the expansion are\n \ + deleted if and only if the placeholder expands to an empty string.\n \ + If you add a ' ' (space) after % of a placeholder, a space is\n \ + inserted immediately before the expansion if and only if\n \ + the placeholder expands to a non-empty string.\n\ + \n \ + See also the respective git help: https://git-scm.com/docs/pretty-formats\n") + .required(false) + .num_args(1), + ) +} + +fn match_format_args(_ses: &mut Session, matches: &ArgMatches) -> Result { + let format = match matches.get_one::("format") { + None => CommitFormat::OneLine, + Some(str) => CommitFormat::from_str(str)?, + }; + Ok(format) +} + // // Run application // From e8c769a232ac65600ee4134c2d645fef527b7c1f Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 25 Nov 2025 17:02:20 +0100 Subject: [PATCH 10/21] split from_args - match_args --- src/main.rs | 124 +++++++++++++++++++++++++++------------------------- 1 file changed, 65 insertions(+), 59 deletions(-) diff --git a/src/main.rs b/src/main.rs index 899b9fe..62fd4b3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -35,6 +35,69 @@ fn from_args() -> Result<(), String> { let mut ses = Session::new(); store_default_models(&mut ses)?; + let matches = match_args(); + + if match_model_list(&mut ses, &matches)? { + return Ok(()); // Exit after showing model list + } + + match_repo_args(&mut ses, &matches)?; + + if match_model_subcommand(&mut ses, &matches)? { + return Ok(()); // Exit after model subcommand + } + match_commit_limit_args(&mut ses, &matches)?; + + let include_remote = !matches.get_flag("local"); + + let reverse_commit_order = matches.get_flag("reverse"); + + ses.svg = matches.get_flag("svg"); + let compact = !matches.get_flag("sparse"); + let debug = matches.get_flag("debug"); + let style = matches + .get_one::("style") + .map(|s| Characters::from_str(s)) + .unwrap_or_else(|| Ok(Characters::thin()))?; + + let style = if reverse_commit_order { + style.reverse() + } else { + style + }; + + let model = match_model_opt(&mut ses, &matches)?; + + let format = match_format_args(&mut ses, &matches)?; + + let colored = match_color_args(&mut ses, &matches)?; + + let wrapping = match_wrap_args(&mut ses, &matches)?; + + let settings = Settings { + reverse_commit_order, + debug, + colored, + compact, + include_remote, + format, + wrapping, + characters: style, + branch_order: BranchOrder::ShortestFirst(true), + branches: BranchSettings::from(model).map_err(|err| err.to_string())?, + merge_patterns: MergePatterns::default(), + }; + + run( + ses.repository.unwrap(), + &settings, + ses.svg, + ses.commit_limit, + ) +} + +fn match_args() -> ArgMatches { + // Declare command line argument interface for clap let app = Command::new("git-graph").version(crate_version!()).about( "Structured Git graphs for your branching model.\n \ https://github.com/mlange-42/git-graph\n\ @@ -109,65 +172,8 @@ fn from_args() -> Result<(), String> { .num_args(1), ); - let matches = app.get_matches(); - - if match_model_list(&mut ses, &matches)? { - return Ok(()); // Exit after showing model list - } - - match_repo_args(&mut ses, &matches)?; - - if match_model_subcommand(&mut ses, &matches)? { - return Ok(()); // Exit after model subcommand - } - match_commit_limit_args(&mut ses, &matches)?; - - let include_remote = !matches.get_flag("local"); - - let reverse_commit_order = matches.get_flag("reverse"); - - ses.svg = matches.get_flag("svg"); - let compact = !matches.get_flag("sparse"); - let debug = matches.get_flag("debug"); - let style = matches - .get_one::("style") - .map(|s| Characters::from_str(s)) - .unwrap_or_else(|| Ok(Characters::thin()))?; - - let style = if reverse_commit_order { - style.reverse() - } else { - style - }; - - let model = match_model_opt(&mut ses, &matches)?; - - let format = match_format_args(&mut ses, &matches)?; - - let colored = match_color_args(&mut ses, &matches)?; - - let wrapping = match_wrap_args(&mut ses, &matches)?; - - let settings = Settings { - reverse_commit_order, - debug, - colored, - compact, - include_remote, - format, - wrapping, - characters: style, - branch_order: BranchOrder::ShortestFirst(true), - branches: BranchSettings::from(model).map_err(|err| err.to_string())?, - merge_patterns: MergePatterns::default(), - }; - - run( - ses.repository.unwrap(), - &settings, - ses.svg, - ses.commit_limit, - ) + // Return match of declared arguments with what is present on command line + app.get_matches() } struct Session { From 42f2678770deee2a0bbe95550f2e0a6a73f43957 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 25 Nov 2025 17:10:19 +0100 Subject: [PATCH 11/21] split from_args - configure_session --- src/main.rs | 119 +++++++++++++++++++++++++++++----------------------- 1 file changed, 66 insertions(+), 53 deletions(-) diff --git a/src/main.rs b/src/main.rs index 62fd4b3..6682904 100644 --- a/src/main.rs +++ b/src/main.rs @@ -36,61 +36,13 @@ fn from_args() -> Result<(), String> { store_default_models(&mut ses)?; let matches = match_args(); - - if match_model_list(&mut ses, &matches)? { - return Ok(()); // Exit after showing model list - } - - match_repo_args(&mut ses, &matches)?; - - if match_model_subcommand(&mut ses, &matches)? { - return Ok(()); // Exit after model subcommand - } - match_commit_limit_args(&mut ses, &matches)?; - - let include_remote = !matches.get_flag("local"); - - let reverse_commit_order = matches.get_flag("reverse"); - - ses.svg = matches.get_flag("svg"); - let compact = !matches.get_flag("sparse"); - let debug = matches.get_flag("debug"); - let style = matches - .get_one::("style") - .map(|s| Characters::from_str(s)) - .unwrap_or_else(|| Ok(Characters::thin()))?; - - let style = if reverse_commit_order { - style.reverse() - } else { - style - }; - - let model = match_model_opt(&mut ses, &matches)?; - - let format = match_format_args(&mut ses, &matches)?; - - let colored = match_color_args(&mut ses, &matches)?; - - let wrapping = match_wrap_args(&mut ses, &matches)?; - - let settings = Settings { - reverse_commit_order, - debug, - colored, - compact, - include_remote, - format, - wrapping, - characters: style, - branch_order: BranchOrder::ShortestFirst(true), - branches: BranchSettings::from(model).map_err(|err| err.to_string())?, - merge_patterns: MergePatterns::default(), + if !configure_session(&mut ses, &matches)? { + return Ok(()); // If the configuration decided session should not start }; run( ses.repository.unwrap(), - &settings, + ses.settings.as_ref().unwrap(), ses.svg, ses.commit_limit, ) @@ -176,12 +128,73 @@ fn match_args() -> ArgMatches { app.get_matches() } +/// Return true if session should continue, false if it should exit now +fn configure_session(ses: &mut Session, matches: &ArgMatches) -> Result { + // return values + let exit_now = false; + let run_application = true; + + if match_model_list(ses, matches)? { + return Ok(exit_now); // Exit after showing model list + } + + match_repo_args(ses, matches)?; + + if match_model_subcommand(ses, matches)? { + return Ok(exit_now); // Exit after model subcommand + } + match_commit_limit_args(ses, matches)?; + + let include_remote = !matches.get_flag("local"); + + let reverse_commit_order = matches.get_flag("reverse"); + + ses.svg = matches.get_flag("svg"); + let compact = !matches.get_flag("sparse"); + let debug = matches.get_flag("debug"); + let style = matches + .get_one::("style") + .map(|s| Characters::from_str(s)) + .unwrap_or_else(|| Ok(Characters::thin()))?; + + let style = if reverse_commit_order { + style.reverse() + } else { + style + }; + + let model = match_model_opt(ses, matches)?; + + let format = match_format_args(ses, matches)?; + + let colored = match_color_args(ses, matches)?; + + let wrapping = match_wrap_args(ses, matches)?; + + let settings = Settings { + reverse_commit_order, + debug, + colored, + compact, + include_remote, + format, + wrapping, + characters: style, + branch_order: BranchOrder::ShortestFirst(true), + branches: BranchSettings::from(model).map_err(|err| err.to_string())?, + merge_patterns: MergePatterns::default(), + }; + ses.settings = Some(settings); + + Ok(run_application) +} + struct Session { // models related fields models_dir: PathBuf, // Settings related fields - // TODO + pub settings: Option, // Other fields pub repository: Option, @@ -196,7 +209,7 @@ impl Session { models_dir: PathBuf::new(), // Settings related fields - // TODO + settings: None, // Other fields repository: None, From b8bb2cd30d0d7740fd6fc5350dab677eaf0690c9 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Thu, 20 Nov 2025 04:55:11 +0100 Subject: [PATCH 12/21] Add unit tests for fn hline The hline function could benefit from a heavy refactoring, but that has the risk that corner cases may fail. Therefore, add some unit test that pass on the old code. --- src/print/unicode.rs | 380 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 380 insertions(+) diff --git a/src/print/unicode.rs b/src/print/unicode.rs index d4f8821..006f64f 100644 --- a/src/print/unicode.rs +++ b/src/print/unicode.rs @@ -868,3 +868,383 @@ impl Grid { } } } + +#[cfg(test)] +mod tests { + use super::*; + // A dummy `Characters` struct is needed for `GridCell::char` but is not + // directly used in `hline` tests, so we can omit it by not calling `char()`. + + // --- Test Cases --- + + /* Testing hline + + Note that hline is given a graph column as input, + which indexes a grid column at 2*graph_col + // Graph column: 0 1 2 3 4 5 + // Grid columns: 0 1 2 3 4 5 6 7 8 9 + // Grid row 0: _ _ _ _ _ _ _ _ _ _ + // Grid row 1: _ _ _ _ _ _ _ _ _ _ + // Grid row 2: _ _ _ _ _ _ _ _ _ _ + + A horizontal line from 1 to 3, would occupy columns 2, 3, 4, 5, 6 inclusive + + */ + + const DEF_CH: u8 = SPACE; + const DEF_COL: u8 = 0; + const DEF_PERS: u8 = 10; // low persistence, will always be overwritten + const DEFAULT_CELL: GridCell = GridCell { + character: DEF_CH, + color: DEF_COL, + pers: DEF_PERS, + }; + const ROW_INDEX: usize = 1; + const LINE_COLOR: u8 = 14; + const LINE_PERS: u8 = 5; + + #[test] + fn hline_skip() { + let (width, height) = (10, 3); + let mut grid = Grid::new(width, height, DEFAULT_CELL); + // Graph column: 0 1 2 3 4 5 + // Grid columns: 0 1 2 3 4 5 6 7 8 9 + // Grid row 0: _ _ _ _ _ _ _ _ _ _ + // Grid row 1: _ _ _ _ _ _ _ _ _ _ + // Grid row 2: _ _ _ _ _ _ _ _ _ _ + + // Case 1: from == to (should do nothing) + let initial_char = grid.get_tuple(4 * 2, ROW_INDEX).0; + super::hline(&mut grid, ROW_INDEX, (4, 4), true, LINE_COLOR, LINE_PERS); + // Graph column: 0 1 2 3 4 5 + // Grid columns: 0 1 2 3 4 5 6 7 8 9 + // Grid row 0: _ _ _ _ _ _ _ _ _ _ + // Grid row 1: _ _ _ _ _ _ _ _X_ _ + // Grid row 2: _ _ _ _ _ _ _ _ _ _ + + assert_eq!( + grid.get_tuple(4 * 2, ROW_INDEX).0, + initial_char, + "Same index call should not modify grid" + ); + } + + /// Case 2: Forward draw (from < to), no merge + /// Case 2a: out of bounds + #[test] + fn hline_forward_no_merge_out_of_bounds() { + let (width, height) = (10, 3); + let mut grid = Grid::new(width, height, DEFAULT_CELL); + super::hline(&mut grid, ROW_INDEX, (2, 5), false, LINE_COLOR, LINE_PERS); + // Graph column: 0 1 2 3 4 5 + // Grid columns: 0 1 2 3 4 5 6 7 8 9 + // Grid row 0: _ _ _ _ _ _ _ _ _ _ + // Grid row 1: _ _ _ _F- - - - - - *T (F=from, T=to) + // Grid row 2: _ _ _ _ _ _ _ _ _ _ + + // from: 2, to: 5 + // Start: from*2 = 4, End: to*2 = 10. + // Range: start+1..end = 5..=9. Grid columns updated: 5, 6, 7, 8, 9. (HOR) + // Ends updated: start=4, end=10. (VER_R) + + // Columns outside the line range (before start) should be default + assert_eq!( + grid.get_tuple(0, ROW_INDEX).0, + SPACE, + "SPACE at start of row" + ); + assert_eq!(grid.get_tuple(3, ROW_INDEX).0, SPACE, "SPACE before hline"); + + // Start (column 4): Should be R_D - assuming a vline below + assert_eq!(grid.get_tuple(4, ROW_INDEX).0, R_D, "R_D at start of hline"); + assert_eq!( + grid.get_tuple(4, ROW_INDEX).1, + LINE_COLOR, + "line_color at start of hline" + ); + assert_eq!( + grid.get_tuple(4, ROW_INDEX).2, + LINE_PERS, + "line_pers at start of hline" + ); + + // End (column 10) is out of bounds for width 10 (index 0-9). The `Grid` + // implementation should handle this (or it's an expected panic/logic error). + // *Assuming* the provided `Grid` is simplified for this example and we should + // test only within bounds. Let's adjust the indices to be safe and meaningful. + } + + /// Case 2: Forward draw (from < to), no merge + /// Case 2b: Inside bounds + #[test] + fn hline_forward_no_merge_at_bounds() { + let safe_width = 7; // Max column index 6, max graph column 2 = grid col 5 + let height = 3; + let mut grid = Grid::new(safe_width, height, DEFAULT_CELL); + // Graph column: 0 1 2 3 + // Grid columns: 0 1 2 3 4 5 6 + // Grid row 0: _ _ _ _ _ _ _ + // Grid row 1: _ _ _ _ _ _ _ + // Grid row 2: _ _ _ _ _ _ _ + + let from_idx = 1; + let to_idx = 3; + // Index: 0 1 2 3 + // Cell: - F - T + // From: 1, To: 3. + // Start: 2, End: 6. + // Range: 3..5 (Columns 3, 4, 5) -> HOR + // Ends: 2, 6 -> R_D, L_U + + assert_eq!( + grid.get_tuple(2, ROW_INDEX).0, + SPACE, + "SPACE at start of line, before written" + ); + super::hline( + &mut grid, + ROW_INDEX, + (from_idx, to_idx), + false, + LINE_COLOR, + LINE_PERS, + ); + // Graph column: 0 1 2 3 + // Grid columns: 0 1 2 3 4 5 6 + // Grid row 0: _ _ _ _ _ _ _ + // Grid row 1: _ _(╭ ─ ─ ─ ┘) + // Grid row 2: _ _ _ _ _ _ _ + + // Check column before start + let grid_cell = grid.get_tuple(1, ROW_INDEX); + assert_eq!(grid_cell.0, SPACE, "SPACE before hline"); + assert_eq!(grid_cell.1, DEF_COL, "default colour before hline"); + assert_eq!(grid_cell.2, DEF_PERS, "default persistence before hline"); + + // Start (column 2): R_D + let grid_cell = grid.get_tuple(2, ROW_INDEX); + assert_eq!(grid_cell.0, R_D, "R_D at start of hline"); + assert_eq!(grid_cell.1, LINE_COLOR, "line_color at start of hline"); + assert_eq!(grid_cell.2, LINE_PERS, "line_pers at start of hline"); + + // Range (columns 3, 4, 5): HOR + let grid_cell = grid.get_tuple(3, ROW_INDEX); + assert_eq!(grid_cell.0, HOR, "HOR in range of hline"); + assert_eq!(grid_cell.1, LINE_COLOR, "line_color in range of hline"); + assert_eq!(grid_cell.2, LINE_PERS, "line_pers in range of hline"); + + let grid_cell = grid.get_tuple(4, ROW_INDEX); + assert_eq!(grid_cell.0, HOR, "HOR in range of hline"); + assert_eq!(grid_cell.1, LINE_COLOR, "line_color in range of hline"); + assert_eq!(grid_cell.2, LINE_PERS, "line_pers in range of hline"); + + let grid_cell = grid.get_tuple(5, ROW_INDEX); + assert_eq!(grid_cell.0, HOR, "HOR in range of hline"); + assert_eq!(grid_cell.1, LINE_COLOR, "line_color in range of hline"); + assert_eq!(grid_cell.2, LINE_PERS, "line_pers in range of hline"); + + // End (column 6): L_U + let grid_cell = grid.get_tuple(6, ROW_INDEX); + assert_eq!(grid_cell.0, L_U, "L_U at end of hline"); + assert_eq!(grid_cell.1, LINE_COLOR, "line_color at end of hline"); + assert_eq!(grid_cell.2, LINE_PERS, "line_pers at end of hline"); + + // Check column after end + // This is undefined, as max grid col is 6 + // TODO make expected panic + let grid_cell = grid.get_tuple(7, ROW_INDEX); + assert_eq!(grid_cell.0, SPACE, "SPACE before hline"); + assert_eq!(grid_cell.1, DEF_COL, "default colour before hline"); + assert_eq!(grid_cell.2, DEF_PERS, "default persistence before hline"); + } + + /// Case 3: Backward draw (from > to), with merge + #[test] + fn hline_backward() { + let (width, height) = (10, 3); + let mut grid = Grid::new(width, height, DEFAULT_CELL); + // Set an existing symbol at an end for better coverage: + grid.set(4, ROW_INDEX, VER, 10, 10); // Start/From pos + grid.set(8, ROW_INDEX, HOR, 10, 10); // End/To pos + + // Graph column: 0 1 2 3 4 + // Grid columns: 0 1 2 3 4 5 6 7 8 9 + // Grid row 0: _ _ _ _ _ _ _ _ _ _ + // Grid row 1: _ _ _ _ │ _ _ _ ─ _ + // Grid row 2: _ _ _ _ _ _ _ _ _ _ + + let from_idx = 4; + let to_idx = 2; + let merge = true; + // Index: 0 1 2 3 4 + // Cell: - - T - F + // Forward is false. + // start (orig from*2) = 8, end (orig to*2) = 4. Swapped: start=4, end=8. + // Range: start+1..end = 5..8. Columns updated: 5, 6, 7 -> HOR + // Merge: column = start = 4. Symbol = ARR_L. + // Ends: start=4 (backward), end=8 (forward). (Both should be L_D/R_U if they weren't SPACE) + + super::hline( + &mut grid, + ROW_INDEX, + (from_idx, to_idx), + merge, + LINE_COLOR, + LINE_PERS, + ); + // Graph column: 0 1 2 3 4 + // Grid columns: 0 1 2 3 4 5 6 7 8 9 + // Grid row 0: _ _ _ _ _ _ _ _ _ _ + // Grid row 1: _ _ _ _ ├ < ─ ─ ┬ _ + // Grid row 2: _ _ _ _ _ _ _ _ _ _ + + // Check columns before start + assert_eq!(grid.get_tuple(3, ROW_INDEX).0, SPACE, "SPACE before hline"); + assert_eq!( + grid.get_tuple(3, ROW_INDEX).1, + DEF_COL, + "default colour before hline" + ); + assert_eq!( + grid.get_tuple(3, ROW_INDEX).2, + DEF_PERS, + "default persistence before hline" + ); + + // Merge: column 4 (start). Should be VER_R. + assert_eq!(grid.get_tuple(4, ROW_INDEX).0, VER_R, "VER_R at hline 'to'"); + assert_eq!( + grid.get_tuple(4, ROW_INDEX).1, + 10, + "unchanged color at hline 'to'" + ); + assert_eq!( + grid.get_tuple(4, ROW_INDEX).2, + 10, + "unchanged pers at hline 'to'" + ); + + // Merge (column 5): ARR_l + assert_eq!( + grid.get_tuple(5, ROW_INDEX).0, + ARR_L, + "ARR_L before hline 'to'" + ); + assert_eq!( + grid.get_tuple(5, ROW_INDEX).1, + LINE_COLOR, + "line_color in hline" + ); + assert_eq!( + grid.get_tuple(5, ROW_INDEX).2, + LINE_PERS, + "line_pers in hline" + ); + + // Range (columns 5, 6): HOR + assert_eq!(grid.get_tuple(6, ROW_INDEX).0, HOR, "HOR in hline"); + assert_eq!( + grid.get_tuple(6, ROW_INDEX).1, + LINE_COLOR, + "line_color in hline" + ); + assert_eq!( + grid.get_tuple(6, ROW_INDEX).2, + LINE_PERS, + "line_pers in hline" + ); + + assert_eq!(grid.get_tuple(7, ROW_INDEX).0, HOR, "HOR in hline"); + assert_eq!( + grid.get_tuple(7, ROW_INDEX).1, + LINE_COLOR, + "line_color in hline" + ); + assert_eq!( + grid.get_tuple(7, ROW_INDEX).2, + LINE_PERS, + "line_pers in hline" + ); + + // Cell 8 (end/from): HOR_D + assert_eq!( + grid.get_tuple(8, ROW_INDEX).0, + HOR_D, + "HOR_D at hline 'from'" + ); + assert_eq!( + grid.get_tuple(8, ROW_INDEX).1, + LINE_COLOR, + "line_color at hline 'from'" + ); + assert_eq!( + grid.get_tuple(8, ROW_INDEX).2, + LINE_PERS, + "line_pers at hline 'from'" + ); + } + + /// Case 4: Forward draw, with merge, onto a crossing symbol + #[test] + fn hline_forward_merge() { + let merge = true; + let (width, height) = (7, 3); + let mut grid = Grid::new(width, height, DEFAULT_CELL); + grid.set(5, ROW_INDEX, R_U, 10, 10); // Set a symbol that changes range + grid.set(6, ROW_INDEX, VER, 11, 10); // Set symbol for merge target + + // Graph column: 0 1 2 3 + // Grid columns: 0 1 2 3 4 5 6 + // Grid row 0: _ _ _ _ _ _ _ + // Grid row 1: _ _ _ _ _ └ │ + // Grid row 2: _ _ _ _ _ _ _ + + let from_idx = 1; + let to_idx = 3; + // Start: 2, End: 6. + // Index: 0 1 2 3 4 5 6 + // Cell: - - F - - R_U T + // Range: 3..6. Columns: 3, 4, 5. + // Column 5: R_U -> HOR_D (in update_range) + // Merge: column = end - 1 = 5. Symbol = ARR_R. Overwrites HOR_D. + // Ends: 2 (forward), 6 (forward). + + super::hline( + &mut grid, + ROW_INDEX, + (from_idx, to_idx), + merge, + LINE_COLOR, + LINE_PERS, + ); + // Graph column: 0 1 2 3 + // Grid columns: 0 1 2 3 4 5 6 + // Grid row 0: _ _ _ _ _ _ _ + // Grid row 1: _ _(╭ ─ ─ > ┤) + // Grid row 2: _ _ _ _ _ _ _ + + // Start (column 2): R_D + assert_eq!(grid.get_tuple(2, ROW_INDEX).0, R_D); + assert_eq!(grid.get_tuple(2, ROW_INDEX).1, LINE_COLOR); + assert_eq!(grid.get_tuple(2, ROW_INDEX).2, LINE_PERS); + + // Range (column 3, 4): HOR + assert_eq!(grid.get_tuple(3, ROW_INDEX).0, HOR); + assert_eq!(grid.get_tuple(3, ROW_INDEX).1, LINE_COLOR); + assert_eq!(grid.get_tuple(3, ROW_INDEX).2, LINE_PERS); + + assert_eq!(grid.get_tuple(4, ROW_INDEX).0, HOR); + assert_eq!(grid.get_tuple(4, ROW_INDEX).1, LINE_COLOR); + assert_eq!(grid.get_tuple(4, ROW_INDEX).2, LINE_PERS); + + // Merge column (end - 1 = 5): ARR_R (Merge overwrites update_range) + assert_eq!(grid.get_tuple(5, ROW_INDEX).0, ARR_R); + assert_eq!(grid.get_tuple(5, ROW_INDEX).1, LINE_COLOR); + assert_eq!(grid.get_tuple(5, ROW_INDEX).2, LINE_PERS); + + // End (column 6): VER_L + assert_eq!(grid.get_tuple(6, ROW_INDEX).0, VER_L); + assert_eq!(grid.get_tuple(6, ROW_INDEX).1, 11); + assert_eq!(grid.get_tuple(6, ROW_INDEX).2, 10); + } +} From e23ae0077a45b7d191e737015cfe9f0dabef49ab Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Wed, 19 Nov 2025 00:54:44 +0100 Subject: [PATCH 13/21] Split fn hline This function had more than 100 lines. --- src/print/unicode.rs | 235 +++++++++++++++++++++++++------------------ 1 file changed, 135 insertions(+), 100 deletions(-) diff --git a/src/print/unicode.rs b/src/print/unicode.rs index 006f64f..7c646dc 100644 --- a/src/print/unicode.rs +++ b/src/print/unicode.rs @@ -311,7 +311,8 @@ fn vline(grid: &mut Grid, (from, to): (usize, usize), column: usize, color: u8, } } -/// Draws a horizontal line +/// Draw a horizontal line. +/// If from > to, this will cause a backward draw. fn hline( grid: &mut Grid, index: usize, @@ -323,123 +324,157 @@ fn hline( if from == to { return; } + let from_2 = from * 2; let to_2 = to * 2; + if from < to { - for column in (from_2 + 1)..to_2 { - if merge && column == to_2 - 1 { - grid.set(column, index, ARR_R, color, pers); + update_range_forward(grid, index, from_2, to_2, merge, color, pers); + update_left_cell_forward(grid, index, from_2, color, pers); + update_right_cell_forward(grid, index, to_2, color, pers); + } else { + update_range_backward(grid, index, from_2, to_2, merge, color, pers); + update_left_cell_backward(grid, index, to_2, color, pers); + update_right_cell_backward(grid, index, from_2, color, pers); + } +} + +fn update_range_forward( + grid: &mut Grid, + index: usize, + from_2: usize, + to_2: usize, + merge: bool, + color: u8, + pers: u8, +) { + for column in (from_2 + 1)..to_2 { + if merge && column == to_2 - 1 { + grid.set(column, index, ARR_R, color, pers); + } else { + let (curr, _, old_pers) = grid.get_tuple(column, index); + let (new_col, new_pers) = if pers < old_pers { + (Some(color), Some(pers)) } else { - let (curr, _, old_pers) = grid.get_tuple(column, index); - let (new_col, new_pers) = if pers < old_pers { - (Some(color), Some(pers)) - } else { - (None, None) - }; - match curr { - DOT | CIRCLE => {} - VER => grid.set_opt(column, index, Some(CROSS), None, None), - HOR | CROSS | HOR_U | HOR_D => { - grid.set_opt(column, index, None, new_col, new_pers) - } - L_U | R_U => grid.set_opt(column, index, Some(HOR_U), new_col, new_pers), - L_D | R_D => grid.set_opt(column, index, Some(HOR_D), new_col, new_pers), - _ => { - grid.set_opt(column, index, Some(HOR), new_col, new_pers); - } + (None, None) + }; + match curr { + DOT | CIRCLE => {} + VER => grid.set_opt(column, index, Some(CROSS), None, None), + HOR | CROSS | HOR_U | HOR_D => grid.set_opt(column, index, None, new_col, new_pers), + L_U | R_U => grid.set_opt(column, index, Some(HOR_U), new_col, new_pers), + L_D | R_D => grid.set_opt(column, index, Some(HOR_D), new_col, new_pers), + _ => { + grid.set_opt(column, index, Some(HOR), new_col, new_pers); } } } + } +} - let (left, _, old_pers) = grid.get_tuple(from_2, index); - let (new_col, new_pers) = if pers < old_pers { - (Some(color), Some(pers)) - } else { - (None, None) - }; - match left { - DOT | CIRCLE => {} - VER => grid.set_opt(from_2, index, Some(VER_R), new_col, new_pers), - VER_L => grid.set_opt(from_2, index, Some(CROSS), None, None), - VER_R => {} - HOR | L_U => grid.set_opt(from_2, index, Some(HOR_U), new_col, new_pers), - _ => { - grid.set_opt(from_2, index, Some(R_D), new_col, new_pers); - } +fn update_left_cell_forward(grid: &mut Grid, index: usize, from_2: usize, color: u8, pers: u8) { + let (left, _, old_pers) = grid.get_tuple(from_2, index); + let (new_col, new_pers) = if pers < old_pers { + (Some(color), Some(pers)) + } else { + (None, None) + }; + match left { + DOT | CIRCLE => {} + VER => grid.set_opt(from_2, index, Some(VER_R), new_col, new_pers), + VER_L => grid.set_opt(from_2, index, Some(CROSS), None, None), + VER_R => {} + HOR | L_U => grid.set_opt(from_2, index, Some(HOR_U), new_col, new_pers), + _ => { + grid.set_opt(from_2, index, Some(R_D), new_col, new_pers); } + } +} - let (right, _, old_pers) = grid.get_tuple(to_2, index); - let (new_col, new_pers) = if pers < old_pers { - (Some(color), Some(pers)) - } else { - (None, None) - }; - match right { - DOT | CIRCLE => {} - VER => grid.set_opt(to_2, index, Some(VER_L), None, None), - VER_L | HOR_U => grid.set_opt(to_2, index, None, new_col, new_pers), - HOR | R_U => grid.set_opt(to_2, index, Some(HOR_U), new_col, new_pers), - _ => { - grid.set_opt(to_2, index, Some(L_U), new_col, new_pers); - } - } +fn update_right_cell_forward(grid: &mut Grid, index: usize, to_2: usize, color: u8, pers: u8) { + let (right, _, old_pers) = grid.get_tuple(to_2, index); + let (new_col, new_pers) = if pers < old_pers { + (Some(color), Some(pers)) } else { - for column in (to_2 + 1)..from_2 { - if merge && column == to_2 + 1 { - grid.set(column, index, ARR_L, color, pers); + (None, None) + }; + match right { + DOT | CIRCLE => {} + VER => grid.set_opt(to_2, index, Some(VER_L), None, None), + VER_L | HOR_U => grid.set_opt(to_2, index, None, new_col, new_pers), + HOR | R_U => grid.set_opt(to_2, index, Some(HOR_U), new_col, new_pers), + _ => { + grid.set_opt(to_2, index, Some(L_U), new_col, new_pers); + } + } +} + +fn update_range_backward( + grid: &mut Grid, + index: usize, + from_2: usize, + to_2: usize, + merge: bool, + color: u8, + pers: u8, +) { + for column in (to_2 + 1)..from_2 { + if merge && column == to_2 + 1 { + grid.set(column, index, ARR_L, color, pers); + } else { + let (curr, _, old_pers) = grid.get_tuple(column, index); + let (new_col, new_pers) = if pers < old_pers { + (Some(color), Some(pers)) } else { - let (curr, _, old_pers) = grid.get_tuple(column, index); - let (new_col, new_pers) = if pers < old_pers { - (Some(color), Some(pers)) - } else { - (None, None) - }; - match curr { - DOT | CIRCLE => {} - VER => grid.set_opt(column, index, Some(CROSS), None, None), - HOR | CROSS | HOR_U | HOR_D => { - grid.set_opt(column, index, None, new_col, new_pers) - } - L_U | R_U => grid.set_opt(column, index, Some(HOR_U), new_col, new_pers), - L_D | R_D => grid.set_opt(column, index, Some(HOR_D), new_col, new_pers), - _ => { - grid.set_opt(column, index, Some(HOR), new_col, new_pers); - } + (None, None) + }; + match curr { + DOT | CIRCLE => {} + VER => grid.set_opt(column, index, Some(CROSS), None, None), + HOR | CROSS | HOR_U | HOR_D => grid.set_opt(column, index, None, new_col, new_pers), + L_U | R_U => grid.set_opt(column, index, Some(HOR_U), new_col, new_pers), + L_D | R_D => grid.set_opt(column, index, Some(HOR_D), new_col, new_pers), + _ => { + grid.set_opt(column, index, Some(HOR), new_col, new_pers); } } } + } +} - let (left, _, old_pers) = grid.get_tuple(to_2, index); - let (new_col, new_pers) = if pers < old_pers { - (Some(color), Some(pers)) - } else { - (None, None) - }; - match left { - DOT | CIRCLE => {} - VER => grid.set_opt(to_2, index, Some(VER_R), None, None), - VER_R => grid.set_opt(to_2, index, None, new_col, new_pers), - HOR | L_U => grid.set_opt(to_2, index, Some(HOR_U), new_col, new_pers), - _ => { - grid.set_opt(to_2, index, Some(R_U), new_col, new_pers); - } +fn update_left_cell_backward(grid: &mut Grid, index: usize, to_2: usize, color: u8, pers: u8) { + let (left, _, old_pers) = grid.get_tuple(to_2, index); + let (new_col, new_pers) = if pers < old_pers { + (Some(color), Some(pers)) + } else { + (None, None) + }; + match left { + DOT | CIRCLE => {} + VER => grid.set_opt(to_2, index, Some(VER_R), None, None), + VER_R => grid.set_opt(to_2, index, None, new_col, new_pers), + HOR | L_U => grid.set_opt(to_2, index, Some(HOR_U), new_col, new_pers), + _ => { + grid.set_opt(to_2, index, Some(R_U), new_col, new_pers); } + } +} - let (right, _, old_pers) = grid.get_tuple(from_2, index); - let (new_col, new_pers) = if pers < old_pers { - (Some(color), Some(pers)) - } else { - (None, None) - }; - match right { - DOT | CIRCLE => {} - VER => grid.set_opt(from_2, index, Some(VER_L), new_col, new_pers), - VER_R => grid.set_opt(from_2, index, Some(CROSS), None, None), - VER_L => grid.set_opt(from_2, index, None, new_col, new_pers), - HOR | R_D => grid.set_opt(from_2, index, Some(HOR_D), new_col, new_pers), - _ => { - grid.set_opt(from_2, index, Some(L_D), new_col, new_pers); - } +fn update_right_cell_backward(grid: &mut Grid, index: usize, from_2: usize, color: u8, pers: u8) { + let (right, _, old_pers) = grid.get_tuple(from_2, index); + let (new_col, new_pers) = if pers < old_pers { + (Some(color), Some(pers)) + } else { + (None, None) + }; + match right { + DOT | CIRCLE => {} + VER => grid.set_opt(from_2, index, Some(VER_L), new_col, new_pers), + VER_R => grid.set_opt(from_2, index, Some(CROSS), None, None), + VER_L => grid.set_opt(from_2, index, None, new_col, new_pers), + HOR | R_D => grid.set_opt(from_2, index, Some(HOR_D), new_col, new_pers), + _ => { + grid.set_opt(from_2, index, Some(L_D), new_col, new_pers); } } } From f5bf074a79694611217693051d55b324ac509ab2 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Thu, 20 Nov 2025 19:17:47 +0100 Subject: [PATCH 14/21] Extract draw_parent_lines from print_unicode --- src/print/unicode.rs | 121 ++++++++++++++++++++++++++----------------- 1 file changed, 73 insertions(+), 48 deletions(-) diff --git a/src/print/unicode.rs b/src/print/unicode.rs index 7c646dc..2e36a6d 100644 --- a/src/print/unicode.rs +++ b/src/print/unicode.rs @@ -1,6 +1,6 @@ //! Create graphs in Unicode format with ANSI X3.64 / ISO 6429 colour codes -use crate::graph::{CommitInfo, GitGraph, HeadInfo}; +use crate::graph::{BranchInfo, CommitInfo, GitGraph, HeadInfo}; use crate::print::format::CommitFormat; use crate::settings::{Characters, Settings}; use itertools::Itertools; @@ -157,53 +157,15 @@ pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result idx_map + 1 { - vline(&mut grid, (idx_map, par_idx_map), column, color, pers); - } - } else { - let split_index = super::get_deviate_index(graph, idx, *par_idx); - let split_idx_map = index_map[split_index]; - let insert_idx = find_insert_idx(&inserts[&split_index], idx, *par_idx).unwrap(); - let idx_split = split_idx_map + insert_idx; - - let is_secondary_merge = info.is_merge && p > 0; - - let row123 = (idx_map, idx_split, par_idx_map); - let col12 = (column, par_column); - zig_zag_line(&mut grid, row123, col12, is_secondary_merge, color, pers); - } - } + draw_parent_lines( + graph, + branch, + &mut grid, + info, + &inserts, + &index_map, + idx, + ); } if settings.reverse_commit_order { @@ -216,6 +178,69 @@ pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result>>, + index_map: &[usize], + idx: usize, +) { + let column = branch.visual.column.unwrap(); + let idx_map = index_map[idx]; + + let branch_color = branch.visual.term_color; + + for p in 0..2 { + let parent = info.parents[p]; + let Some(par_oid) = parent else { + continue; + }; + let Some(par_idx) = graph.indices.get(&par_oid) else { + // Parent is outside scope of graph.indices + // so draw a vertical line to the bottom + let idx_bottom = grid.height; + vline( + grid, + (idx_map, idx_bottom), + column, + branch_color, + branch.persistence, + ); + continue; + }; + + let par_idx_map = index_map[*par_idx]; + let par_info = &graph.commits[*par_idx]; + let par_branch = &graph.all_branches[par_info.branch_trace.unwrap()]; + let par_column = par_branch.visual.column.unwrap(); + + let (color, pers) = if info.is_merge { + (par_branch.visual.term_color, par_branch.persistence) + } else { + (branch_color, branch.persistence) + }; + + if branch.visual.column == par_branch.visual.column { + if par_idx_map > idx_map + 1 { + vline(grid, (idx_map, par_idx_map), column, color, pers); + } + } else { + let split_index = super::get_deviate_index(graph, idx, *par_idx); + let split_idx_map = index_map[split_index]; + let insert_idx = find_insert_idx(&inserts[&split_index], idx, *par_idx).unwrap(); + let idx_split = split_idx_map + insert_idx; + + let is_secondary_merge = info.is_merge && p > 0; + + let row123 = (idx_map, idx_split, par_idx_map); + let col12 = (column, par_column); + zig_zag_line(grid, row123, col12, is_secondary_merge, color, pers); + } + } +} + /// Create `textwrap::Options` from width and indent. fn create_wrapping_options<'a>( width: Option, From abe62060757916101e16dc2e9903a1f13ab6cba1 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Wed, 26 Nov 2025 14:34:31 +0100 Subject: [PATCH 15/21] Sketch refactoring target The following commits performs a step-by-step refactoring. To make it clearer what the goal is, and to minimize code movement during refactoring, this commit presents the target function and a residual that gradually shrinks until everything is refactored. --- src/print/unicode.rs | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/print/unicode.rs b/src/print/unicode.rs index 2e36a6d..d321809 100644 --- a/src/print/unicode.rs +++ b/src/print/unicode.rs @@ -53,9 +53,34 @@ graph-lines, text-lines, start-row 3. start_row: `Vec`: Starting row for commit in the `graph.commits` vector. */ pub type UnicodeGraphInfo = (Vec, Vec, Vec); - /// Creates a text-based visual representation of a graph. pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result { + + // 1. Calculate dimensions and inserts + // TODO + + // 2. Prepare wrapping for commit text (using references to the new indent strings) + // TODO + + // 3. Compute commit text and index map + // TODO + + // 4. Calculate total rows and initialize/draw the grid + // TODO + + // 5. Handle reverse order + // TODO + + // 6. Final printing and result + // TODO + + // REFACTOR IN PROGRESS + old_print_unicode(graph, settings) +} + +/// This is the remaining old code, that gradually will be moved to separate +/// functions or the new print_unicode +fn old_print_unicode(graph: &GitGraph, settings: &Settings) -> Result { if graph.all_branches.is_empty() { return Ok((vec![], vec![], vec![])); } From c8ec1a2cdfb3a1d91216b0630b5e749358e2656d Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Thu, 20 Nov 2025 20:28:30 +0100 Subject: [PATCH 16/21] Move empty case --- src/print/unicode.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/print/unicode.rs b/src/print/unicode.rs index d321809..923bf53 100644 --- a/src/print/unicode.rs +++ b/src/print/unicode.rs @@ -55,6 +55,9 @@ graph-lines, text-lines, start-row pub type UnicodeGraphInfo = (Vec, Vec, Vec); /// Creates a text-based visual representation of a graph. pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result { + if graph.all_branches.is_empty() { + return Ok((vec![], vec![], vec![])); + } // 1. Calculate dimensions and inserts // TODO @@ -81,9 +84,6 @@ pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result Result { - if graph.all_branches.is_empty() { - return Ok((vec![], vec![], vec![])); - } let num_cols = 2 * graph .all_branches .iter() From bdba91e490d1cdf8b67e10b0050bd4072b90efea Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Wed, 26 Nov 2025 15:22:42 +0100 Subject: [PATCH 17/21] Extract grid width calculation grid width = graph width * 2 - 1 This is because the grid is rendered with characters that are twice as high as wide. --- src/print/unicode.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/print/unicode.rs b/src/print/unicode.rs index 923bf53..3ddea73 100644 --- a/src/print/unicode.rs +++ b/src/print/unicode.rs @@ -81,16 +81,21 @@ pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result Result { - let num_cols = 2 * graph +/// Calculates the necessary column count for the graph grid. +fn calculate_graph_dimensions(graph: &GitGraph) -> usize { + 2 * graph .all_branches .iter() .map(|b| b.visual.column.unwrap_or(0)) .max() - .unwrap() - + 1; + .unwrap_or(0) + + 1 +} + +/// This is the remaining old code, that gradually will be moved to separate +/// functions or the new print_unicode +fn old_print_unicode(graph: &GitGraph, settings: &Settings) -> Result { + let num_cols = calculate_graph_dimensions(graph); let head_idx = graph.indices.get(&graph.head.oid); From 02740205d62de605ab76b0310556d839652d56a5 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Wed, 26 Nov 2025 15:28:32 +0100 Subject: [PATCH 18/21] Move dimensions and inserts --- src/print/unicode.rs | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/print/unicode.rs b/src/print/unicode.rs index 3ddea73..154b6de 100644 --- a/src/print/unicode.rs +++ b/src/print/unicode.rs @@ -60,10 +60,21 @@ pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result Result usize { /// This is the remaining old code, that gradually will be moved to separate /// functions or the new print_unicode -fn old_print_unicode(graph: &GitGraph, settings: &Settings) -> Result { - let num_cols = calculate_graph_dimensions(graph); +fn old_print_unicode<'a>( + graph: &GitGraph, + settings: &Settings, + num_cols: usize, + inserts: HashMap>>, + wrap_options: Option>, +) -> Result { let head_idx = graph.indices.get(&graph.head.oid); - let inserts = get_inserts(graph, settings.compact); - - let (indent1, indent2) = if let Some((_, ind1, ind2)) = settings.wrapping { - (" ".repeat(ind1.unwrap_or(0)), " ".repeat(ind2.unwrap_or(0))) - } else { - ("".to_string(), "".to_string()) - }; - - let wrap_options = if let Some((width, _, _)) = settings.wrapping { - create_wrapping_options(width, &indent1, &indent2, num_cols + 4)? - } else { - None - }; - // Compute commit text into text_lines and add blank rows // if needed to match branch graph inserts. let mut index_map = vec![]; From 5cccb86550ba1ee360bbc74237277afbb0cf6f6d Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Thu, 20 Nov 2025 20:28:30 +0100 Subject: [PATCH 19/21] Extract fn draw_graph_lines --- src/print/unicode.rs | 72 ++++++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 32 deletions(-) diff --git a/src/print/unicode.rs b/src/print/unicode.rs index 154b6de..57d2fcb 100644 --- a/src/print/unicode.rs +++ b/src/print/unicode.rs @@ -53,6 +53,11 @@ graph-lines, text-lines, start-row 3. start_row: `Vec`: Starting row for commit in the `graph.commits` vector. */ pub type UnicodeGraphInfo = (Vec, Vec, Vec); +type RefactorData = ( + /* offset */ usize, + /* index_map */ Vec, + /* text_lines */ Vec>, +); /// Creates a text-based visual representation of a graph. pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result { if graph.all_branches.is_empty() { @@ -79,17 +84,25 @@ pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result usize { /// This is the remaining old code, that gradually will be moved to separate /// functions or the new print_unicode fn old_print_unicode<'a>( - graph: &GitGraph, + graph: &GitGraph, settings: &Settings, - num_cols: usize, - inserts: HashMap>>, + inserts: &HashMap>>, wrap_options: Option>, -) -> Result { - +) -> Result { let head_idx = graph.indices.get(&graph.head.oid); // Compute commit text into text_lines and add blank rows @@ -161,9 +172,22 @@ fn old_print_unicode<'a>( offset += max_inserts; } + // REFACTOR IN PROGRESS + Ok((offset, index_map, text_lines)) +} + +/// Initializes the grid and draws all commit/branch connections. +fn draw_graph_lines( + graph: &GitGraph, + settings: &Settings, + num_cols: usize, + inserts: &HashMap>>, + index_map: &[usize], + total_rows: usize, +) -> Grid { let mut grid = Grid::new( num_cols, - graph.commits.len() + offset, + total_rows, GridCell { character: SPACE, color: WHITE, @@ -171,7 +195,6 @@ fn old_print_unicode<'a>( }, ); - // Compute branch lines in grid for (idx, info) in graph.commits.iter().enumerate() { let Some(trace) = info.branch_trace else { continue; @@ -180,34 +203,19 @@ fn old_print_unicode<'a>( let column = branch.visual.column.unwrap(); let idx_map = index_map[idx]; - let branch_color = branch.visual.term_color; - + // Draw commit point (DOT or CIRCLE) grid.set( column * 2, idx_map, if info.is_merge { CIRCLE } else { DOT }, - branch_color, + branch.visual.term_color, branch.persistence, ); - draw_parent_lines( - graph, - branch, - &mut grid, - info, - &inserts, - &index_map, - idx, - ); - } - if settings.reverse_commit_order { - text_lines.reverse(); - grid.reverse(); + // Draw parent lines from this commit + draw_parent_lines(graph, branch, &mut grid, info, inserts, index_map, idx); } - - let lines = print_graph(&settings.characters, &grid, text_lines, settings.colored); - - Ok((lines.0, lines.1, index_map)) + grid } fn draw_parent_lines( From eb42f5479fdeadfa716e6ba91ce8a4fad03060d6 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Thu, 20 Nov 2025 20:28:30 +0100 Subject: [PATCH 20/21] Extract fn build_commit_lines_and_map Note, total_rows computation was a complicated way to count lines found in text_lines. --- src/print/unicode.rs | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/print/unicode.rs b/src/print/unicode.rs index 57d2fcb..63fd788 100644 --- a/src/print/unicode.rs +++ b/src/print/unicode.rs @@ -53,11 +53,7 @@ graph-lines, text-lines, start-row 3. start_row: `Vec`: Starting row for commit in the `graph.commits` vector. */ pub type UnicodeGraphInfo = (Vec, Vec, Vec); -type RefactorData = ( - /* offset */ usize, - /* index_map */ Vec, - /* text_lines */ Vec>, -); + /// Creates a text-based visual representation of a graph. pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result { if graph.all_branches.is_empty() { @@ -82,14 +78,11 @@ pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result usize { + 1 } -/// This is the remaining old code, that gradually will be moved to separate -/// functions or the new print_unicode -fn old_print_unicode<'a>( +/// Iterates through commits to compute text lines, blank line inserts, and the index map. +fn build_commit_lines_and_map<'a>( graph: &GitGraph, settings: &Settings, inserts: &HashMap>>, - wrap_options: Option>, -) -> Result { + wrap_options: &Option>, +) -> Result<(Vec>, Vec), String> { let head_idx = graph.indices.get(&graph.head.oid); // Compute commit text into text_lines and add blank rows @@ -131,8 +123,11 @@ fn old_print_unicode<'a>( let mut index_map = vec![]; let mut text_lines = vec![]; let mut offset = 0; + for (idx, info) in graph.commits.iter().enumerate() { index_map.push(idx + offset); + + // Calculate needed graph inserts (for ranges only) let cnt_inserts = if let Some(inserts) = inserts.get(&idx) { inserts .iter() @@ -153,27 +148,28 @@ fn old_print_unicode<'a>( None }; + // Format the commit message lines let lines = format( &settings.format, graph, info, head, settings.colored, - &wrap_options, + wrap_options, )?; let num_lines = if lines.is_empty() { 0 } else { lines.len() - 1 }; let max_inserts = max(cnt_inserts, num_lines); let add_lines = max_inserts - num_lines; + // Extend text_lines with commit lines and blank lines for padding text_lines.extend(lines.into_iter().map(Some)); text_lines.extend((0..add_lines).map(|_| None)); offset += max_inserts; } - // REFACTOR IN PROGRESS - Ok((offset, index_map, text_lines)) + Ok((text_lines, index_map)) } /// Initializes the grid and draws all commit/branch connections. From ee03829987b1fe168f2436459df7016d8faa56b6 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Thu, 20 Nov 2025 20:28:30 +0100 Subject: [PATCH 21/21] Extract fn get_wrapping_options --- src/print/unicode.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/print/unicode.rs b/src/print/unicode.rs index 63fd788..17ca61f 100644 --- a/src/print/unicode.rs +++ b/src/print/unicode.rs @@ -71,11 +71,7 @@ pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result usize { + 1 } +/// Prepares wrapping options, returning the options structure. +// 'a now refers to the lifetime of the indent strings passed in. +fn get_wrapping_options<'a>( + settings: &Settings, + num_cols: usize, + indent1: &'a str, // Takes reference to owned string + indent2: &'a str, // Takes reference to owned string +) -> Result>, String> { + if let Some((width, _, _)) = settings.wrapping { + // We now pass the references directly to create_wrapping_options + create_wrapping_options(width, indent1, indent2, num_cols + 4) + } else { + Ok(None) + } +} + /// Iterates through commits to compute text lines, blank line inserts, and the index map. fn build_commit_lines_and_map<'a>( graph: &GitGraph,