-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
doc: enhance glob pattern documentation in fs.md #58988
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
base: main
Are you sure you want to change the base?
Conversation
2bc0ba6 to
bc710d9
Compare
iamstarkov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you, looks good to me
should it be mentioned for path.matchesGlob(path, pattern) as well?
|
I’ve added a note referencing |
bc710d9 to
45a5d32
Compare
|
and path.matchesGlob() does not support exclude? |
|
@mizulu @jasnell @lpinca |
Clarify that path.matchesGlob() only performs pattern matching, and does not support the exclude functionality available in fs.glob methods. This addresses potential user confusion as noted in code review feedback. Refs: nodejs#58988
|
@mag123c , I think so one can for example implement globing on an in memory file system ( list of paths ) This is out side the scope of this PR I might open an issue for feature request to improve on that. edit: #59015 |
doc/api/fs.md
Outdated
| for await (const entry of glob('test/**/*.+(spec|test).js')) | ||
| console.log(entry); // Test files ending with .spec.js or .test.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will match foo.spectest.js, bar.testtesttesttestspecttest.js, etc.
Seems like a bad example of this pattern type?
doc/api/fs.md
Outdated
| * `callback` {Function} | ||
| * `err` {Error} | ||
| * `matches` {string\[]|Dirent\[]} An array of paths or Dirent objects that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are returned paths relative to options.cwd or absolute?
Are they normalized to the conventions of the current platform or always using / as the directory separator, consistent with how globs are specified?
Add note about +(spec|test) pattern matching behavior and provide more precise alternative using brace expansion. Also clarify that returned paths use forward slashes and are relative to cwd. Refs: nodejs#58988
|
@dmichon-msft
|
more precise alternative using brace expansion. Also clarify that returned paths use forward slashes and are relative to cwd. Refs: nodejs#58988
236358a to
da0b8e0
Compare
Add note about +(spec|test) pattern matching behavior and provide more precise alternative using brace expansion. Also clarify that returned paths use platform-specific separators. Refs: nodejs#58988
da0b8e0 to
cf75289
Compare
|
This PR unfortunately seems to have stalled somewhat? Found it looking for more docs on |
e2d9fb9 to
4cc26f6
Compare
|
@louwers Thanks for the review! I've addressed all your feedback. |
d66b457 to
26df356
Compare
26df356 to
92fb0eb
Compare
92fb0eb to
3756192
Compare
This PR improves the documentation for
fs.glob,fs.globSync, andfsPromises.globby adding comprehensive pattern syntax explanations and practical examples.In the current docs, the pattern parameter is mentioned but not explained clearly, making it difficult for developers to understand what glob patterns are supported and how to use them effectively.
The updated docs now include:
*,?,[abc]), globstar (**), brace expansion ({a,b,c}), andextended glob patterns (
+(pattern),!(pattern))This change resolves #58981