Skip to content

docs: explain why path.posix.normalize does not replace windows slashes #12298

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
JestDotty opened this issue Apr 10, 2017 · 10 comments
Closed
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.

Comments

@JestDotty
Copy link

JestDotty commented Apr 10, 2017

  • Version: 4.4.2
  • Platform: Windows 8.1 64 bit
  • Subsystem: node.js native path module

console.log('1', path.posix.normalize("\\some\\thing\\like\\this"))
console.log('2', path.posix.normalize("/some/thing/like/this"))
console.log('3', path.win32.normalize("\\some\\thing\\like\\this"))
console.log('4', path.win32.normalize("/some/thing/like/this"))

output:
image

1 output should be the same as 2s though
Docs need to explain why this is the expected output

@vsemozhetbyt vsemozhetbyt added the path Issues and PRs related to the path subsystem. label Apr 10, 2017
@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Apr 10, 2017

If I get this right, win32 methods can process both separators, while posix methods don't consider \ a valid separator. Compare how other win32 methods parse paths with / properly, while the posix methods do not this for \:

const path = require('path');

const win32ToPosix = '\\win32\\to\\posix';
const posixToWin32 = '/posix/to/win32';

console.log(`
  ${path.posix.basename(win32ToPosix)}
  ${path.win32.basename(posixToWin32)}

  ${path.posix.dirname(win32ToPosix)}
  ${path.win32.dirname(posixToWin32)}

  ${path.posix.normalize(win32ToPosix)}
  ${path.win32.normalize(posixToWin32)}

  ${JSON.stringify(path.posix.parse(win32ToPosix))}
  ${JSON.stringify(path.win32.parse(posixToWin32))}

  ${path.posix.resolve(win32ToPosix)}
  ${path.win32.resolve(posixToWin32)}
`);
c:\>node test.js

  \win32\to\posix
  win32

  .
  /posix/to

  \win32\to\posix
  \posix\to\win32

  {"root":"","dir":"","base":"\\win32\\to\\posix","ext":"","name":"\\win32\\to\\posix"}
  {"root":"/","dir":"/posix/to","base":"win32","ext":"","name":"win32"}

  c:\/\win32\to\posix
  c:\posix\to\win32

c:\>

@bnoordhuis
Copy link
Member

Answered. Closing, working as intended.

@JestDotty
Copy link
Author

JestDotty commented Apr 10, 2017

the docs say:

When multiple, sequential path segment separation characters are found (e.g. / on POSIX and \ on Windows), they are replaced by a single instance of the platform specific path segment separator. Trailing separators are preserved.

which is a little weird so I skimmed it with my understanding, but for postarity this this doesn't work either:

console.log('1', path.posix.normalize("\\\\some\\\\thing\\\\like\\\\this"))
console.log('2', path.posix.normalize("/\\some/\\thing/\\like/\\this"))

image

To be fair it is vague enough to have concluded to my understanding... but on a more conceptual/abstract/module angle a conversion looking like this is intended / not available in a path manipulation library or am I missing something?

path.normalize(file).split(/[\\\/]/g).join(path.posix.sep)

@addaleax
Copy link
Member

@Seudein On POSIX, \\ is not a path separator, so I think path.posix.normalize is right in that /\ shouldn’t get contracted into a single /… what would your expected output for that be?

@refack
Copy link
Contributor

refack commented Apr 10, 2017

Maybe we need a posix2windows / windows2posix methods

@addaleax
Copy link
Member

@refack Sorry, could you explain how those methods would behave?

@refack
Copy link
Contributor

refack commented Apr 10, 2017

@refack Sorry, could you explain how those methods would behave?

posix2windows would assume the input is a posix path and flip the /
(then do some voodoo to try and find on which drive the file exists image [or not])

windows2posix would assume that the input is windows and make sure all \ are turned into /

I think what @Seudein assumed was that using path.posix.* on a windows platform will act like windows2posix


Answered. Closing, working as intended.

@bnoordhuis I believe that if a user open an issue based on wrong assumptions that are not addressed in the docs, it should be treated as a bug in the docs.

@refack refack added doc Issues and PRs related to the documentations. and removed path Issues and PRs related to the path subsystem. labels Apr 10, 2017
@refack refack changed the title path.posix.normalize does not replace windows slashes docs: explain why path.posix.normalize does not replace windows slashes Apr 10, 2017
@refack refack added the good first issue Issues that are suitable for first-time contributors. label Apr 10, 2017
@refack refack reopened this Apr 10, 2017
@refack
Copy link
Contributor

refack commented Apr 10, 2017

Reopened as a doc bug

@addaleax
Copy link
Member

then do some voodoo to try and find on which drive the file exists

path methods generally don’t actually query the file system, I would like to keep it that way.

@refack
Copy link
Contributor

refack commented Apr 10, 2017

I would like to keep it that way.

I agree. Super important!
I was sarcastic, I was hoping the huge emoji and [or not] would convey that, sorry...

@addaleax addaleax closed this as completed May 7, 2017
addaleax pushed a commit that referenced this issue May 7, 2017
Add section to path docs that explains that path.posix.normalize
does not replace Windows slashes with POSIX slashes because POSIX
does not recognize / as a valid path separator.

Fixes: #12298
PR-URL: #12700
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
anchnk pushed a commit to anchnk/node that referenced this issue May 19, 2017
Add section to path docs that explains that path.posix.normalize
does not replace Windows slashes with POSIX slashes because POSIX
does not recognize / as a valid path separator.

Fixes: nodejs#12298
PR-URL: nodejs#12700
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
gibfahn pushed a commit that referenced this issue Jun 20, 2017
Add section to path docs that explains that path.posix.normalize
does not replace Windows slashes with POSIX slashes because POSIX
does not recognize / as a valid path separator.

Fixes: #12298
PR-URL: #12700
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
MylesBorins pushed a commit that referenced this issue Jul 11, 2017
Add section to path docs that explains that path.posix.normalize
does not replace Windows slashes with POSIX slashes because POSIX
does not recognize / as a valid path separator.

Fixes: #12298
PR-URL: #12700
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

No branches or pull requests

5 participants