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

Error when glob matches directory #10

Open
conradz opened this issue Mar 17, 2015 · 17 comments
Open

Error when glob matches directory #10

conradz opened this issue Mar 17, 2015 · 17 comments

Comments

@conradz
Copy link

conradz commented Mar 17, 2015

Currently it throws an error (EISDIR) when the glob matches a directory. I want to copy all files and directory structure to another directory. I am using cpy(['**/*'], '../output/dist', { cwd: 'assets', parents: true }, cb);. This does not work since **/* also matches the directory names.

I could make a PR to fix this if you know what behavior we should have. Should we just ignore empty directories or create them? I'm currently leaning toward creating any directories, which I think is the most intuitive.

@schnittstabil
Copy link
Collaborator

The options are also passed to node-glob. This should do what you want:

cpy(['**/*'], '../output/dist', { cwd: 'assets', parents: true, nodir: true }, cb);

@schnittstabil
Copy link
Collaborator

Or did you expect, that cpy should also copy (empty) directories? (Atm cpy only copies files.)

@conradz
Copy link
Author

conradz commented Mar 18, 2015

OK, the nodir option does fix my use case (somehow I never saw that option in the list). I think it would be nicest to copy empty directories, but I don't actually need it.

@pgilad
Copy link

pgilad commented Mar 21, 2015

I ran into the same issue. I think cpy(['**/*']) should copy entire directory and preserve empty directories. That means that cp-file should be able to copy a blank directory (without included files).

@schnittstabil
Copy link
Collaborator

@pgilad It would be the easiest to support this feature within cp-file, but that is out of the scope of cp-file: cp-file can be compared with fs.rename, fs.symlink or fs.mkdir.

@schnittstabil
Copy link
Collaborator

@conradz, @pgilad: I think a recursive option would make more sense:

  • cpy(['./**/*'], …): copy all files under .
  • cpy(['.'], …, {recursive: true}, …): copy the whole directory

@sindresorhus, @kevva: What do you think?

@schnittstabil
Copy link
Collaborator

Or, even better:

  • cpy(['./**/*'], …): copy all files and folders under .
  • cpy(['./**/*'], …, {nodir: true }, …): copy only files
  • cpy(['./**/*/'], …): copy the directory structure (w/o files)

@sindresorhus
Copy link
Owner

cpy(['./*//'], …): copy the directory structure (w/o files)

I don't like how a trailing / can completely change the behaviour.

@schnittstabil
Copy link
Collaborator

@sindresorhus: cpy already handle trailing /, via globby, e.g:

> globby.sync('node_modules/globby/*/')
[ 'node_modules/globby/node_modules/' ]

My last suggestion is the same @conradz already suggested, which I hadn't thoroughly thought out:
if cp-file fails because src is a directory, cpy creates the appropriate dest directory.

But I do not know whether this makes sense:

The following would neither copy a single file nor throw an error:

cpy(['node_modules'], 'dist', ) // same as `mkdir dist/node_modules`

Comparing the behavior of cp, a --recursive option seems to offer a way out of this dilemma:

$ cp node_modules dist
cp: omitting directory 'node_modules'
$ echo $?
1

$ cp --recursive node_modules/ dist
$ echo $?
0

But I think it is not worth it. Especially in combination with globbing, --recursive may lead to problems, which are much too complicated to handle:

cpy(['**/node_modules'], , {recursive: true}, )
// apply cp-file by means of eachAsync to:
// 'node_modules',
// 'node_modules/globby/node_modules',
// …

@sindresorhus
Copy link
Owner

As a user it would make sense that cpy(['node_modules'], 'dist', …) actually copied the whole node_modules directory with files into dist.

I wonder how the behaviour of ncp compares.

@helmoski
Copy link

nodir is not currently an option for CLI

chimon2000 referenced this issue in chimon2000/cpy-cli Mar 17, 2016
@schnittstabil
Copy link
Collaborator

But I think it is not worth it. Especially in combination with globbing, --recursive may lead to problems, which are much too complicated to handle…

Things have changed, today cpy and the underlying globby use Promises which simplifies many aspects of this problem.

@sindresorhus Hence, I would like to implement a recursive option for globby and use it within cpy (by default, no option).

@sindresorhus
Copy link
Owner

👍 Go for it :)

@schnittstabil
Copy link
Collaborator

It turns out, that a globby option is rather unattractive (sindresorhus/globby#24)

@sindresorhus If the following behavior is acceptable, then the things will become much easier:

$ tree 
.
└── src
    └── index.js

$ cpy --recursive 'src' '!**/*.js' 'dest'

$ tree 
.
├── dest
│   └── index.js
└── src
    └── index.js

I think that this is reasonable; it's similar to bash and cp: the patterns are evaluated before cp is called.

@sindresorhus
Copy link
Owner

I don't get it. In your example '!**/*.js' is moot. Did you mean to add more nested files to the src folder tree?

@schnittstabil
Copy link
Collaborator

I can think of two ways how negative patterns can have an effect in in conjunction with --recursive:

$ tree 
.
└── src
    └── index.js
    └── index.css
  • Glob first:
cpy --recursive 'src' '!**/*.js' 'dest'
# equals to
cpy --recursive 'src' 'dest'

# => also copies 'src/index.js' to 'dest'
  • --recursive changes globbing:
cpy --recursive 'src' '!**/*.js' 'dest'
# equals to
cpy 'src' 'src/index.css' 'dest'

# => does not copy 'src/index.js' to 'dest'

The first one is similar to bash and cp, but personally I would prefer the second one.

@danielweck
Copy link

Related issue: sindresorhus/cpy-cli#10 (comment)
=> usage of glob **/*.* vs. **/*, typically matches folder names (no filename extension)

schuer added a commit to FriendsOfREDAXO/redaxo-mit-bimmelbam that referenced this issue Mar 14, 2018
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

6 participants