Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposed change to copyTree() to allow pre-existing target directories #65

Closed
ghost opened this issue Nov 13, 2013 · 1 comment
Closed

Comments

@ghost
Copy link

ghost commented Nov 13, 2013

The CommonJS/Fileystem/A spec is vague about various behavior (unless there's standard expected behavior I'm not aware of), but having copyTree() fail because makeDirectory() fails due to an existing target directory, seems unfortunate and inconsistent with the behavior at the file level, which just overwrites existing target files (as long as they're writable).

I propose changing copyTree() to allow for preexisting directories. A less desirable (IMO) alternative would be to leave copyTree() as is and add this as, say, "mergeTree()".

  exports.copyTree = function(source, target) {
    var self = this;
    return Q.when(self.stat(source), function(stat) {
      if (stat.isFile()) {
        return self.copy(source, target);
      } else if (stat.isDirectory()) {
        return self.exists(target).then(function(dstExists) {
          var copySubTree = Q.when(self.list(source), function(list) {
            return Q.all(list.map(function(child) {
              return self.copyTree(self.join(source, child), self.join(target, child));
            }));
          });
          if (dstExists) {
            return copySubTree;
          } else {
            return Q.when(self.makeDirectory(target), function() {
              return copySubTree;
            });
          }
        });
      } else if (stat.isSymbolicLink()) {
        return self.symbolicCopy(source, target);
      }
    });
  }; 
kriskowal added a commit that referenced this issue Nov 19, 2013
Allows copyTree to be applied over existing directory trees.
Fixes #65
@ghost
Copy link
Author

ghost commented Nov 19, 2013

Clearly I failed to link this properly to the pull request. Not sure how. Closing manually as it was assigned to issue #69.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

0 participants