Skip to content

doc: Change doc on fs.rename api w.r.t to windows behaviour #22014

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

doc: Change doc on fs.rename api w.r.t to windows behaviour #22014

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions doc/api/fs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2697,10 +2697,19 @@ changes:
* `callback` {Function}
* `err` {Error}

Asynchronously rename file at `oldPath` to the pathname provided
Asynchronously rename file or directory at `oldPath` to the pathname provided
as `newPath`. In the case that `newPath` already exists, it will
be overwritten. No arguments other than a possible exception are
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Jul 28, 2018

Choose a reason for hiding this comment

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

Is the deletion of the last sentence intended?

given to the completion callback.
be overwritten. `fs.rename` can throw the below error:

- On **Linux** when renaming directory exists and is NOT empty, it throws a
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Aug 21, 2018

Choose a reason for hiding this comment

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

  1. Not sure if renaming is a right form here and below. Let's see what native speakers think.
  2. a `ENOTEMPTY` - > an `ENOTEMPTY` (as well as below).

Copy link
Contributor

@sam-github sam-github Nov 20, 2018

Choose a reason for hiding this comment

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

Its not clear or grammatical, call it after the name of the arg: "if newPath exists"... and likewise in other places.

`ENOTEMPTY` error.
- On **Unix** when renaming directory exists and is NOT empty, it throws a
`ENOENT` error.
- On **Windows** regardless of renaming directory is empty, it throws a
`EPERM` error.

No arguments other than a possible exception (as mentioned above) are given to
Copy link
Contributor

@sam-github sam-github Nov 20, 2018

Choose a reason for hiding this comment

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

This just repeats line 2698 in a more complex written form, and it adds confusion. It says an exception is given to the completion callback, but it is an error that is given. Also, more errors than the few documented can occur, and the sentence reads to me as saying that only the errors documented above can be passed. Better, I think, to remove the sentence.

Copy link
Contributor Author

@antsmartian antsmartian Nov 20, 2018

Choose a reason for hiding this comment

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

@sam-github Make sense.

the completion callback.

See also: rename(2).

Expand Down