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

Wildcards not supported in v4 anymore #249

Closed
sven-n opened this issue Jan 14, 2023 · 16 comments
Closed

Wildcards not supported in v4 anymore #249

sven-n opened this issue Jan 14, 2023 · 16 comments

Comments

@sven-n
Copy link

sven-n commented Jan 14, 2023

Hello,
Unfortunately, the recent update broke one of my builds.
I run rimraf in a C# project file with the following line, which should delete all markdown files beginning with C:

<Exec Command="npx rimraf ../../../docs/Packets/C*.md"></Exec>

Was this change intended?
If yes, is my solution correct by specifying the version in the command like this?:

<Exec Command="npx rimraf@3.0.2 ../../../docs/Packets/C*.md"></Exec>

Thanks

@isaacs
Copy link
Owner

isaacs commented Jan 14, 2023

Yes, removing globs was intentional, as stated in the readme and changelog. You can specify the version as rimraf@3 to avoid the semver major jump.

@isaacs isaacs closed this as completed Jan 14, 2023
@RoenLie
Copy link

RoenLie commented Jan 16, 2023

What was the reason for removing glob support?
If it was because of dependency and security concerns, will there be glob support in the future built into the package?

@nicojs
Copy link

nicojs commented Jan 17, 2023

I understand that it is communicated in the changelog and readme, but I would appreciate a motivation or workaround.

I use rimraf exclusively for my cleanup tasks because it is the only way for me to make sure the globbing works on Windows as well as Linux.

@isaacs
Copy link
Owner

isaacs commented Jan 17, 2023

If it was because of dependency and security concerns, will there be glob support in the future built into the package?

I'll add it to the bin in an upcoming minor version, perhaps restricted only to Windows systems since posix shells expand globs automatically, but having it automatically glob in the JS module opened the door for way too many complications and surprising bugs.

I'm not concerned about the dependency or security, since I maintain node-glob, but most people really don't fully grok glob patterns, and find it very surprising when for example rimraf('a[bc]d') deletes 'abd' and 'acd' but not 'a[bc]d'. Having to escape special characters is less surprising on a shell, since those are globs anyway, but even there, it's weird to escape the globs for the shell, and then have to escape them again for the binary itself.

Another idea I'd had is to have a glob bin, so you could put glob rimraf "*" and have it expand that way. A glob bin could be useful for other scripts, but of course that's a little more of a hassle if you only want to rm stuff.

@ghiscoding
Copy link

ghiscoding commented Jan 18, 2023

It might also be useful to change the error message, it took me some time to realize the glob patterns were removed on windows and the error message wasn't helping at all

> rimraf packages/*/dist

Error: Illegal characters in path.
    at pathArg (C:\GitHub\lerna-lite\node_modules\.pnpm\rimraf@4.1.1\node_modules\rimraf\dist\cjs\src\path-arg.js:46:33)
    at X:\GitHub\lerna-lite\node_modules\.pnpm\rimraf@4.1.1\node_modules\rimraf\dist\cjs\src\index.js:39:66
    at Array.map (<anonymous>)
    ....

So in my case I was able to fix the issue by just getting rid of the /* (which I didn't need) and that fixed it, but it would have been more helpful if the error message would say Illegal characters in path. Note that Globs are not supported

@isaacs
Copy link
Owner

isaacs commented Jan 19, 2023

Ok, decided, I'll bring back globs when glob v9 lands. isaacs/node-glob#489

@isaacs isaacs reopened this Jan 19, 2023
@kristian
Copy link

Thanks a lot @isaacs, listening to the community, especially in open source projects can be hard sometimes, as there are so many opinions to consider. Great to hear that a future version of rimraf will again support globs. Maybe two questions:

a) Will you consider releasing the glob-enabled future version again as a major upgrade, because if so, I will now go for a workaround (as described in #251) for version 4., as soon as 5. is released with glob support re-added I will remove my workaround again. A major bump would help to clearly "signal" that glob support was re-added and the workaround can be removed.

b) Do you think the glob support could be interface compatible with version 3 of the library? Including passing glob options into rimraf as { glob: ... }? Thanks!

@isaacs
Copy link
Owner

isaacs commented Jan 19, 2023

a) it'll be a new feature, so a semver minor version bump. 4.2 or whatever. Unless some breaking change is required, which I don't anticipate. But the default npm ^ dependency will pick up the right version if you install at that point.

b) the glob options will likely be different, as it's a semver major glob update, so it likely won't be entirely interface compatible with v3. In any event, I'm not going to bring back the callback api, so it will be a break from v3 for that reason anyway. But the bin behavior and the general flavor will be pretty close, I expect.

@kristian
Copy link

Thanks @isaacs, so I know what to expect. Very helpful. Have a great rest of your day.

@bombillazo
Copy link

thanks for considering reincorporating glob! It's a very useful feature.

@isaacs
Copy link
Owner

isaacs commented Feb 27, 2023

Glob v9 published, going to get on this soon.

This was referenced Feb 27, 2023
@isaacs isaacs closed this as completed in f768f26 Mar 3, 2023
@isaacs
Copy link
Owner

isaacs commented Mar 3, 2023

Landed on 4.2.0

@ghiscoding
Copy link

ghiscoding commented Mar 3, 2023

@isaacs just to confirm, it looks like we now need to use a new flag --glob or else an error is thrown (as shown below)

> rimraf packages/**/*.tgz
Error: Illegal characters in path.

however with --glob, it's all good

> rimraf --glob packages/**/*.tgz
 *  Terminal will be reused by tasks, press any key to close it.

That's the new direction to opt-in via --glob, I assume it's for perf reasons, is that it?

@kai-dorschner-twinsity
Copy link

kai-dorschner-twinsity commented Mar 3, 2023

changelog 420, I giggled

@isaacs
Copy link
Owner

isaacs commented Mar 3, 2023

That's the new direction to opt-in via --glob, I assume it's for perf reasons, is that it?

Yes perf, but also to keep it from being a breaking change from 4.1

@spasecookee
Copy link

That's the new direction to opt-in via --glob, I assume it's for perf reasons, is that it?

Yes perf, but also to keep it from being a breaking change from 4.1

This is probably a dumb question, but how would one use the --glob option with rimraf in a gulpfile?
in the gulpfile, you use rimraf('pathWithGlob/**', { 'glob': true });

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

9 participants