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

feat: handle config.root is not exist #4616

Merged
merged 2 commits into from
Jan 29, 2021

Conversation

jiangtj
Copy link
Member

@jiangtj jiangtj commented Jan 20, 2021

What does it do?

See this description about url and root, https://github.com/hexojs/hexo-starter/blob/3d337683d271aa28e5bebd31dd35ba12a7a2be82/_config.yml#L15

We can know that url and root are closely related, so why do we configure it twice? root can be generated by url

# You just need to configure it
url: https://hexo.io/foo
# Not required, optional
# root: /foo/

How to test

git clone -b BRANCH https://github.com/USER/hexo.git
cd hexo
npm install
npm test

Screenshots

Pull request tasks

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

@coveralls
Copy link

coveralls commented Jan 20, 2021

Coverage Status

Coverage increased (+0.002%) to 98.307% when pulling 81118ce on jiangtj:remove-config-root into 30f34f5 on hexojs:master.

@@ -26,6 +26,10 @@ module.exports = async ctx => {
ctx.log.debug('Config loaded: %s', magenta(tildify(configPath)));

ctx.config = deepMerge(ctx.config, config);
// If root is not exist, create it by config.url
if (!config.root) {
ctx.config.root = new URL(ctx.config.url).pathname + '/';
Copy link
Member

Choose a reason for hiding this comment

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

How about

let { pathname } = new URL(ctx.config.url);
if (!pathname.endsWith('/')) pathname += '/';
ctx.config.root = pathname;

@stevenjoezhang stevenjoezhang requested a review from a team January 26, 2021 14:55
@jiangtj jiangtj merged commit a62417b into hexojs:master Jan 29, 2021
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