fix: prevent side-by-side panic with narrow width or large tab width
When rendering a side-by-side diff, `SourceDimensions::new` could compute a per-column content width that was not greater than the tab width. That violates the precondition of `split_and_apply`, which asserts `max_len > tab_width` so it can always fit at least one expanded tab and make progress when wrapping. As a result difftastic aborted with a panic on ordinary input, for example `difft --width 14 a b` or, even at the default terminal width, `difft --tab-width 100 a b`. A very small `--width` could also drive the total display width down to two columns or fewer, underflowing the `display_width - SPACER.len()` subtraction (previously guarded by an assertion that itself aborted). Enforce both invariants in `SourceDimensions::new`: keep room for the column spacer, and ensure each content column is wider than the tab width. Narrow terminals now degrade gracefully instead of crashing.
This commit is contained in:
1 parent
32b79c1a63
commit
7f1f320438
2 files changed
+84
-7
No files matched your search
@@ -178,6 +178,7 @@ impl SourceDimensions {
|
|||||||
rhs_max_line_in_file: LineNumber,
|
rhs_max_line_in_file: LineNumber,
|
||||||
lhs_content_max_width: usize,
|
lhs_content_max_width: usize,
|
||||||
rhs_content_max_width: usize,
|
rhs_content_max_width: usize,
|
||||||
|
tab_width: usize,
|
||||||
) -> Self {
|
) -> Self {
|
||||||
let lhs_line_nums_width = format_line_num(lhs_max_line_visible).len();
|
let lhs_line_nums_width = format_line_num(lhs_max_line_visible).len();
|
||||||
let rhs_line_nums_width = format_line_num(rhs_max_line_visible).len();
|
let rhs_line_nums_width = format_line_num(rhs_max_line_visible).len();
|
||||||
@@ -206,11 +207,11 @@ impl SourceDimensions {
|
|||||||
+ rhs_line_nums_width
|
+ rhs_line_nums_width
|
||||||
+ content_max_width;
|
+ content_max_width;
|
||||||
let display_width = min(terminal_width, width_without_truncation);
|
let display_width = min(terminal_width, width_without_truncation);
|
||||||
|
// Always leave room for the spacer between the two columns, even
|
||||||
|
// when the user requests a tiny `--width`. Without this floor the
|
||||||
|
// subtractions below underflow.
|
||||||
|
let display_width = max(display_width, SPACER.len() + 1);
|
||||||
|
|
||||||
assert!(
|
|
||||||
display_width > SPACER.len(),
|
|
||||||
"Terminal total width should not overflow"
|
|
||||||
);
|
|
||||||
let lhs_total_width = (display_width - SPACER.len()) / 2;
|
let lhs_total_width = (display_width - SPACER.len()) / 2;
|
||||||
|
|
||||||
let lhs_content_width = if lhs_line_nums_width < lhs_total_width {
|
let lhs_content_width = if lhs_line_nums_width < lhs_total_width {
|
||||||
@@ -235,6 +236,13 @@ impl SourceDimensions {
|
|||||||
// same point on both sides.
|
// same point on both sides.
|
||||||
let content_width = min(lhs_content_width, rhs_content_width);
|
let content_width = min(lhs_content_width, rhs_content_width);
|
||||||
|
|
||||||
|
// Each content column must be wide enough to hold at least one
|
||||||
|
// expanded tab. `split_and_apply` relies on this to make
|
||||||
|
// progress when wrapping, and panics otherwise. A small
|
||||||
|
// `--width` or a large `--tab-width` can push the computed
|
||||||
|
// content width below this bound, so enforce it here.
|
||||||
|
let content_width = max(content_width, tab_width + 1);
|
||||||
|
|
||||||
Self {
|
Self {
|
||||||
lhs_content_display_width: content_width,
|
lhs_content_display_width: content_width,
|
||||||
rhs_content_display_width: content_width,
|
rhs_content_display_width: content_width,
|
||||||
@@ -596,6 +604,7 @@ pub(crate) fn print(
|
|||||||
rhs_max_line_in_file,
|
rhs_max_line_in_file,
|
||||||
lhs_content_max_width,
|
lhs_content_max_width,
|
||||||
rhs_content_max_width,
|
rhs_content_max_width,
|
||||||
|
display_options.tab_width,
|
||||||
);
|
);
|
||||||
|
|
||||||
for (i, hunk) in hunks.iter().enumerate() {
|
for (i, hunk) in hunks.iter().enumerate() {
|
||||||
@@ -786,7 +795,7 @@ mod tests {
|
|||||||
use pretty_assertions::assert_eq;
|
use pretty_assertions::assert_eq;
|
||||||
|
|
||||||
use super::*;
|
use super::*;
|
||||||
use crate::options::DEFAULT_TERMINAL_WIDTH;
|
use crate::options::{DEFAULT_TAB_WIDTH, DEFAULT_TERMINAL_WIDTH};
|
||||||
use crate::parse::guess_language::Language;
|
use crate::parse::guess_language::Language;
|
||||||
use crate::syntax::{AtomKind, MatchKind, TokenKind};
|
use crate::syntax::{AtomKind, MatchKind, TokenKind};
|
||||||
|
|
||||||
@@ -800,12 +809,49 @@ mod tests {
|
|||||||
10.into(),
|
10.into(),
|
||||||
9999,
|
9999,
|
||||||
9999,
|
9999,
|
||||||
|
DEFAULT_TAB_WIDTH,
|
||||||
);
|
);
|
||||||
|
|
||||||
assert_eq!(source_dims.lhs_line_nums_width, 2);
|
assert_eq!(source_dims.lhs_line_nums_width, 2);
|
||||||
assert_eq!(source_dims.rhs_line_nums_width, 3);
|
assert_eq!(source_dims.rhs_line_nums_width, 3);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The content columns must always be wide enough to hold at least
|
||||||
|
/// one expanded tab, otherwise `split_and_apply` panics. This must
|
||||||
|
/// hold even for a tiny `--width` or a large `--tab-width`.
|
||||||
|
#[test]
|
||||||
|
fn test_content_width_exceeds_tab_width() {
|
||||||
|
for terminal_width in [0, 1, 2, 3, 14, 80] {
|
||||||
|
for tab_width in [1, 4, 8, 100] {
|
||||||
|
let source_dims = SourceDimensions::new(
|
||||||
|
terminal_width,
|
||||||
|
1.into(),
|
||||||
|
1.into(),
|
||||||
|
1.into(),
|
||||||
|
1.into(),
|
||||||
|
9999,
|
||||||
|
9999,
|
||||||
|
tab_width,
|
||||||
|
);
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
source_dims.lhs_content_display_width > tab_width,
|
||||||
|
"lhs content width {} must exceed tab width {} (terminal_width = {})",
|
||||||
|
source_dims.lhs_content_display_width,
|
||||||
|
tab_width,
|
||||||
|
terminal_width,
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
source_dims.rhs_content_display_width > tab_width,
|
||||||
|
"rhs content width {} must exceed tab width {} (terminal_width = {})",
|
||||||
|
source_dims.rhs_content_display_width,
|
||||||
|
tab_width,
|
||||||
|
terminal_width,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_format_missing_line_num() {
|
fn test_format_missing_line_num() {
|
||||||
let source_dims = SourceDimensions::new(
|
let source_dims = SourceDimensions::new(
|
||||||
@@ -816,6 +862,7 @@ mod tests {
|
|||||||
1.into(),
|
1.into(),
|
||||||
9999,
|
9999,
|
||||||
9999,
|
9999,
|
||||||
|
DEFAULT_TAB_WIDTH,
|
||||||
);
|
);
|
||||||
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
@@ -830,8 +877,16 @@ mod tests {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_format_missing_line_num_at_end() {
|
fn test_format_missing_line_num_at_end() {
|
||||||
let source_dims =
|
let source_dims = SourceDimensions::new(
|
||||||
SourceDimensions::new(80, 1.into(), 1.into(), 1.into(), 1.into(), 9999, 9999);
|
80,
|
||||||
|
1.into(),
|
||||||
|
1.into(),
|
||||||
|
1.into(),
|
||||||
|
1.into(),
|
||||||
|
9999,
|
||||||
|
9999,
|
||||||
|
DEFAULT_TAB_WIDTH,
|
||||||
|
);
|
||||||
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
format_missing_line_num(1.into(), &source_dims, Side::Left, false, true),
|
format_missing_line_num(1.into(), &source_dims, Side::Left, false, true),
|
||||||
|
|||||||
@@ -137,6 +137,28 @@ fn text_changes_at_end_doesnt_crash() {
|
|||||||
cmd.assert().success();
|
cmd.assert().success();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn narrow_width_doesnt_crash() {
|
||||||
|
let mut cmd = get_base_command();
|
||||||
|
|
||||||
|
cmd.arg("--width=14")
|
||||||
|
.arg("sample_files/text_1.txt")
|
||||||
|
.arg("sample_files/text_2.txt");
|
||||||
|
|
||||||
|
cmd.assert().success();
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn large_tab_width_doesnt_crash() {
|
||||||
|
let mut cmd = get_base_command();
|
||||||
|
|
||||||
|
cmd.arg("--tab-width=100")
|
||||||
|
.arg("sample_files/text_1.txt")
|
||||||
|
.arg("sample_files/text_2.txt");
|
||||||
|
|
||||||
|
cmd.assert().success();
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn makefile_text_as_atom() {
|
fn makefile_text_as_atom() {
|
||||||
let mut cmd = get_base_command();
|
let mut cmd = get_base_command();
|
||||||
|
|||||||
Reference in new issue
Block a user