diff: add support for --targets

[sandeepmistry]

• ❗ I have followed the Contributing to DVC checklist.

• 📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

---

Will close #3388.

The filter code needs to be improved (something smarter than starts with) and I think an error needs to be outputted if the target parameter is not found in the list of tracked files or folders. The documentation will also need to be updated accordingly once this is merged.

For now I've started with the original behaviour the issue started with (optional --target arg). Let me know if the behaviour from comment #3388 (comment) would be preferred (dvc diff [-h] [-q | -v] [-t TARGET] [--show-json] [--show-hash] [--show-md] [--hide-missing] [a_rev] [b_rev] [targets [targets]]).

[sandeepmistry]

@efiop please take a look at the latest changes when you get a chance.

If things look good I will proceed to added tests and updating the documentation.

[pmrowla]

isin() will not cover every case here, I think we need to use path_info.overlaps(...) instead.

Let's say, output is a tracked directory dir/, which contains a file foo. output.path_info here will contain the path to the tracked dir: dir/.

If our target is dir/foo, output.path_info.isin_or_eq(target) will be False. In this case the output dir/ itself should not be yielded (which you have correctly accounted for already), but the file dir/foo within output does need to be yielded (meaning we still have to call _dir_output_paths(...)

[sandeepmistry]

I think this was working before because yield_output was set to no_targets or output.path_info.isin_or_eq(targets).

I've done another pass to try to make this clearer let me know. Also, let me know if there is an additional scenario for this that needs to be covered in the unit test I added.

[pmrowla]

@sandeepmistry Nice progress on this PR! There's still a couple minor issues that I think need to be cleaned up, but for the most part it looks good to me.

[sandeepmistry]

@pmrowla thanks for the another round of suggestions and feedback - I think I've addressed everything in the last round.

I've opened PR for the documentation updates for this as well treeverse/dvc.org#2002.

[jorgeorpinel]

Should this PR be linked to #3388 ?

Also could the [targets] argument be implicit (after the revs) instead of a --targets flag? Per #3388 (comment) and #3388 (comment)

Thanks

[efiop]

Should this PR be linked to #3388 ?

@jorgeorpinel Good point.

Also could the [targets] argument be implicit (after the revs) instead of a --targets flag? Per #3388 (comment) and #3388 (comment)

No, we can't do it right now. --targets is consistent with dvc metrics/plots diff.

[efiop]

Thank you @sandeepmistry ! 🙏