Skip to content

Commit

Permalink
Fix side-by-side mode without line numbers
Browse files Browse the repository at this point in the history
Even though this feature enables line numbers, these can be disabled via
the git config `delta.line-numbers=false`. So always enable line numbers,
even if they do not print numbers.

Construct more structs with ..Self::default()
  • Loading branch information
th1000s authored and dandavison committed Oct 26, 2021
1 parent fbb654d commit 59e8908
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 25 deletions.
24 changes: 20 additions & 4 deletions src/features/line_numbers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use regex::Regex;
use crate::config;
use crate::delta::State;
use crate::features::hyperlinks;
use crate::features::side_by_side::ansifill;
use crate::features::side_by_side::ansifill::{self, ODD_PAD_CHAR};
use crate::features::side_by_side::{Left, PanelSide, Right};
use crate::features::OptionValueFunction;
use crate::format::{self, Align, Placeholder};
Expand Down Expand Up @@ -175,9 +175,7 @@ impl<'a> LineNumbersData<'a> {
insert_center_space_on_odd_width,
),
),
line_number: MinusPlus::new(0, 0),
hunk_max_line_number_width: 0,
plus_file: "".to_string(),
..Self::default()
}
}

Expand All @@ -193,6 +191,24 @@ impl<'a> LineNumbersData<'a> {
self.plus_file = plus_file;
}

pub fn empty_for_sbs(use_full_width: ansifill::UseFullPanelWidth) -> LineNumbersData<'a> {
let insert_center_space_on_odd_width = use_full_width.pad_width();
Self {
format_data: if insert_center_space_on_odd_width {
let format_left = vec![format::FormatStringPlaceholderData::default()];
let format_right = vec![format::FormatStringPlaceholderData {
prefix: format!("{}", ODD_PAD_CHAR).into(),
prefix_len: 1,
..Default::default()
}];
MinusPlus::new(format_left, format_right)
} else {
MinusPlus::default()
},
..Self::default()
}
}

