Skip to content

feat: joinPath() - #230

Open
curbengh wants to merge 1 commit into
hexojs:masterfrom
curbengh:join-path
Open

feat: joinPath()#230
curbengh wants to merge 1 commit into
hexojs:masterfrom
curbengh:join-path

Conversation

@curbengh

@curbengh curbengh commented Aug 16, 2020

Copy link
Copy Markdown
Contributor

ref: hexojs/hexo#4479

this enhances path.join() to convert slash, but this joinPath() doesn't resolve '../' yet.

Comment thread README.md
Comment on lines +352 to +353
/* If the joined path string is a zero-length string, then '.' will be returned,
representing the current working directory. */

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Similar behaviour as path.join()

@coveralls

coveralls commented Aug 16, 2020

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.05%) to 97.476% when pulling c1295d6 on curbengh:join-path into bf42b66 on hexojs:master.

@jiangtj

jiangtj commented Aug 17, 2020

Copy link
Copy Markdown
Member

but resolving '../' is not supported

path.resolve() is work?

@curbengh

curbengh commented Aug 17, 2020

Copy link
Copy Markdown
Contributor Author

path.resolve() is work?

I mean this joinPath() doesn't support resolving path yet, perhaps in future if there is a demand, currently it's out of scope.

for now, need to joinPath(resolve(from, to), pathtwo).


fixed unit test, ready for review.

@curbengh
curbengh requested a review from a team August 17, 2020 08:32
@SukkaW

SukkaW commented Sep 7, 2020

Copy link
Copy Markdown
Member

@curbengh

Recently I have found a package on npm: https://npm.im/url-join

Have a look?

@stevenjoezhang

Copy link
Copy Markdown
Member

See also hexojs/hexo#5457
I believe there are many more pieces of code that could utilize this joinPath function.

@uiolee

uiolee commented Apr 14, 2024

Copy link
Copy Markdown
Contributor

I think it should be implemented based on path or any other module instead of string concatenation.

@stevenjoezhang

Copy link
Copy Markdown
Member

Yes, maybe path.posix.join will work for such cases.
Moreover, some other languages have more comprehensive libraries for path manipulation, rather than just simple string operations. I particularly like Rust's PathBuf, which is very elegant. I'm not sure if Node.js has a similar implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants