From 0642856b8f4428ca8fc9ac95ec644433fd5f0259 Mon Sep 17 00:00:00 2001 From: VILJkid Date: Fri, 21 Jun 2024 21:45:31 +0530 Subject: [PATCH] (fix) : code refactor and cleanup --- all_test.go | 3 +- copy.go | 95 ++++++++++++++++++++++++++++++----------------------- options.go | 3 +- 3 files changed, 58 insertions(+), 43 deletions(-) diff --git a/all_test.go b/all_test.go index a701f49..0e7bce4 100644 --- a/all_test.go +++ b/all_test.go @@ -461,7 +461,8 @@ func TestOptions_RenameDestination(t *testing.T) { // Your own logic here if strings.HasSuffix(s, ".template") { return strings.TrimSuffix(d, ".template"), nil - } else if strings.HasSuffix(s, ".tpl") { + } + if strings.HasSuffix(s, ".tpl") { return strings.TrimSuffix(d, ".tpl"), nil } return d, nil // otherwise do nothing diff --git a/copy.go b/copy.go index f9787cd..c9d16ad 100644 --- a/copy.go +++ b/copy.go @@ -86,18 +86,12 @@ func copyNextOrSkip(src, dest string, info os.FileInfo, opt Options) error { // with considering existence of parent directory // and file permission. func fcopy(src, dest string, info os.FileInfo, opt Options) (err error) { - - var readcloser io.ReadCloser - if opt.FS != nil { - readcloser, err = opt.FS.Open(src) - } else { - readcloser, err = os.Open(src) - } + readcloser, err := fopen(src, opt) if err != nil { - if os.IsNotExist(err) { - return nil + if !os.IsNotExist(err) { + return } - return + return nil } defer fclose(readcloser, &err) @@ -155,14 +149,23 @@ func fcopy(src, dest string, info os.FileInfo, opt Options) (err error) { return } +// fopen opens the named file, +// and returns it regardless of its type +// fs.File or *os.File +func fopen(src string, opt Options) (io.ReadCloser, error) { + if opt.FS != nil { + return opt.FS.Open(src) + } + return os.Open(src) +} + // dcopy is for a directory, // with scanning contents inside the directory // and pass everything to "copy" recursively. func dcopy(srcdir, destdir string, info os.FileInfo, opt Options) (err error) { - if skip, err := onDirExists(opt, srcdir, destdir); err != nil { - return err - } else if skip { - return nil + skip, err := onDirExists(opt, srcdir, destdir) + if err != nil || skip { + return } // Make dest dir with 0755 so that everything writable. @@ -172,20 +175,9 @@ func dcopy(srcdir, destdir string, info os.FileInfo, opt Options) (err error) { } defer chmodfunc(&err) - var entries []fs.DirEntry - if opt.FS != nil { - entries, err = fs.ReadDir(opt.FS, srcdir) - if err != nil { - return err - } - } else { - entries, err = os.ReadDir(srcdir) - if err != nil { - if os.IsNotExist(err) { - return nil - } - return err - } + entries, err := dread(srcdir, opt.FS) + if err != nil || entries == nil { + return } contents := make([]fs.FileInfo, 0, len(entries)) @@ -197,16 +189,12 @@ func dcopy(srcdir, destdir string, info os.FileInfo, opt Options) (err error) { contents = append(contents, info) } - if yes, err := shouldCopyDirectoryConcurrent(opt, srcdir, destdir); err != nil { - return err - } else if yes { - if err := dcopyConcurrent(srcdir, destdir, contents, opt); err != nil { - return err - } - } else { - if err := dcopySequential(srcdir, destdir, contents, opt); err != nil { - return err - } + shouldCopyConcurrent, err := shouldCopyDirectoryConcurrent(opt, srcdir, destdir) + if err != nil { + return + } + if err = chooseAndPerformCopyMethod(shouldCopyConcurrent, srcdir, destdir, contents, opt); err != nil { + return } if opt.PreserveTimes { @@ -224,6 +212,30 @@ func dcopy(srcdir, destdir string, info os.FileInfo, opt Options) (err error) { return } +// dread reads the named directory, +// it regardless if it's a filesystem +// and returns list of directory entries. +func dread(srcdir string, fsys fs.FS) ([]fs.DirEntry, error) { + if fsys != nil { + return fs.ReadDir(fsys, srcdir) + } + entries, err := os.ReadDir(srcdir) + if err != nil { + if !os.IsNotExist(err) { + return nil, err + } + return nil, nil + } + return entries, nil +} + +func chooseAndPerformCopyMethod(shouldCopyConcurrent bool, srcdir, destdir string, contents []fs.FileInfo, opt Options) error { + if shouldCopyConcurrent { + return dcopyConcurrent(srcdir, destdir, contents, opt) + } + return dcopySequential(srcdir, destdir, contents, opt) +} + func dcopySequential(srcdir, destdir string, contents []os.FileInfo, opt Options) error { for _, content := range contents { cs, cd := filepath.Join(srcdir, content.Name()), filepath.Join(destdir, content.Name()) @@ -262,7 +274,10 @@ func dcopyConcurrent(srcdir, destdir string, contents []os.FileInfo, opt Options func onDirExists(opt Options, srcdir, destdir string) (bool, error) { _, err := os.Stat(destdir) - if err == nil && opt.OnDirExists != nil && destdir != opt.intent.dest { + if err != nil && !os.IsNotExist(err) { + return true, err // Unwelcome error type...! + } + if opt.OnDirExists != nil && destdir != opt.intent.dest { switch opt.OnDirExists(srcdir, destdir) { case Replace: if err := os.RemoveAll(destdir); err != nil { @@ -271,8 +286,6 @@ func onDirExists(opt Options, srcdir, destdir string) (bool, error) { case Untouchable: return true, nil } // case "Merge" is default behaviour. Go through. - } else if err != nil && !os.IsNotExist(err) { - return true, err // Unwelcome error type...! } return false, nil } diff --git a/options.go b/options.go index 2ebbb1f..3c8a6ec 100644 --- a/options.go +++ b/options.go @@ -155,7 +155,8 @@ func assureOptions(src, dest string, opts ...Options) Options { } if opts[0].AddPermission > 0 { opts[0].PermissionControl = AddPermission(opts[0].AddPermission) - } else if opts[0].PermissionControl == nil { + } + if opts[0].PermissionControl == nil { opts[0].PermissionControl = PreservePermission } opts[0].intent.src = defopt.intent.src