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

doc: clarify that path.isAbsolute on windows accepts / and \ #8291

Closed
wants to merge 2 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Aug 27, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

Clarify that path.isAbsolute() on windows accepts / and \ as path delims

Fixes: #6520

@jasnell jasnell added doc Issues and PRs related to the documentations. path Issues and PRs related to the path subsystem. labels Aug 27, 2016
```

*Note*: On Windows, the `path.isAbsolute()` method will accept both forward
slash (`/`) and backwards slash (`\`) characters as path delimiters.
Copy link
Member

Choose a reason for hiding this comment

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

s/backwards/backward/ ? I'm not a native speaker but "backward slash" has more occurrences on Google.

Copy link
Member

Choose a reason for hiding this comment

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

btw, this sounds a bit overly specific, although I might be wrong… is there anything in the Node API where / isn’t accepted on Windows?

Copy link
Member Author

Choose a reason for hiding this comment

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

@addaleax ... good point...

@jasnell jasnell force-pushed the doc-path-absolute-slash branch from bf100e1 to 08b12f2 Compare August 30, 2016 18:22
@jasnell
Copy link
Member Author

jasnell commented Aug 30, 2016

@lpinca @addaleax ... updated! PTAL!

@lpinca
Copy link
Member

lpinca commented Aug 30, 2016

LGTM.

@addaleax
Copy link
Member

LGTM

jasnell added a commit that referenced this pull request Aug 30, 2016
Fixes: #6520
PR-URL: #8291
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@jasnell
Copy link
Member Author

jasnell commented Aug 30, 2016

Landed in 86067f0. Thanks!

@jasnell jasnell closed this Aug 30, 2016
@Fishrock123 Fishrock123 mentioned this pull request Sep 6, 2016
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
Fixes: nodejs#6520
PR-URL: nodejs#8291
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Fishrock123 pushed a commit that referenced this pull request Sep 9, 2016
Fixes: #6520
PR-URL: #8291
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
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. path Issues and PRs related to the path subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants