TestableIO / System.IO.Abstractions.Extensions

Convenience functionality on top of System.IO.Abstractions
MIT License
21 stars 6 forks source link

feat: Improve CopyTo directory extension to allow overwrites #58

Closed alwaysbusy closed 9 months ago

alwaysbusy commented 9 months ago

Using the IDirectoryInfoExtensions.CopyTo method is very useful, however it doesn't support overwriting files. This resulted in a modified copy of this within my own code. It also raised the question of when it fails due to a file already existing the destination directory is left with files partially copied.

This PR implements both passing the overwrite argument, and additional checks to prevent only a partial copy succeeding. If the copy is likely to fail on a first pass to evaluate file existence, then no files are copied. This doesn't account for the slightly trickier case of files being locked or permissions not permitting writing within the directory tree. It didn't seem sensible to implement one without the other.

Hopefully this would allow the function within the library to be even more useful whilst providing a sensible guardrail. My hesitation with this approach is that it would change the behaviour of the function from doing a partial copy to no copy where files already exist in the destination (a behaviour I'd really hope no one relies on). It does also cause a binary compatibility issue to the addition of an optional argument to an existing method (a behaviour I'm much more concerned by, maybe making this warranting of a major version bump).

Sorry for not opening as an issue for discussion first, but I felt the code probably articulated what I was trying to say a bit better.

gigi81 commented 9 months ago

Hi @alwaysbusy thanks for your PR I agree the overwrite option should be available. I added some comments to the code. have a look.

github-actions[bot] commented 7 months ago

This is addressed in release v2.2.1.