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

feat(diff): show diff statistics #1178

Merged
merged 4 commits into from
Aug 10, 2024

Conversation

nardoor
Copy link
Contributor

@nardoor nardoor commented Jul 13, 2024

closes #440

Description

The proposed code contains:

  • a DiffStatistics structure used to hold count of stats
    • this structure offers a few helping functions that will look at a given NodeType to increment the associated counter
    • it has a Display implementation used to display the statistics in STDOUT
  • this structure is used in the diff function

Exemple

rustic --log-level debug -r test-repo --password password diff 8e57051d:config ./config
[INFO] using no config file, none of these exist: /home/nardor/.config/rustic/rustic.toml, /etc/rustic/rustic.toml, ./rustic.toml
[INFO] repository local:test-repo: password is correct.
[INFO] using cache at /home/nardor/.cache/rustic/3797e1fe324e97ac068e303e672da3d34af09cd73888bcfd09538cd72692bf7d
[00:00:00] reading index...               ████████████████████████████████████████          2/2                               
[00:00:00] getting snapshot...            ████████████████████████████████████████          0                                
[INFO] getting snapshot...
[00:00:00] getting snapshot...            ████████████████████████████████████████          0                                 
-    "README.md"
M    "bar"
+    "new_dir"
+    "new_dir/new_file"
-    "services"
-    "services/b2.toml"
-    "services/rclone_ovh-hot-cold.toml"
-    "services/s3_aws.toml"
-    "services/s3_idrive.toml"
-    "services/sftp.toml"
-    "services/sftp_hetzner_sbox.toml"
-    "services/webdav_owncloud_nextcloud.toml"
Files:	1 new,	8 removed, 	1 changed
Dirs:	1 new,	1 removed
Others:	0 new,	0 removed

Testing

I am not sure what tests I can add.
Please let me know if you think of any test about this.

Thanks in advance for any feedback.

Copy link
Member

@aawsome aawsome left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I would only request small changes:

  • maybe name "file changed" into "content changed"
  • and either add more detailed "change types" for the other changes which are currently collected under "file changed"
  • or add a single "other changes" to collect all those other changes.

src/commands/diff.rs Outdated Show resolved Hide resolved
src/commands/diff.rs Outdated Show resolved Hide resolved
src/commands/diff.rs Outdated Show resolved Hide resolved
@aawsome
Copy link
Member

aawsome commented Jul 29, 2024

@nardoor Sorry for the late review. About tests, in rustic we only have integration tests (and very few) so you could try to generate some testdata in a tar-files (or just one tar and modify files by hand), run the command and check the output (maybe using the insta crate). But this is pretty complicated as these tests may be OS-specific. In rustic we are not that strict requesting unit tests, so far.

@nardoor
Copy link
Contributor Author

nardoor commented Jul 31, 2024

Hello @aawsome,

Thanks for taking the time to review the proposed changes.
I'll gladly apply the review comments asap.

About tests, I might try to add an integration test about this feature, but if I get stuck on some OS specifics as you warned me, I might leave it (for later?).

@nardoor
Copy link
Contributor Author

nardoor commented Aug 5, 2024

with new types added:

❯ ./target/debug/rustic --log-level debug -r test-repo --password password diff 8e57051d:config ./config  --metadata
[INFO] using no config file, none of these exist: /home/nardor/.config/rustic/rustic.toml, /etc/rustic/rustic.toml, ./rustic.toml
[INFO] repository local:test-repo: password is correct.
[INFO] using cache at /home/nardor/.cache/rustic/3797e1fe324e97ac068e303e672da3d34af09cd73888bcfd09538cd72692bf7d
[00:00:00] reading index...               ████████████████████████████████████████          7/7                                                                                              [00:00:00] getting snapshot...            ████████████████████████████████████████          0                                                                                                [INFO] getting snapshot...
U    "README.md"
-    "bar"
T    "full.toml"
+    "full.toml/coco"
+    "lnsymlink"
U    "local.toml"
+    "new_dir"
-    "services/b2.toml"
-    "services/rclone_ovh-hot-cold.toml"
-    "services/s3_aws.toml"
-    "services/s3_idrive.toml"
-    "services/sftp.toml"
-    "services/sftp_hetzner_sbox.toml"
-    "services/webdav_owncloud_nextcloud.toml"
Files   :	1 new,	8 removed,	0 changed
Symlinks:	1 new,	0 removed,	0 changed
Dirs    :	1 new,	0 removed
NodeType:	1 changed
Metadata:	2 changed

@aawsome aawsome enabled auto-merge August 10, 2024 07:47
Copy link
Member

@aawsome aawsome left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks again @nardoor

I removed the unused "use" statement.

@nardoor
Copy link
Contributor Author

nardoor commented Aug 10, 2024

Thanks for finishing the work and looking at my PR!

@aawsome aawsome added this pull request to the merge queue Aug 10, 2024
Merged via the queue into rustic-rs:main with commit 1c9969c Aug 10, 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.

diff: show statistic
2 participants