TestableIO / System.IO.Abstractions.Extensions

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

Copy Directory recursive #18

Closed mayermart closed 1 year ago

mayermart commented 2 years ago

Hi,

i would like to implement an Extension Method for recursively copy a Directory based on https://docs.microsoft.com/de-de/dotnet/standard/io/how-to-copy-directories

Would it be helpful to create a Pull request and add it to this Package? And if so, should it be implemented as IDirectoryInfo.CopyTo(string destDirectoryName) (as it already exits in FileInfo.CopyTo) or better as IDirectory.CopyDirectory(string sourceDir, string destinationDir, [options]), or both?

Thanks Martin

gigi81 commented 2 years ago

of course @mayermart you are welcome to submit a PR!

This signature makes more sense to me: IDirectoryInfo.CopyTo(string` destDirectoryName);

However, something like this would be better in my opinion: IDirectoryInfo.CopyTo(IDirectoryInfo destination, bool recursive = true);

gigi81 commented 2 years ago

if you submit a PR please include unit test and also update the readme

mayermart commented 2 years ago

For consistency with FileInfo.CopyTo i would stay with a Signature like IDirectoryInfo.CopyTo(string destDirectoryName, bool recursive = false); and returning a IDirectoryInfo for the Destination Directory

So you can use it in two lines like

IDirectoryInfo source = fileSystem.DirectoryInfo.FromDirectoryName(sourceDirectoryName);
IDirectoryInfo dest = source.CopyTo(destDirectoryName, recursive: true);

otherwise it would require three lines:

IDirectoryInfo source = fileSystem.DirectoryInfo.FromDirectoryName(sourceDirectoryName);
IDirectoryInfo dest = fileSystem.DirectoryInfo.FromDirectoryName(destDirectoryName);
source.CopyTo(dest , recursive: true);

But as the first line would be creating the dest IDirectoryInfo anyway, it is easy to create an overload, returning the injected IDirectoryInfo.

Also i would set recursive default to false, as this is the behavior of many other tools (e.g. rm, mv, cp on linux bash).

gigi81 commented 2 years ago

Yeah it all makes sense. Maybe have both overloads so we are both happy :-) but yeah I agree with all the rest.

mayermart commented 1 year ago

@gigi81 just in case there was no notification: I created a PR containing a CopyTo Feature: #19 If you got some time, please take a look :)

gigi81 commented 1 year ago

This is implemented in 2.0.3