Skip to content

Conversation

@rodo-gatti
Copy link

This PR adds a Rust implementation of diff3.

Fixes #15

@codecov
Copy link

codecov bot commented Dec 14, 2025

Codecov Report

❌ Patch coverage is 91.02711% with 235 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.66%. Comparing base (4eee9ce) to head (6636042).
⚠️ Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
src/diff3.rs 87.84% 197 Missing and 10 partials ⚠️
tests/integration.rs 96.93% 27 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #148      +/-   ##
==========================================
+ Coverage   86.26%   87.66%   +1.39%     
==========================================
  Files          13       14       +1     
  Lines        6203     8835    +2632     
  Branches      514      674     +160     
==========================================
+ Hits         5351     7745    +2394     
- Misses        851     1078     +227     
- Partials        1       12      +11     
Flag Coverage Δ
macos_latest 87.29% <89.91%> (+1.08%) ⬆️
ubuntu_latest 87.67% <90.98%> (+1.34%) ⬆️
windows_latest 22.06% <26.41%> (+2.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

src/diff3.rs Outdated
// Handle options
if param_str.starts_with('-') && param_str != "-" {
// Check for combined short options
let param_str = param_str.as_ref();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

param_str is already a reference, no ?
is as_ref() necessary ?

src/diff3.rs Outdated
}
}

// Collect remaining arguments
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lot of duplication from line 179

src/diff3.rs Outdated

/// Fast content hash for quick equality checks on large files
/// Uses a simple FNV-1a hash for performance optimization
#[allow(dead_code)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why keep it if it is dead code ?

src/diff3.rs Outdated
}

#[derive(Debug, Clone)]
#[allow(dead_code)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same, if it is dead code, why keep it ?

src/diff3.rs Outdated

for &byte in &content[..sample_size] {
// Non-text bytes are control characters (0-8, 14-31, 127) except common ones (9=tab, 10=LF, 13=CR)
if (byte < 9 || (byte > 13 && byte < 32) || byte == 127)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be simplified to matches!(byte, 0..=8 | 14..=31 | 127)
no ?

src/diff3.rs Outdated
writeln!(
&mut output,
"Binary files {} and {} differ",
params.mine.to_string_lossy(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple to_string_lossy() calls - convert once and reuse for efficiency"

output_mode: Diff3OutputMode::All,
text: false,
labels: [None, None, None],
strip_trailing_cr: false,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strip_trailing_cr parameter is defined but never implemented in the processing logic

pub text: bool,
pub labels: [Option<String>; 3],
pub strip_trailing_cr: bool,
pub initial_tab: bool,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initial_tab parameter is defined but never used in output formatting

@sylvestre
Copy link
Collaborator

i did a first pass but many things needs to be improved before i spend more time on it :)

@rodo-gatti rodo-gatti marked this pull request as draft January 3, 2026 20:55
src/diff3.rs Outdated
output
}

#[allow(clippy::too_many_arguments)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refactor to avoid this warning

src/diff3.rs Outdated
output
}

#[allow(clippy::too_many_arguments)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

src/diff3.rs Outdated
let mut yours = None;
let mut label_count = 0;

#[allow(clippy::while_let_on_iterator)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix this

…h BSD diff3 which has different exit code behavior for the -E flag compared to GNU diff3. Added is_gnu_diff3() helper and modified gnu_compat_ed_with_overlap test to skip on BSD diff3 systems.
@rodo-gatti rodo-gatti marked this pull request as ready for review January 4, 2026 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

diff3 command is not implemented

2 participants