hexojs / hexo

A fast, simple & powerful blog framework, powered by Node.js.
https://hexo.io
MIT License
38.83k stars 4.75k forks source link

fix(tag): use url_for #5385

Closed stevenjoezhang closed 2 months ago

stevenjoezhang commented 6 months ago

What does it do?

Issue resolved: https://github.com/hexojs/hexo/issues/5383

Screenshots

Pull request tasks

github-actions[bot] commented 6 months ago

How to test

git clone -b post_link https://github.com/hexojs/hexo.git
cd hexo
npm install
npm test
coveralls commented 6 months ago

Pull Request Test Coverage Report for Build 8611939935

Details


Totals Coverage Status
Change from base Build 8578073587: -0.001%
Covered Lines: 9326
Relevant Lines: 9370

💛 - Coveralls
hx-yinpengfei commented 6 months ago

Help, when will the package be updated?

uiolee commented 6 months ago

fail to pass test on Windows because of backslash (%5c)

log

try: https://github.com/hexojs/hexo/blob/f5e0f92eda9491116c585142c565efcf7bf7a3e3/lib/theme/index.ts#L53

github-actions[bot] commented 2 months ago

How to test

git clone -b post_link https://github.com/hexojs/hexo.git
cd hexo
npm install
npm test
github-actions[bot] commented 2 months ago

flamegraph

https://2841be169799bd071696bb5b92e98498e843c1bf-14-hexo.surge.sh/flamegraph.html https://2841be169799bd071696bb5b92e98498e843c1bf-16-hexo.surge.sh/flamegraph.html https://2841be169799bd071696bb5b92e98498e843c1bf-18-hexo.surge.sh/flamegraph.html

stevenjoezhang commented 2 months ago

@uiolee I have made the modification, and indeed, I haven't found a more elegant solution. In the future, we can consider adding this path replacement inside url_for. The WHATWG URL Standard is capable of handling such paths.

> new URL('\\a\\b\\', 'http://example.com').pathname
'/a/b/'
uiolee commented 2 months ago

Yes. I noticed that hexo uses different styles of paths in different places. Maybe we should use the same style of paths everywhere so we can avoid conversions

stevenjoezhang commented 2 months ago

I think we can introduce a new convention: In the database models used by Hexo (including Asset, Page, Post, etc.), the source attribute is used to specify the file system path under the source directory, which includes backslashes (\) on Windows; whereas the path attribute is used to specify their paths on the web, where backslashes should be replaced by /.

stevenjoezhang commented 2 months ago

Database model Asset also contains an _id attribute (instead of using the UUID automatically assigned by warehouse), which by default is the file's relative path. I have found that when using it, backslashes are replaced, which seems unnecessary. I will try to continue with the refactoring.

See https://github.com/hexojs/hexo/commit/2876faf

stevenjoezhang commented 2 months ago

Another issue is that when calculating relative paths, sometimes substring is used and sometimes relative is used. This inconsistency also needs to be addressed and fixed in the future.