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.

Sign up for GitHub

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

Jump to bottom

fs: allow 'withFileTypes' to be used with globs #52837

Merged
merged 8 commits into from
May 7, 2024
Merged

fs: allow 'withFileTypes' to be used with globs #52837

merged 8 commits into from
May 7, 2024

Conversation

RedYetiDev
Copy link
Member

Fixes #52752

This PR enables the usage of withFileTypes in fs.glob (and similar) functions.

When withFileTypes is provided, the exclude() function will receive Dirent objects as arguments, along with the return value and callbacks.

@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels May 5, 2024
MoLow
MoLow reviewed May 5, 2024
View reviewed changes
lib/internal/fs/glob.js Outdated Show resolved Hide resolved
MoLow
MoLow approved these changes May 5, 2024
View reviewed changes
Copy link
Member

@MoLow MoLow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

aduh95
aduh95 reviewed May 5, 2024
View reviewed changes
lib/internal/fs/glob.js Outdated Show resolved Hide resolved
benjamingr
benjamingr approved these changes May 5, 2024
View reviewed changes
Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good work

RedYetiDev and others added 2 commits May 5, 2024 11:39
@RedYetiDev @aduh95
Update glob.js
b70ecc4 
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@RedYetiDev @aduh95
Update test-fs-glob.mjs
0bd1e1b 
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@RedYetiDev
Copy link
Member Author

Thanks for the reviews! I'll fix the lint later today :-)

@benjamingr benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label May 5, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 5, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58961/

targos
targos reviewed May 5, 2024
View reviewed changes
doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
doc/api/fs.md Outdated Show resolved Hide resolved
RedYetiDev and others added 3 commits May 5, 2024 14:26
@RedYetiDev @targos
Update doc/api/fs.md
75041e2 
Co-authored-by: Michaël Zasso <targos@protonmail.com>
@RedYetiDev @targos
Update doc/api/fs.md
bd621ca 
Co-authored-by: Michaël Zasso <targos@protonmail.com>
@RedYetiDev @targos
Update doc/api/fs.md
8d2e905 
Co-authored-by: Michaël Zasso <targos@protonmail.com>
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58965/

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels May 5, 2024
@targos
Copy link
Member

targos commented May 6, 2024

That's a warning and can be ignored.

The actual error is fatal error C1060: compiler is out of heap space. This requires to Resume CI (I just did that)

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58973/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58979/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58984/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59005/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59008/

@MoLow MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label May 7, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 7, 2024
@nodejs-github-bot nodejs-github-bot merged commit 303c80c into nodejs:main May 7, 2024
59 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 303c80c

@RedYetiDev RedYetiDev deleted the withfiletypes-glob branch May 7, 2024 11:17
Ch3nYuY pushed a commit to Ch3nYuY/node that referenced this pull request May 8, 2024
@RedYetiDev @Ch3nYuY
fs: allow 'withFileTypes' to be used with globs
2d81222 
PR-URL: nodejs#52837
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
targos pushed a commit that referenced this pull request May 8, 2024
@RedYetiDev @targos
fs: allow 'withFileTypes' to be used with globs
7d7a762 
PR-URL: #52837
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
@targos targos added the semver-minor PRs that contain new features and should be released in the next minor version. label May 13, 2024
@targos
Copy link
Member

targos commented May 13, 2024

This PR adds a feature so it should be labeled semver-minor PRs that contain new features and should be released in the next minor version. (this is not supposed to be done by releasers).

targos added a commit that referenced this pull request May 13, 2024
@targos
2024-05-14, Version 22.2.0 (Current)
7d92816 
Notable changes:

cli:
  * (SEMVER-MINOR) allow running wasm in limited vmem with
    --disable-wasm-trap-handler (Joyee Cheung)
    #52766
doc:
  * add pimterry to collaborators (Tim Perry)
    #52874
fs:
  * (SEMVER-MINOR) allow 'withFileTypes' to be used with globs
    (Aviv Keller) #52837
inspector:
  * (SEMVER-MINOR) introduce the `--inspect-wait` flag (Kohei Ueno)
    #52734
lib,src:
  * remove --experimental-policy (Rafael Gonzaga)
    #52583
perf_hooks:
  * (SEMVER-MINOR) add `deliveryType` and `responseStatus` fields
    (Matthew Aitken) #51589
test_runner:
  * (SEMVER-MINOR) support test plans (Colin Ihrig)
    #52860
zlib:
  * (SEMVER-MINOR) expose zlib.crc32() (Joyee Cheung)
    #52692

PR-URL: #52971
@targos targos mentioned this pull request May 13, 2024
Merged
targos added a commit that referenced this pull request May 15, 2024
@targos
2024-05-15, Version 22.2.0 (Current)
d57af10 
Notable changes:

cli:
  * (SEMVER-MINOR) allow running wasm in limited vmem with
    --disable-wasm-trap-handler (Joyee Cheung)
    #52766
doc:
  * add pimterry to collaborators (Tim Perry)
    #52874
fs:
  * (SEMVER-MINOR) allow 'withFileTypes' to be used with globs
    (Aviv Keller) #52837
