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: general improvements to path.md copy #7122

Closed
wants to merge 5 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jun 3, 2016

Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc (path)

Description of change

General improvements to path.md documentation

@nodejs/documentation

@jasnell jasnell added doc Issues and PRs related to the documentations. path Issues and PRs related to the path subsystem. labels Jun 3, 2016

The file system is not consulted to determine if paths are valid.

## Windows vs. posix
Copy link
Contributor

@mscdex mscdex Jun 3, 2016

Choose a reason for hiding this comment

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

POSIX should probably be capitalized everywhere, except for path.posix code references of course.

@jasnell
Copy link
Member Author

jasnell commented Jun 3, 2016

Updated :-)

* `[, ...]` {String} Additional path segments

The `path.join()` method join all given path segments together using the
platform specific separator (`path.sep`) as a delimiter, then normalizes
Copy link
Contributor

Choose a reason for hiding this comment

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

Referencing path.sep specifically here may be a bit misleading in case someone tries to modify path.sep, since path.sep is not actually used internally by the path functions.

@jasnell
Copy link
Member Author

jasnell commented Jun 3, 2016

Updated :-)

The `path.normalize()` method normalizes a path, resolving `'..'` and `'.'`
segments.

When multiple, sequential `/` (slash) characters are found, they are replaced
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be reworded to not mention specific slash characters and instead use wording like 'platform specific path segment separator' or something shorter (but still differentiates from the platform specific path separator -- ; or :).

@jasnell
Copy link
Member Author

jasnell commented Jun 3, 2016

Done

## path.posix

Provide access to aforementioned `path` methods but always interact in a posix
compatible way.
The `path.posix' property provides access to POSIX specific implementations
Copy link
Contributor

Choose a reason for hiding this comment

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

The ' should be a backtick.

@jasnell
Copy link
Member Author

jasnell commented Jun 3, 2016

Updated!

The resulting path is normalized and trailing slashes are removed unless the
path is resolved to the root directory.

Any `path` segments passed as empty strings are ignored.
Copy link
Contributor

Choose a reason for hiding this comment

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

This sounds a bit odd, what about 'Any empty path strings are ignored.'?

@jasnell
Copy link
Member Author

jasnell commented Jun 6, 2016

Updated

@jasnell
Copy link
Member Author

jasnell commented Jun 9, 2016

ping @mscdex

@@ -2,37 +2,93 @@

Stability: 2 - Stable

This module contains utilities for handling and transforming file
paths. The file system is not consulted to check whether paths are valid.
The `path` module provides utilities for working with file paths. It can be
Copy link
Contributor

@mscdex mscdex Jun 9, 2016

Choose a reason for hiding this comment

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

minor nit: s/file/file/directory/ or s/file/file system/ ? Although the latter makes it sounds like it necessarily touches the file system, which is not always the case.


## path.basename(path[, ext])
<!-- YAML
added: v0.1.25
-->

Return the last portion of a path, similar to the Unix `basename` command.
`path` must be a string. `ext`, if given, must also be a string.
* `path` {String} A file path
Copy link
Contributor

@mscdex mscdex Jun 9, 2016

Choose a reason for hiding this comment

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

s/A file path// here and all of the other instances

@jasnell
Copy link
Member Author

jasnell commented Jun 9, 2016

@mscdex ... updated!

@mscdex
Copy link
Contributor

mscdex commented Jun 11, 2016

LGTM

jasnell added a commit that referenced this pull request Jun 12, 2016
PR-URL: #7122
Reviewed-By: Brian White <mscdex@mscdex.net>
@jasnell
Copy link
Member Author

jasnell commented Jun 12, 2016

Landed in a173483

@jasnell jasnell closed this Jun 12, 2016
evanlucas pushed a commit that referenced this pull request Jun 16, 2016
PR-URL: #7122
Reviewed-By: Brian White <mscdex@mscdex.net>
@evanlucas evanlucas mentioned this pull request Jun 16, 2016
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