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: create post with a path composed of numbers throw an error #5117

Closed
wants to merge 1 commit into from

Conversation

D-Sketon
Copy link
Member

What does it do?

fix #4334

Screenshots

Pull request tasks

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

@github-actions
Copy link

How to test

git clone -b fix_4334 https://github.com/D-Sketon/hexo.git
cd hexo
npm install
npm test

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 98.778% when pulling 2ec8682 on D-Sketon:fix_4334 into 58a8f8c on hexojs:master.

@stevenjoezhang
Copy link
Member

There will be an edge case: --path 0x100 will become string '256', not '0x100'.

You can try to change the code here: https://github.com/hexojs/hexo-cli/blob/5c5fc8fe2fc781a557a30f1bf2043502825f173c/lib/hexo.ts#L18

minimist(process.argv.slice(2), { string: ['_', 'p', 'path'] })

should work

@stevenjoezhang
Copy link
Member

See also hexojs/hexo-cli#200

@D-Sketon
Copy link
Member Author

@stevenjoezhang should slug also be added to this code?
For example: hexo new page --slug 0x404 will generate 1028.md

@stevenjoezhang
Copy link
Member

stevenjoezhang commented Nov 27, 2022

@D-Sketon Yes, the slug should also have a string type

You can add other test cases like #4363

@stevenjoezhang stevenjoezhang mentioned this pull request Nov 27, 2022
2 tasks
@D-Sketon
Copy link
Member Author

D-Sketon commented Nov 27, 2022

@stevenjoezhang I'm a bit confused, do I just need to add extra test cases in this PR?
I have tried to add test cases like #4363, but they all fail. It seems that the tests all skip hexo-cli by using lib\plugins\console\new.js directly

@stevenjoezhang
Copy link
Member

It's tricky to cover this situation with unit tests, need to hook process.argv somehow

@D-Sketon D-Sketon marked this pull request as draft December 5, 2022 09:19
@uiolee uiolee added the javascript Pull requests that update Javascript code label Jan 12, 2024
@D-Sketon
Copy link
Member Author

Superseded by #5465

@D-Sketon D-Sketon closed this Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create post with a path composed of numbers throw an error
4 participants