Skip to content

fix(permalink): use permalink for parse filename if parsing with new_post_name fails #5660

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

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

Conversation

yoshinorin
Copy link
Member

What does it do?

refs: #5658

In this PR, if parsing fails in parseFilename function with new_post_name, parsing will be attempted using permalink instead.

I've marked this PR as a draft for the following reasons:

  • While all existing tests pass, additional test cases are likely needed.
  • This change may affect existing users, so we should consider alternative solutions that avoid breaking changes — such as introducing a new setting.

Description

I noticed that new_post_name in _config.yml is used for generating the slug. As a result, the post.permalink variable seems to be getting an unexpected value.

Example

For example:

  • Set new_post_name to :year/:month/:day/:title.md
  • Set permalink in _config.yml to :title.html

With this configuration, the post.permalink for source/_posts/foo bar/index.md becomes http://example.com/yyyy/mm/dd/foo-bar-. I think, this is likely not the intended value.

Cause

The issue arises because the parseFilename function uses new_post_name to parse the filename, and when that fails, it returns the article title as the result. This result is then used as the slug.

From what I can see, this behavior has been there since the early implementation, though I don't think it was intentional.

Others

Also, there's no way for users to realize that the new_post_name setting affects the slug. Based on its name, this setting should be used only for post creation, not for anything else. In the future, this value should not be used outside of post creation. However, making this change might inevitably affect existing users...

Screenshots

N/A

Pull request tasks

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

Copy link

How to test

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

@coveralls
Copy link

Pull Request Test Coverage Report for Build 14759827738

Details

  • 15 of 15 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 99.526%

Totals Coverage Status
Change from base Build 14660279550: 0.0%
Covered Lines: 9869
Relevant Lines: 9916

💛 - Coveralls

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.

2 participants