Skip to content

Conversation

mag123c
Copy link

@mag123c mag123c commented Jul 8, 2025

This PR improves the documentation for fs.glob, fs.globSync, and fsPromises.glob by 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:

  • Detailed pattern syntax section covering basic wildcards (*, ?, [abc]), globstar (**), brace expansion ({a,b,c}), and
    extended glob patterns (+(pattern), !(pattern))
  • Comprehensive examples demonstrating recursive searches, multiple patterns, exclusion patterns, and different use cases
  • Platform considerations explaining case sensitivity, path separators, symlink handling, and hidden file behavior
  • Performance notes covering memory efficiency, internal caching, and optimization tips
  • Improved parameter documentation with clearer descriptions and cross-references between API variants

This change resolves #58981

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Jul 8, 2025
@mag123c mag123c force-pushed the improve-fs-glob-docs branch from 2bc0ba6 to bc710d9 Compare July 8, 2025 08:17
Copy link

@iamstarkov iamstarkov left a 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?

@mag123c
Copy link
Author

mag123c commented Jul 8, 2025

I’ve added a note referencing path.matchesGlob() to indicate it uses the same glob pattern syntax.
Let me know if that's sufficient here or if you'd prefer a separate section in path.md for that function instead.

Improved the documentation for fs.glob, fs.globSync, and fsPromises.glob
by adding comprehensive information about the `pattern` parameter.

The update includes:
- Detailed explanations of glob syntax: wildcards (*, ?), character sets,
  globstar (**), brace expansion ({a,b,c}), and extended patterns (+(x), !(x))
- Practical usage examples including recursive patterns, exclusions,
  and multiple patterns
- Platform considerations such as case sensitivity, path separators,
  symlink handling, and hidden file behavior
- Performance notes on caching, memory efficiency, and matching strategies
- Improved parameter descriptions and consistent formatting across variants
- Added a reference to `path.matchesGlob()` which supports the same glob pattern
  syntax for in-memory path matching

Fixes: nodejs#58981
@mag123c mag123c force-pushed the improve-fs-glob-docs branch from bc710d9 to 45a5d32 Compare July 8, 2025 13:46
@mizulu
Copy link

mizulu commented Jul 9, 2025

path.matchesGlob() is nice to have, but how do we use this to mimic the exclude rules
which do not strictly exclude by matching (as noted on the PR code comment)

and path.matchesGlob() does not support exclude?

@mag123c
Copy link
Author

mag123c commented Jul 10, 2025

@mizulu
path.matchesGlob() only performs simple pattern matching; the exclude functionality would need to be implemented separately.

@jasnell @lpinca
I’ve updated the comment to clarify that path.matchesGlob() does not support exclude, as this might be confusing to users.
Would you mind taking another look when you have a moment?

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
@mizulu
Copy link

mizulu commented Jul 10, 2025

@mag123c ,
thanks, for the confirmation.

I think path.matchesGlob should be extended to mimic the glob functionality
to allow users to mimic similar pattern matching as glob

so one can for example implement globing on an in memory file system ( list of paths )
following the same rules as the node glob. using the path.matchesGlob

This is out side the scope of this PR

I might open an issue for feature request to improve on that.

edit: #59015

@mag123c mag123c requested review from jasnell and lpinca July 10, 2025 00:55
Comment on lines +1150 to +1151
for await (const entry of glob('test/**/*.+(spec|test).js'))
console.log(entry); // Test files ending with .spec.js or .test.js

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?

* `callback` {Function}
* `err` {Error}
* `matches` {string\[]|Dirent\[]} An array of paths or Dirent objects that
Copy link

@dmichon-msft dmichon-msft Jul 24, 2025

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?

mag123c added a commit to mag123c/node that referenced this pull request Jul 25, 2025
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
@mag123c
Copy link
Author

mag123c commented Jul 25, 2025

@dmichon-msft
Thank you for the valuable feedback. I've reviewed your comments and updated the documentation accordingly.

  1. Pattern matching: I've added a clarification that the +(spec|test) pattern also matches unintended files like foo.spectest.js. Additionally, I've included an alternative example using brace expansion {spec,test} for more precise matching.

  2. Path format clarification: After examining the implementation and test files, I've corrected the documentation. The returned paths do use platform-specific separators (\ on Windows, / on POSIX), not always forward slashes as previously stated. I've updated all relevant sections to reflect this.

mag123c added a commit to mag123c/node that referenced this pull request Jul 25, 2025
more precise alternative using brace expansion. Also clarify that
returned paths use forward slashes and are relative to cwd.

Refs: nodejs#58988
@mag123c mag123c force-pushed the improve-fs-glob-docs branch from 236358a to da0b8e0 Compare July 25, 2025 02:48
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
@mag123c mag123c force-pushed the improve-fs-glob-docs branch from da0b8e0 to cf75289 Compare July 25, 2025 02:50
Co-authored-by: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs.glob is stable as of v24 but lacks any documentation regarding pattern argument

7 participants