inspector:
  * (SEMVER-MINOR) introduce the `--inspect-wait` flag (Kohei Ueno)
    #52734
lib,src:
  * remove --experimental-policy (Rafael Gonzaga)
    #52583
perf_hooks:
  * (SEMVER-MINOR) add `deliveryType` and `responseStatus` fields
    (Matthew Aitken) #51589
test_runner:
  * (SEMVER-MINOR) support test plans (Colin Ihrig)
    #52860
zlib:
  * (SEMVER-MINOR) expose zlib.crc32() (Joyee Cheung)
    #52692

PR-URL: #52971
targos added a commit that referenced this pull request May 15, 2024
@targos
2024-05-15, Version 22.2.0 (Current)
28b0a58 
Notable changes:

cli:
  * (SEMVER-MINOR) allow running wasm in limited vmem with
    --disable-wasm-trap-handler (Joyee Cheung)
    #52766
doc:
  * add pimterry to collaborators (Tim Perry)
    #52874
fs:
  * (SEMVER-MINOR) allow 'withFileTypes' to be used with globs
    (Aviv Keller) #52837
inspector:
  * (SEMVER-MINOR) introduce the `--inspect-wait` flag (Kohei Ueno)
    #52734
lib,src:
  * remove --experimental-policy (Rafael Gonzaga)
    #52583
perf_hooks:
  * (SEMVER-MINOR) add `deliveryType` and `responseStatus` fields
    (Matthew Aitken) #51589
test_runner:
  * (SEMVER-MINOR) support test plans (Colin Ihrig)
    #52860
zlib:
  * (SEMVER-MINOR) expose zlib.crc32() (Joyee Cheung)
    #52692

PR-URL: #52971
eliphazbouye pushed a commit to eliphazbouye/node that referenced this pull request Jun 20, 2024
@RedYetiDev @eliphazbouye
fs: allow 'withFileTypes' to be used with globs
b4eb464 
PR-URL: nodejs#52837
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
eliphazbouye pushed a commit to eliphazbouye/node that referenced this pull request Jun 20, 2024
@targos @eliphazbouye
2024-05-15, Version 22.2.0 (Current)
eccd98f 
Notable changes:

cli:
  * (SEMVER-MINOR) allow running wasm in limited vmem with
    --disable-wasm-trap-handler (Joyee Cheung)
    nodejs#52766
doc:
  * add pimterry to collaborators (Tim Perry)
    nodejs#52874
fs:
  * (SEMVER-MINOR) allow 'withFileTypes' to be used with globs
    (Aviv Keller) nodejs#52837
inspector:
  * (SEMVER-MINOR) introduce the `--inspect-wait` flag (Kohei Ueno)
    nodejs#52734
lib,src:
  * remove --experimental-policy (Rafael Gonzaga)
    nodejs#52583
perf_hooks:
  * (SEMVER-MINOR) add `deliveryType` and `responseStatus` fields
    (Matthew Aitken) nodejs#51589
test_runner:
  * (SEMVER-MINOR) support test plans (Colin Ihrig)
    nodejs#52860
zlib:
  * (SEMVER-MINOR) expose zlib.crc32() (Joyee Cheung)
    nodejs#52692

PR-URL: nodejs#52971
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
@RedYetiDev @bmeck
fs: allow 'withFileTypes' to be used with globs
6acf662 
PR-URL: nodejs#52837
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
@targos @bmeck
2024-05-15, Version 22.2.0 (Current)
99e317e 
Notable changes:

cli:
  * (SEMVER-MINOR) allow running wasm in limited vmem with
    --disable-wasm-trap-handler (Joyee Cheung)
    nodejs#52766
doc:
  * add pimterry to collaborators (Tim Perry)
    nodejs#52874
fs:
  * (SEMVER-MINOR) allow 'withFileTypes' to be used with globs
    (Aviv Keller) nodejs#52837
inspector:
  * (SEMVER-MINOR) introduce the `--inspect-wait` flag (Kohei Ueno)
    nodejs#52734
lib,src:
  * remove --experimental-policy (Rafael Gonzaga)
    nodejs#52583
perf_hooks:
  * (SEMVER-MINOR) add `deliveryType` and `responseStatus` fields
    (Matthew Aitken) nodejs#51589
test_runner:
  * (SEMVER-MINOR) support test plans (Colin Ihrig)
    nodejs#52860
zlib:
  * (SEMVER-MINOR) expose zlib.crc32() (Joyee Cheung)
    nodejs#52692

PR-URL: nodejs#52971
@targos targos added dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. labels Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduh95 aduh95 aduh95 left review comments

@targos targos targos left review comments

@VoltrexKeyva VoltrexKeyva VoltrexKeyva approved these changes

@MoLow MoLow MoLow approved these changes

@benjamingr benjamingr benjamingr approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

fs.glob add option to only include files (not directories) in the results entries
7 participants
@RedYetiDev @nodejs-github-bot @targos @benjamingr @MoLow @aduh95 @VoltrexKeyva