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

WIP: Allow publish handle files with filenames separated by spaces #5476

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

yapinxxx
Copy link

@yapinxxx yapinxxx commented Apr 21, 2024

Allow the publish command to handle files with filenames separated by spaces in the _draft folder.

What does it do?

The update enables filenames to include spaces in the _draft folder.
However, the source code modifies the original filename(data.slug) and then incorrectly compares it with the filenames in the _draft folder, leading to erroneous results.

Copy link

How to test

git clone -b draft_filename https://github.com/yapinxxx/hexo.git
cd hexo
npm install
npm test

Copy link
Member

@SukkaW SukkaW left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you like to create a test case against your changes?

@yapinxxx
Copy link
Author

Would you like to create a test case against your changes?

If you could assist me in creating a test case, I would greatly appreciate it.


Before submitting my change, I experimented with modifications in the node_modules directory to test my idea.

Once I confirmed the concept could work, I directly edited the TypeScript source code.

However, I faced challenges building the project and comparing changes before and after, so I submitted the pull request without a comparison.

Additionally, creating a test case proved difficult due to my limited expertise in front-end development.
I struggled with building the project and providing a comparison, making it hard to create the test case on my own.

@yapinxxx
Copy link
Author

Would you like to create a test case against your changes?

Hello @SukkaW ,
Could you please provide me with some guidance on creating tests for my changes?
I've already tried using npm test, but I'm not certain if that's what you're referring to.

@SukkaW
Copy link
Member

SukkaW commented May 1, 2024

Would you like to create a test case against your changes?

Hello @SukkaW , Could you please provide me with some guidance on creating tests for my changes? I've already tried using npm test, but I'm not certain if that's what you're referring to.

npm test runs tests against the existing test cases to make sure Hexo's existing behaviors do not change (no regression is introduced in your changes).

You are introducing a new behavior (correct handling of the special filename) that doesn't exist in the current version of Hexo.

You will need to add a test case to make sure that Hexo will always handle the special filename properly. So in the future when we run npm test again, we can make sure Hexo will always handle the special filename correctly.

@yapinxxx yapinxxx force-pushed the draft_filename branch 2 times, most recently from 7e4d138 to 5d31f02 Compare May 7, 2024 15:38
@yapinxxx yapinxxx marked this pull request as draft May 8, 2024 00:19
@yapinxxx yapinxxx changed the title Allow publish handle files with filenames separated by spaces WIP: Allow publish handle files with filenames separated by spaces May 8, 2024
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.

3 participants