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

fix(tag): use url_for #5385

Merged
merged 5 commits into from
Apr 9, 2024
Merged

fix(tag): use url_for #5385

merged 5 commits into from
Apr 9, 2024

Conversation

stevenjoezhang
Copy link
Member

What does it do?

Issue resolved: #5383

Screenshots

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.

This comment was marked as outdated.

@coveralls
Copy link

coveralls commented Dec 22, 2023

Pull Request Test Coverage Report for Build 8611939935

Details

  • 15 of 15 (100.0%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.001%) to 99.53%

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

💛 - Coveralls

@hx-yinpengfei
Copy link

Help, when will the package be updated?

@stevenjoezhang stevenjoezhang marked this pull request as draft December 26, 2023 02:21
@uiolee
Copy link
Member

uiolee commented Dec 28, 2023

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

log

try:

path = path.replace(/\\/g, '/');

Copy link

github-actions bot commented Apr 6, 2024

How to test

git clone -b post_link https://github.com/hexojs/hexo.git
cd hexo
npm install
npm test

@stevenjoezhang stevenjoezhang marked this pull request as ready for review April 6, 2024 12:26
@stevenjoezhang
Copy link
Member Author

@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/'

@stevenjoezhang stevenjoezhang requested a review from a team April 6, 2024 12:30
@uiolee uiolee added this to the 7.2.0 milestone Apr 6, 2024
@uiolee
Copy link
Member

uiolee commented Apr 6, 2024

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

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

stevenjoezhang commented Apr 7, 2024

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 2876faf

@stevenjoezhang
Copy link
Member Author

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.

lib/plugins/tag/post_link.ts Outdated Show resolved Hide resolved
lib/models/post_asset.ts Outdated Show resolved Hide resolved
stevenjoezhang and others added 2 commits April 9, 2024 15:27
Co-authored-by: Uiolee <22849383+uiolee@users.noreply.github.com>
Signed-off-by: Mimi <stevenjoezhang@gmail.com>
Signed-off-by: Mimi <1119186082@qq.com>
@stevenjoezhang stevenjoezhang merged commit 94f6ad3 into master Apr 9, 2024
30 of 31 checks passed
@stevenjoezhang stevenjoezhang deleted the post_link branch April 9, 2024 08:00
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.

_config.yml设置了url如https://username.github.io/project', post_link跳转会丢失project,导致404
4 participants