pub fn formatted_width(&self) -> SideBySideLineWidth {
let format_data_width = |format_data: &format::FormatStringData<'a>| {
// Provide each Placeholder with the max_line_number_width to calculate the
Expand Down
4 changes: 2 additions & 2 deletions src/features/side_by_side.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,8 @@ pub mod ansifill {
/// The solution in this case is to add `ODD_PAD_CHAR` before the first line number in
/// the right panel and increasing its width by one, thus using the full terminal width
/// with the two panels.
/// This also means line numbers can not be disabled in side-by-side mode plus ANSI, as
/// this leaves no place for this fix.
/// This also means line numbers can not be disabled in side-by-side mode, but they may
/// not actually paint numbers.
#[derive(Clone, Debug)]
pub struct UseFullPanelWidth(pub bool);
impl UseFullPanelWidth {
Expand Down
4 changes: 1 addition & 3 deletions src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,9 @@ pub fn parse_line_number_format<'a>(
format_data.push(FormatStringPlaceholderData {
prefix,
prefix_len,
placeholder: None,
alignment_spec: None,
width: None,
suffix: SmolStr::new(format_string),
suffix_len: format_string.graphemes(true).count(),
..Default::default()
})
}

Expand Down
2 changes: 2 additions & 0 deletions src/handlers/hunk_header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ impl<'a> StateMachine<'a> {
if self.config.line_numbers {
self.painter
.line_numbers_data
.as_mut()
.unwrap()
.initialize_hunk(&line_numbers, self.plus_file.to_string());
}

Expand Down
43 changes: 27 additions & 16 deletions src/paint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ pub struct Painter<'a> {
pub highlighter: Option<HighlightLines<'a>>,
pub config: &'a config::Config,
pub output_buffer: String,
pub line_numbers_data: line_numbers::LineNumbersData<'a>,
// If config.line_numbers is true, then the following is always Some().
// In side-by-side mode it is always Some (but possibly an empty one), even
// if config.line_numbers is false. See `UseFullPanelWidth` as well.
pub line_numbers_data: Option<line_numbers::LineNumbersData<'a>>,
}

// How the background of a line is filled up to the end
Expand Down Expand Up @@ -59,17 +62,19 @@ impl<'a> Painter<'a> {
let panel_width_fix = ansifill::UseFullPanelWidth::new(config);

let line_numbers_data = if config.line_numbers {
line_numbers::LineNumbersData::from_format_strings(
Some(line_numbers::LineNumbersData::from_format_strings(
&config.line_numbers_format,
panel_width_fix,
)
))
} else if config.side_by_side {
// If line numbers are disabled in side-by-side then the data is still used
// for width calculaction and to pad odd width to even, see `UseFullPanelWidth`
// for details.
Some(line_numbers::LineNumbersData::empty_for_sbs(
panel_width_fix,
))
} else {
// See `UseFullPanelWidth` for details why, but this can't happen at the time
// of writing because side-by-side automatically activates line numbers.
debug_assert!(
!config.side_by_side && config.line_fill_method == BgFillMethod::TryAnsiSequence
);
line_numbers::LineNumbersData::default()
None
};
Self {
minus_lines: Vec::new(),
Expand Down Expand Up @@ -192,6 +197,10 @@ impl<'a> Painter<'a> {
.collect(),
);

let line_numbers_data = self.line_numbers_data.as_mut().unwrap_or_else(|| {
delta_unreachable("side-by-side requires Some(line_numbers_data)")
});

let bg_fill_left_right = MinusPlus::new(
// Using an ANSI sequence to fill the left panel would not work.
BgShouldFill::With(BgFillMethod::Spaces),
Expand All @@ -207,7 +216,7 @@ impl<'a> Painter<'a> {
if self.config.wrap_config.max_lines == 1 {
(false, MinusPlus::default(), MinusPlus::default())
} else {
let line_width = available_line_width(self.config, &self.line_numbers_data);
let line_width = available_line_width(self.config, line_numbers_data);

let lines = MinusPlus::new(&self.minus_lines, &self.plus_lines);

Expand Down Expand Up @@ -244,18 +253,20 @@ impl<'a> Painter<'a> {
line_alignment,
&mut self.output_buffer,
self.config,
&mut Some(&mut self.line_numbers_data),
&mut Some(line_numbers_data),
bg_fill_left_right,
);
} else {
// Unified mode:

if !self.minus_lines.is_empty() {
Painter::paint_lines(
minus_line_syntax_style_sections,
minus_line_diff_style_sections,
self.minus_lines.iter().map(|(_, state)| state),
&mut self.output_buffer,
self.config,
&mut Some(&mut self.line_numbers_data),
&mut self.line_numbers_data.as_mut(),
if self.config.keep_plus_minus_markers {
Some(self.config.minus_style.paint("-"))
} else {
Expand All @@ -272,7 +283,7 @@ impl<'a> Painter<'a> {
self.plus_lines.iter().map(|(_, state)| state),
&mut self.output_buffer,
self.config,
&mut Some(&mut self.line_numbers_data),
&mut self.line_numbers_data.as_mut(),
if self.config.keep_plus_minus_markers {
Some(self.config.plus_style.paint("+"))
} else {
Expand Down Expand Up @@ -311,7 +322,7 @@ impl<'a> Painter<'a> {
vec![diff_style_sections],
&mut self.output_buffer,
self.config,
&mut Some(&mut self.line_numbers_data),
&mut self.line_numbers_data.as_mut(),
painted_prefix,
BgShouldFill::With(BgFillMethod::Spaces),
);
Expand All @@ -322,7 +333,7 @@ impl<'a> Painter<'a> {
[state].iter(),
&mut self.output_buffer,
self.config,
&mut Some(&mut self.line_numbers_data),
&mut self.line_numbers_data.as_mut(),
painted_prefix,
None,
BgShouldFill::With(BgFillMethod::Spaces),
Expand Down Expand Up @@ -530,7 +541,7 @@ impl<'a> Painter<'a> {
) -> (String, bool) {
let mut ansi_strings = Vec::new();

let output_line_numbers = config.line_numbers && line_numbers_data.is_some();
let output_line_numbers = line_numbers_data.is_some();
if output_line_numbers {
// Unified diff lines are printed in one go, but side-by-side lines
// are printed in two parts, so do not increment line numbers when the
Expand Down

0 comments on commit 59e8908

Please sign in to comment.