From 6705bd3d757e6c25010eba13c33653d44c1ea276 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 10 Jun 2025 04:59:00 +0200 Subject: [PATCH 1/4] refactor: Reduce indentation By using Rust a little differently the amount of curtains can be reduced. --- src/print/svg.rs | 65 +++++----- src/print/unicode.rs | 299 ++++++++++++++++++++++--------------------- 2 files changed, 186 insertions(+), 178 deletions(-) diff --git a/src/print/svg.rs b/src/print/svg.rs index 35cba5a..8676cdf 100644 --- a/src/print/svg.rs +++ b/src/print/svg.rs @@ -37,37 +37,40 @@ pub fn print_svg(graph: &GitGraph, settings: &Settings) -> Result 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 inserts = &inserts[&split_index]; - for (insert_idx, sub_entry) in inserts.iter().enumerate() { - for occ in sub_entry { - match occ { - Occ::Commit(_, _) => {} - Occ::Range(i1, i2, _, _) => { - if *i1 == idx && i2 == par_idx { - vline( - &mut grid, - (idx_map, split_idx_map + insert_idx), - column, - color, - pers, - ); - hline( - &mut grid, - split_idx_map + insert_idx, - (par_column, column), - info.is_merge && p > 0, - color, - pers, - ); - vline( - &mut grid, - (split_idx_map + insert_idx, par_idx_map), - par_column, - color, - pers, - ); - } - } - } + if branch.visual.column == par_branch.visual.column { + if par_idx_map > 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 inserts = &inserts[&split_index]; + for (insert_idx, sub_entry) in inserts.iter().enumerate() { + for occ in sub_entry { + match occ { + Occ::Commit(_, _) => {} + Occ::Range(i1, i2, _, _) => { + if *i1 == idx && i2 == par_idx { + vline( + &mut grid, + (idx_map, split_idx_map + insert_idx), + column, + color, + pers, + ); + hline( + &mut grid, + split_idx_map + insert_idx, + (par_column, column), + info.is_merge && p > 0, + color, + pers, + ); + vline( + &mut grid, + (split_idx_map + insert_idx, par_idx_map), + par_column, + color, + pers, + ); } } } @@ -478,100 +482,101 @@ fn get_inserts(graph: &GitGraph, compact: bool) -> HashMap>> // Iterate through the two possible parents of the current commit. for p in 0..2 { - // If the commit has a parent at this index (0 for the first parent, 1 for the second). - if let Some(par_oid) = info.parents[p] { - // Try to find the index of the parent commit in the `graph.commits` vector. - if let Some(par_idx) = graph.indices.get(&par_oid) { - 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(); - // Determine the sorted range of columns between the current commit and its parent. - let column_range = sorted(column, par_column); - - // If the column of the current commit is different from the column of its parent, - // it means we need to draw a horizontal line (an "insert") to connect them. - if column != par_column { - // Find the index in the `graph.commits` list where the visual connection - // should deviate from the parent's line. This helps in drawing the graph - // correctly when branches diverge or merge. - let split_index = super::get_deviate_index(graph, idx, *par_idx); - // Access the entry in the `inserts` map for the `split_index`. - match inserts.entry(split_index) { - // If there's already an entry at this `split_index` (meaning other - // insertions might be needed before this commit). - Occupied(mut entry) => { - // Find the first available row in the existing vector of rows - // where the new range doesn't overlap with existing occupations. - let mut insert_at = entry.get().len(); - for (insert_idx, sub_entry) in entry.get().iter().enumerate() { - let mut occ = false; - // Check for overlaps with existing `Occ` in the current row. - for other_range in sub_entry { - // Check if the current column range overlaps with the other range. - if other_range.overlaps(&column_range) { - match other_range { - // If the other occupation is a commit. - Occ::Commit(target_index, _) => { - // In compact mode, we might allow overlap with the commit itself - // for merge commits (specifically the second parent) to keep the - // graph tighter. - if !compact - || !info.is_merge - || idx != *target_index - || p == 0 - { - occ = true; - break; - } + let parent = info.parents[p]; + let Some(par_oid) = parent else { + continue; + }; + // Try to find the index of the parent commit in the `graph.commits` vector. + if let Some(par_idx) = graph.indices.get(&par_oid) { + 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(); + // Determine the sorted range of columns between the current commit and its parent. + let column_range = sorted(column, par_column); + + // If the column of the current commit is different from the column of its parent, + // it means we need to draw a horizontal line (an "insert") to connect them. + if column != par_column { + // Find the index in the `graph.commits` list where the visual connection + // should deviate from the parent's line. This helps in drawing the graph + // correctly when branches diverge or merge. + let split_index = super::get_deviate_index(graph, idx, *par_idx); + // Access the entry in the `inserts` map for the `split_index`. + match inserts.entry(split_index) { + // If there's already an entry at this `split_index` (meaning other + // insertions might be needed before this commit). + Occupied(mut entry) => { + // Find the first available row in the existing vector of rows + // where the new range doesn't overlap with existing occupations. + let mut insert_at = entry.get().len(); + for (insert_idx, sub_entry) in entry.get().iter().enumerate() { + let mut occ = false; + // Check for overlaps with existing `Occ` in the current row. + for other_range in sub_entry { + // Check if the current column range overlaps with the other range. + if other_range.overlaps(&column_range) { + match other_range { + // If the other occupation is a commit. + Occ::Commit(target_index, _) => { + // In compact mode, we might allow overlap with the commit itself + // for merge commits (specifically the second parent) to keep the + // graph tighter. + if !compact + || !info.is_merge + || idx != *target_index + || p == 0 + { + occ = true; + break; } - // If the other occupation is a range (another connection). - Occ::Range(o_idx, o_par_idx, _, _) => { - // Avoid overlap with connections between the same commits. - if idx != *o_idx && par_idx != o_par_idx { - occ = true; - break; - } + } + // If the other occupation is a range (another connection). + Occ::Range(o_idx, o_par_idx, _, _) => { + // Avoid overlap with connections between the same commits. + if idx != *o_idx && par_idx != o_par_idx { + occ = true; + break; } } } } - // If no overlap is found in this row, we can insert here. - if !occ { - insert_at = insert_idx; - break; - } } - // Get a mutable reference to the vector of rows for this `split_index`. - let vec = entry.get_mut(); - // If no suitable row was found, add a new row. - if insert_at == vec.len() { - vec.push(vec![Occ::Range( - idx, - *par_idx, - column_range.0, - column_range.1, - )]); - } else { - // Otherwise, insert the new range into the found row. - vec[insert_at].push(Occ::Range( - idx, - *par_idx, - column_range.0, - column_range.1, - )); + // If no overlap is found in this row, we can insert here. + if !occ { + insert_at = insert_idx; + break; } } - // If there's no entry at this `split_index` yet. - Vacant(entry) => { - // Create a new entry with a single row containing the range. - entry.insert(vec![vec![Occ::Range( + // Get a mutable reference to the vector of rows for this `split_index`. + let vec = entry.get_mut(); + // If no suitable row was found, add a new row. + if insert_at == vec.len() { + vec.push(vec![Occ::Range( idx, *par_idx, column_range.0, column_range.1, - )]]); + )]); + } else { + // Otherwise, insert the new range into the found row. + vec[insert_at].push(Occ::Range( + idx, + *par_idx, + column_range.0, + column_range.1, + )); } } + // If there's no entry at this `split_index` yet. + Vacant(entry) => { + // Create a new entry with a single row containing the range. + entry.insert(vec![vec![Occ::Range( + idx, + *par_idx, + column_range.0, + column_range.1, + )]]); + } } } } From 1d6844015f3b6552358d1a6adca865ba69b3d39d Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 10 Jun 2025 04:59:00 +0200 Subject: [PATCH 2/4] refactor: Extract function zig_zag_line Move code out of deeply nested function --- src/print/unicode.rs | 45 ++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/print/unicode.rs b/src/print/unicode.rs index 5a20062..e90c81d 100644 --- a/src/print/unicode.rs +++ b/src/print/unicode.rs @@ -191,28 +191,13 @@ pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result {} Occ::Range(i1, i2, _, _) => { if *i1 == idx && i2 == par_idx { - vline( - &mut grid, - (idx_map, split_idx_map + insert_idx), - column, - color, - pers, - ); - hline( - &mut grid, - split_idx_map + insert_idx, - (par_column, column), - info.is_merge && p > 0, - color, - pers, - ); - vline( - &mut grid, - (split_idx_map + insert_idx, par_idx_map), - par_column, - color, - pers, - ); + 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); } } } @@ -265,6 +250,22 @@ fn create_wrapping_options<'a>( Ok(wrapping) } +/// Draw a line that connects two commits on different columns +fn zig_zag_line( + grid: &mut Grid, + row123: (usize, usize, usize), + col12: (usize, usize), + is_merge: bool, + color: u8, + pers: u8, +) { + let (row1, row2, row3) = row123; + let (col1, col2) = col12; + vline(grid, (row1, row2), col1, color, pers); + hline(grid, row2, (col2, col1), is_merge, color, pers); + vline(grid, (row2, row3), col2, color, pers); +} + /// Draws a vertical line fn vline(grid: &mut Grid, (from, to): (usize, usize), column: usize, color: u8, pers: u8) { for i in (from + 1)..to { From 7f34f9afea0863cddf10c6096ad9a689ae68225d Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Tue, 10 Jun 2025 04:59:00 +0200 Subject: [PATCH 3/4] refactor: Extract function find_insert_idx Move code out of deeply nested function --- src/print/unicode.rs | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/print/unicode.rs b/src/print/unicode.rs index e90c81d..a18b42e 100644 --- a/src/print/unicode.rs +++ b/src/print/unicode.rs @@ -184,25 +184,14 @@ pub fn print_unicode(graph: &GitGraph, settings: &Settings) -> Result {} - Occ::Range(i1, i2, _, _) => { - if *i1 == idx && i2 == par_idx { - 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); - } - } - } - } - } + 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); } } } @@ -250,6 +239,20 @@ fn create_wrapping_options<'a>( Ok(wrapping) } +/// Find the index of the insert that connects the two commits +fn find_insert_idx(inserts: &[Vec], child_idx: usize, parent_idx: usize) -> Option { + for (insert_idx, sub_entry) in inserts.iter().enumerate() { + for occ in sub_entry { + if let Occ::Range(i1, i2, _, _) = occ { + if *i1 == child_idx && *i2 == parent_idx { + return Some(insert_idx); + } + } + } + } + None +} + /// Draw a line that connects two commits on different columns fn zig_zag_line( grid: &mut Grid, From 9cbc0600232f980dc70438480bab6aa14f1b1d63 Mon Sep 17 00:00:00 2001 From: Peer Sommerlund Date: Mon, 2 Jun 2025 08:03:26 +0200 Subject: [PATCH 4/4] Fix #67: Commit parent outside scope --- src/print/svg.rs | 10 ++++++++++ src/print/unicode.rs | 12 ++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/print/svg.rs b/src/print/svg.rs index 8676cdf..f6ef745 100644 --- a/src/print/svg.rs +++ b/src/print/svg.rs @@ -42,6 +42,16 @@ pub fn print_svg(graph: &GitGraph, settings: &Settings) -> Result Result, @@ -809,6 +820,7 @@ impl Grid { pub fn new(width: usize, height: usize, initial: GridCell) -> Self { Grid { width, + height, data: vec![initial; width * height], } }