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

path: fix toNamespacedPath on Windows #52915

Conversation

huseyinacacak-janea
Copy link
Contributor

There was an inconsistency in the implementation of win32.toNamespacedPath(). It appears that the forward slash / wasn't consistently being converted to a backslash \. This PR fixes this issue by returning resolvedPath in the toNamespacedPath().

I've also added a test case to make sure that there will not be any regression.

Fixes: #30224

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. path Issues and PRs related to the path subsystem. labels May 9, 2024
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label May 9, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 9, 2024
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@avivkeller avivkeller left a comment

Choose a reason for hiding this comment

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

LGTM

Tip

My review, while it shows my support, is not blocking/approving.

@nodejs-github-bot
Copy link
Collaborator

@huseyinacacak-janea
Copy link
Contributor Author

Is there anything else I can do to help this PR move forward?

@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label May 13, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 13, 2024
@nodejs-github-bot nodejs-github-bot merged commit dcd8b82 into nodejs:main May 13, 2024
63 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in dcd8b82

targos pushed a commit that referenced this pull request May 15, 2024
PR-URL: #52915
Fixes: #30224
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024
PR-URL: #52915
Fixes: #30224
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024
PR-URL: #52915
Fixes: #30224
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Jun 17, 2024
PR-URL: #52915
Fixes: #30224
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
sophoniie pushed a commit to sophoniie/node that referenced this pull request Jun 20, 2024
PR-URL: nodejs#52915
Fixes: nodejs#30224
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
PR-URL: nodejs#52915
Fixes: nodejs#30224
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. path Issues and PRs related to the path subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent results for path.toNamespacedPath() for win32 UNC paths
6 participants