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

Path.format doesn't concat .name and .ext #6437

Closed
eight04 opened this issue Apr 28, 2016 · 7 comments
Closed

Path.format doesn't concat .name and .ext #6437

eight04 opened this issue Apr 28, 2016 · 7 comments
Labels
path Issues and PRs related to the path subsystem.

Comments

@eight04
Copy link

eight04 commented Apr 28, 2016

  • Version: v4.4.3
  • Platform: Windows 7 64-bit
  • Subsystem: path

https://nodejs.org/docs/v4.4.3/api/path.html#path_path_format_pathobject

Under path.format, the document said that:

If the base property is not supplied, a concatenation of the name property and the ext property will be used as the base property.

However, This feature seems not working properly. With following code:

var path = require("path");

var file = path.format({
    root : "C:\\",
    dir : "C:\\path\\dir",
    ext : ".txt",
    name : "file"
});

console.log(file);

I always get:

C:\path\dir\
@mscdex mscdex added the path Issues and PRs related to the path subsystem. label Apr 28, 2016
@mscdex
Copy link
Contributor

mscdex commented Apr 28, 2016

It works for me in v5.10.1 and v6.0.0. Looks like something may need to be backported?

@nwoltman
Copy link
Contributor

nwoltman commented Apr 29, 2016

For some reason the docs were backported but the code wasn't.

The PR that introduced that functionality is #2408. That PR has the semver-major label so that's why it was never backported. Also, after @mscdex re-wrote the path module, a lot of commits were never backported. Those are being tracked in this issue.

So either something needs to be backported or the doc changes need to be reverted in the v4.x branch.

@jasnell
Copy link
Member

jasnell commented Apr 29, 2016

Odd... the doc change may have been picked up along with some other doc updates and just overlooked. The right thing to do would be to revert the doc change in the v4.x.

@MylesBorins
Copy link
Contributor

MylesBorins commented Apr 29, 2016 via email

@nwoltman
Copy link
Contributor

I think these are all of them: daf3ef6, 0943001 (contains other changes), 79d26ae, aed22d0

bf1fe46 also changes the path.format docs and was made after 0943001 but it should be kept since it has nothing to do with the changed functionality.

@MylesBorins
Copy link
Contributor

I have submitted a change to revert the docs. Thanks for reporting @eight04

@imyller
Copy link
Member

imyller commented Sep 16, 2016

Closing this issue as resolved with PR #6530

@imyller imyller closed this as completed Sep 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
path Issues and PRs related to the path subsystem.
Projects
None yet
Development

No branches or pull requests

6 participants