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

feat: joinPath() #230

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: joinPath() #230

wants to merge 1 commit into from

Conversation

curbengh
Copy link
Contributor

@curbengh curbengh commented Aug 16, 2020

ref: hexojs/hexo#4479

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

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
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
Copy link

coveralls commented Aug 16, 2020

Coverage Status

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

@jiangtj
Copy link
Member

jiangtj commented Aug 17, 2020

but resolving '../' is not supported

path.resolve() is work?

@curbengh
Copy link
Contributor Author

curbengh commented Aug 17, 2020

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
Copy link
Member

SukkaW commented Sep 7, 2020

@curbengh

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

Have a look?

@stevenjoezhang
Copy link
Member

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

@uiolee
Copy link
Member

uiolee commented Apr 14, 2024

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

@stevenjoezhang
Copy link
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