Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle directory-file and file-directory comparisons in the diff #56

Merged
merged 5 commits into from
Apr 23, 2024

Conversation

TanmayPatil105
Copy link
Contributor

@TanmayPatil105 TanmayPatil105 commented Apr 20, 2024

GNU diff treats diff DIRECTORY FILE as diff DIRECTORY/FILE FILE

$ echo a > a
$ mkdir d
$  echo da > d/a
$ diff -u d a
--- d/a	2024-04-20 22:16:11.145369745 +0530
+++ a	2024-04-20 22:15:57.785813179 +0530
@@ -1 +1 @@
-da
+a
$ diff -u a d
--- a	2024-04-20 22:15:57.785813179 +0530
+++ d/a	2024-04-20 22:16:11.145369745 +0530
@@ -1 +1 @@
-a
+da
$ cargo run -- -u d a
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/diffutils -u d a`
--- d/a	2024-04-20 22:16:11.145369745 +0530
+++ a	2024-04-20 22:15:57.785813179 +0530
@@ -1 +1 @@
-da
+a
$ cargo run -- -u a d
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/diffutils -u a d`
--- a	2024-04-20 22:15:57.785813179 +0530
+++ d/a	2024-04-20 22:16:11.145369745 +0530
@@ -1 +1 @@
-a
+da

@TanmayPatil105
Copy link
Contributor Author

TanmayPatil105 commented Apr 20, 2024

Well, this will handle diff -u d a but not diff -u - a < d.

$ diff -u - a < d
--- -/a	2024-04-20 22:16:11.145369745 +0530
+++ a	2024-04-20 22:15:57.785813179 +0530
@@ -1 +1 @@
-da
+a
$ cargo run -- -u - a < d
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s
     Running `target/debug/diffutils -u - a`
Failed to read from-file: Is a directory (os error 21)

GNU diff (3.9+) implementes stdin directory input using openat
See: https://git.savannah.gnu.org/cgit/diffutils.git/tree/src/diff.c#n1541

Copy link

codecov bot commented Apr 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.82%. Comparing base (8de0ca6) to head (8c6a648).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #56      +/-   ##
==========================================
+ Coverage   81.78%   81.82%   +0.03%     
==========================================
  Files          10       10              
  Lines        2877     2927      +50     
  Branches      740      747       +7     
==========================================
+ Hits         2353     2395      +42     
- Misses        503      509       +6     
- Partials       21       23       +2     
Flag Coverage Δ
macos_latest 82.70% <100.00%> (+0.59%) ⬆️
ubuntu_latest 75.19% <76.66%> (+0.44%) ⬆️
windows_latest 15.24% <20.00%> (?)

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.

@sylvestre
Copy link
Sponsor Collaborator

sorry but your test fails on windows with:

---- read_from_directory stdout ----
---- read_from_directory stderr ----
thread 'main' panicked at tests\integration.rs:257:5:
assertion `left == right` failed
  left: "--- target/integration/d\\a\tTIMESTAMP\n+++ target/integration/a\tTIMESTAMP\n@@ -1 +1 @@\n-da\n+a\n"
 right: "--- target/integration/d/a\tTIMESTAMP\n+++ target/integration/a\tTIMESTAMP\n@@ -1 +1 @@\n-da\n+a\n"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


@oSoMoN
Copy link
Collaborator

oSoMoN commented Apr 23, 2024

Well, this will handle diff -u d a but not diff -u - a < d.

That would be nice to have, but it can be implemented in a followup PR.

tests/integration.rs Outdated Show resolved Hide resolved
tests/integration.rs Outdated Show resolved Hide resolved
@oSoMoN oSoMoN merged commit e7dc655 into uutils:main Apr 23, 2024
25 checks passed
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.

3 participants