holmgr / cargo-sweep

A cargo subcommand for cleaning up unused build files generated by Cargo
MIT License
694 stars 31 forks source link

Make default output less noisy #81

Closed marcospb19 closed 1 year ago

marcospb19 commented 1 year ago

Fixes #74.

cargo-sweep logs every deleted file, this makes the output very noisy and hard to follow (outputs approximately a thousand lines per cleaned project).

This PR changes the default output to omit deleted files, and only show them with the --verbose flag.

marcospb19 commented 1 year ago

PR is a draft because this is the remaining output after the changes:

[INFO] Cleaned 5.34 GiB
[INFO] Using all installed toolchains: ["stable-2022-09-22-x86_64-unknown-linux-
gnu", "stable-x86_64-unknown-linux-gnu", "nightly-2022-06-20-x86_64-unknown-linu
x-gnu", "nightly-2022-08-06-x86_64-unknown-linux-gnu", "nightly-x86_64-unknown-l
inux-gnu", "1.63.0-x86_64-unknown-linux-gnu", "1.64.0-x86_64-unknown-linux-gnu"]

So now you don't know what project was cleaned.

marcospb19 commented 1 year ago

The list of installed toolchains is shown for every project it sweeps, but I think it is always the same.

So we should probably just show it once at the start.

This is the function that loads the toolchains:

https://github.com/holmgr/cargo-sweep/blob/75414ca5dfd138691031c6f701fbaa350fd5b095/src/fingerprint.rs#L373-L374

It's always constant, because it calls the rustup binary, I think we can assume the binary doesn't change (also, cargo sweep nevers cd into other directory).

marcospb19 commented 1 year ago

Should be ready for review now, fixing the list of toolchains being outputted repeatedly will require a little bit of refactoring, so I prefer doing it in another PR.

marcospb19 commented 1 year ago

Sorry I had to force push again, cargo clippy --fix broke a test.

jyn514 commented 1 year ago

I just fixed the clippy warnings in #80 (and made a couple other changes that should make the errors easier to read) - do you mind rebasing over master?

Also, please revert the changes to the assertion message, like the error says they don't do what you expect in 2018 edition and I would prefer to switch the edition in a separate PR (edit: done https://github.com/holmgr/cargo-sweep/pull/83).

marcospb19 commented 1 year ago

Oh ok, I'll do that, I was having a lot of trouble with the assertion message, so I force pushed like 5 times, let me fix it.

jyn514 commented 1 year ago

Do you mind adding a test showing the new output? Comparing run(sweep(&["--maxsize", "0"])).get_output().stdout to some string would be ideal; I guess it's annoying because it hashes in the name but maybe you can normalize those?

marcospb19 commented 1 year ago

Now it's failing because of Windows paths

"C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\.tmpBWHDFH\\debug\\.fingerprint\\
marcospb19 commented 1 year ago

Fixed, if you want I can squash the last (one or two) commits.

jyn514 commented 1 year ago

Thanks for sticking with this!