path: update win32 toNamespacedPath to support device namespace paths

[injunchoi98]

Updated the win32 toNamespacedPath function to support device namespace paths as per #54180 and #54161. url.pathToFileURL was not modified since device namespace paths are not valid file URIs.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 85.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 88.23%. Comparing base (880c446) to head (9eb494a). Report is 477 commits behind head on main.

Files with missing lines	Patch %	Lines
src/path.cc	57.14%	6 Missing and 3 partials :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54367      +/-   ##
==========================================
+ Coverage   87.09%   88.23%   +1.14%     
==========================================
  Files         648      651       +3     
  Lines      182216   183895    +1679     
  Branches    34965    35847     +882     
==========================================
+ Hits       158704   162267    +3563     
+ Misses      16785    14913    -1872     
+ Partials     6727     6715      -12     

Files with missing lines	Coverage Δ
lib/path.js	95.61% <100.00%> (+0.44%)	:arrow_up:
src/path.cc	67.39% <57.14%> (-1.66%)	:arrow_down:

... and 248 files with indirect coverage changes

[targos]

I believe this is already being fixed in https://github.com/nodejs/node/pull/54224

[injunchoi98]

Thank you for let me know sir. I'll close this PR

[injunchoi98]

sir Im bit confused that #54224 seems to be focused on fixing an issue where a trailing backslash was incorrectly added in the resolve function, whereas my PR is specifically aimed at converting file paths to namespace paths in the tonamespaced function. It seems these are addressing different concerns..?

[huseyinacacak-janea]

I would work on this issue after finishing my current work. So, I reviewed this PR.

This PR fixes a different problem than https://github.com/nodejs/node/pull/54224 and LGTM.

I think the author can continue working on this by reopening it.

[huseyinacacak-janea]

I believe this PR could fix https://github.com/nodejs/node/issues/54161 as well. The conout$ and conin$ could be added as regular expressions. What do you think? Refs: https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file Refs: https://learn.microsoft.com/en-us/windows/console/console-handles

[injunchoi98]

Thank you for your valuable feedback. I initially included only CON as a regular expression, but I completely agree that conout$ and conin$ should also be added as regular expressions. I will reopen this PR tomorrow. I appreciate your guidance.

[targos]

Note that toNamespacedPath also has a C++ implementation that would have to be updated as well.

[injunchoi98]

Thank you for let me know. I just updated C++ implementation as well.

[injunchoi98]

@targos I made the changes based on your feedback. Could you review this PR when you have a moment?

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[daeyeon]

Could you update the doc also?

[injunchoi98]

Of course. Thank you for let me know. I updated the doc also according to your feedback.

[joyeecheung]

Hi, is the original issue still relevant? I was in the process of backporting https://github.com/nodejs/node/pull/52135 which seems to have caused some regressions and it seems this is fixing one of the them that are still around?

[injunchoi98]

The original issue (the lack of support for the device namespace in toNamespace on Windows) still seems unresolved. However, if #52135 modified the C++ implementation of toNamespace, it seems likely that the C++ portion of this PR would also need adjustments.