search

bnjbvr / cargo-machete

Remove unused Rust dependencies with this one weird trick!
MIT License
784 stars 28 forks source link

Add ignore flag #95

Closed mickvangelderen closed 7 months ago

mickvangelderen commented 1 year ago

Add the ignore flag which make cargo-machete respect .ignore and .gitignore files when searching for files.

I've tried to make minimal changes. I've also tried not to make any breaking changes. The behavior should only change when passing --ignore. No guarantee of correctness.

IgnisDa commented 1 year ago

I feel like this should be the default behavior. Maybe via a breaking change in a major release?

mickvangelderen commented 1 year ago

I'd say that doing so is a separate issue. I wouldn't want to block this capability on the discussion about whether it is the right time to do a major release, and what needs to go in that release. However, this is not up to me. That is up to the maintainer.

mickvangelderen commented 12 months ago

@bnjbvr pinging you in case you are not yet aware of this PR

bnjbvr commented 12 months ago

Howdy! Sorry for not answering earlier, happy to see a PR for this, thanks. It might take a bit of time for me to review, since I'm in the middle of some big personal changes, but j should be able to get to it in the next week or so. In any case if someone from the community wants to review/test etc, feel free to do so!

mickvangelderen commented 12 months ago

I'm in the middle of some big personal changes

Good luck with that!

Can you add two tests please:

  • one where there's an ignored file that's filtered out because the ignore flag is set?

I believe this is covered by these lines: https://github.com/bnjbvr/cargo-machete/pull/95/files#diff-42cb6807ad74b3e201c5a7ca98b911c5fa08380e942be6e4ac5807f8377f87fcR305-R310. If skip_target_dir is false and respect_ignore_files is true, then no manifests are found because /target is in the .gitignore.

  • one where the ignore flag isn't set and machete successfully analyzes a crate that's in a directory that would be ignored if the flag was set?

I believe this is covered by https://github.com/bnjbvr/cargo-machete/pull/95/files#diff-42cb6807ad74b3e201c5a7ca98b911c5fa08380e942be6e4ac5807f8377f87fcR312-R317. If both skip_target_dir and respect_ignore_files are false, then at least one manifest is found.

I can split the test function into multiple test functions. I have not done so because the original already contained two cases. I am happy to refactor things a bit, but I'd like your approval before doing so. The PR will become a bit larger and require more effort to review properly. I can also do a follow-up PR. In fact I have some changes ready here.

bnjbvr commented 8 months ago

(Oops, sorry I've forgotten about this PR, I'll try to get back to it soon™)