chore(git): remove dead DiffDelta::similarity and simplify parse_diff_raw#1336
chore(git): remove dead DiffDelta::similarity and simplify parse_diff_raw#1336jwiegley wants to merge 1 commit into
Conversation
jwiegley
force-pushed
the
dc/git-diff-tree-cleanup
branch
from
5c54e93 to
c5aad7a
Compare
jwiegley
force-pushed
the
dc/git-rewrite-log-cleanup
branch
from
0dd3d00 to
580dd2c
Compare
jwiegley
force-pushed
the
dc/git-diff-tree-cleanup
branch
from
c5aad7a to
906017f
Compare
jwiegley
force-pushed
the
dc/git-rewrite-log-cleanup
branch
2 times, most recently
from
27302e2 to
613436b
Compare
jwiegley
force-pushed
the
dc/git-diff-tree-cleanup
branch
3 times, most recently
from
d4ae464 to
a7d4c29
Compare
jwiegley
force-pushed
the
dc/git-rewrite-log-cleanup
branch
from
1db811d to
8a9c5f3
Compare
jwiegley
force-pushed
the
dc/git-diff-tree-cleanup
branch
from
a7d4c29 to
93e2dc8
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 Pre-existing: rename/copy path ordering may be swapped vs git's actual output
The parse_diff_raw function treats the first NUL-separated path after metadata as the new_path and the second as the old_path for renames/copies (line 250-258). However, git's diff --raw -z format outputs <src_path>\0<dst_path>\0 for renames, where src_path is the old/source path and dst_path is the new/destination path. The test at line 305 also places paths in the non-standard order (src/renamed.rs\0src/original.rs\0 instead of src/original.rs\0src/renamed.rs\0), so the test passes despite the potential inversion. This is a pre-existing issue not introduced by this PR, and the PR does not modify this logic. Worth verifying against real git output to confirm whether the path ordering is correct.
(Refers to lines 250-258)
Was this helpful? React with 👍 or 👎 to provide feedback.
jwiegley
force-pushed
the
dc/git-diff-tree-cleanup
branch
from
93e2dc8 to
ef483d0
Compare
jwiegley
force-pushed
the
dc/git-rewrite-log-cleanup
branch
from
3733b1b to
6bdf5c8
Compare
| pub fn len(&self) -> usize { | ||
| self.deltas.len() | ||
| } | ||
|
|
||
| #[allow(dead_code)] | ||
| pub fn is_empty(&self) -> bool { | ||
| self.deltas.is_empty() | ||
| } |
There was a problem hiding this comment.
🚩 Diff::len() and Diff::is_empty() may now trigger dead_code warnings
Diff::len() and Diff::is_empty() had their #[allow(dead_code)] removed, but I could not find any callers of these methods outside of the module's own tests (where Vec::len() is used on the raw Vec<DiffDelta>, not on Diff). If these are truly unused in production code, removing the annotation will produce compiler warnings. This would be caught by CI (task lint), so it's not a runtime bug, but it may cause the build to fail if warnings are treated as errors.
Was this helpful? React with 👍 or 👎 to provide feedback.
…_raw DiffDelta::similarity() had no callers — the similarity score is parsed and stored for completeness but only accessed directly in same-module tests. Also removes stale #[allow(dead_code)] from DiffStatus, DiffFile, and DiffDelta (all actively used), and simplifies a redundant if_same_then_else branch in parse_diff_raw whose two arms were identical. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jwiegley
force-pushed
the
dc/git-rewrite-log-cleanup
branch
from
6bdf5c8 to
559d264
Compare
jwiegley
force-pushed
the
dc/git-diff-tree-cleanup
branch
from
ef483d0 to
360b57e
Compare
1 participant
Summary
DiffDelta::similarity() had no callers — the score is parsed and stored but only accessed directly in same-module tests. Also removes stale #[allow(dead_code)] from DiffStatus, DiffFile, and DiffDelta (all actively used), and simplifies a redundant if_same_then_else branch in parse_diff_raw whose two arms were identical.
Test plan
task buildpassestask lintpasses🤖 Generated with Claude Code