sindresorhus / del

Delete files and directories
MIT License
1.32k stars 65 forks source link

delete nothing with file name like "[app+1]xxx" #70

Open bi-kai opened 6 years ago

bi-kai commented 6 years ago

delete nothing with file name like: [app+1]OMStarPS_omstarps_12.3.0_5b866543ab78483cf07c4012.zip

if change file name to: app_1_OMStarPS_omstarps_12.3.0_5b866653ab78483cf07c401b.zip could be delete correctly.

can you gave some advise?

nijynot commented 5 years ago

Had a similar problem. Has something to do with the parsing of the square brackets in filename I think.

deadcoder0904 commented 5 years ago

can confirm. this indeed does not work.

on further investigation, I found out that this has to do with https://github.com/isaacs/minimatch/issues/50

minimatch is a dependency of glob which is a dependency of globby which is a dependency of del

TL;DR

del
└── globby
    └── glob
        └── minimatch

It might work if we escape [] with double-slash \\ like \\[\\].

Unfortunately, I couldn't find where the code change happens :(

kevva commented 5 years ago

minimatch is a dependency of glob which is a dependency of globby which is a dependency of del

Newer versions of globby doesn't use glob, but fast-glob and the micromatch package. However, I can still reproduce the issue. The culprit seems to be using the + sign inside square brackets.

jonschlinkert commented 5 years ago

It might work if we escape [] with double-slash \\ like \\[\\].

Yes, try escaping special characters, like: \\[app\\+1\\]. You might be able to just escape the +.

However, it seems like this might be a bug. IMHO it seems like that should match without having to escape anything. I'll look into it in micromatch, since this might be confusing for some users when a file path is passed, versus a glob pattern. Beyond that... here are a few potential solutions that come to mind in the meantime, or in addition to patching micromatch:

  1. Have an option for explicitly disabling glob matching when literal file path is passed. Something like del(path, { glob: false }).
  2. Update is-glob, to return false when the string has brackets, but no other glob patterns outside the brackets. Or add an option for this.
  3. Update micromatch to do the same.

If someone wants to create an issue on micromatch that would be great. Otherwise I'll create one as soon as I have a chance.

kevva commented 5 years ago

@jonschlinkert, I tried escaping and found that it actually doesn't work at all, at least using \\. However, using \ "works" or has no impact, I couldn't tell. Nothing matched the path with + (or any other quantifier) inside the brackets.

foo
└── foo [bar]
    └── [foo].js
    [bar].js
    [unicorn+1].js
    foo+1.js
// Unescaped
await globby([
    'foo/[bar].js',
    'foo/foo [bar]/[foo].js', 
    'foo/[unicorn+1].js',
    'foo/foo+1.js'
]);
//=> ['foo/[bar].js', 'foo/foo [bar]/[foo].js', 'foo/foo+1.js']

// Single slash
await globby([
    'foo/\[bar\].js',
    'foo/foo \[bar\]/\[foo\].js',
    'foo/\[unicorn\+1\].js',
    'foo/foo+1.js'
]);
//=> ['foo/[bar].js', 'foo/foo [bar]/[foo].js', 'foo/foo+1.js']

// Double slash 
await globby([
    'foo/\\[bar\\].js',
    'foo/foo \\[bar\\]/\\[foo\\].js',
    'foo/\\[unicorn\\+1\\].js',
    'foo/foo+1.js'
]);
//=> ['foo/foo+1.js']

This might be related https://github.com/sindresorhus/globby/issues/81.

jonschlinkert commented 5 years ago

This might be related sindresorhus/globby#81.

Got it, thanks for investigating. I'll look into it a bit more as well.