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

ERROR when trying to use two alternative configs #2518

Closed
teggr opened this Issue Apr 17, 2017 · 9 comments

Comments

2 participants
@teggr

teggr commented Apr 17, 2017

Environment Info

Windows 10 using both CMD and Git Bash

Node version(node -v): v6.9.1

Hexo and Plugin version(npm ls --depth 0):

+-- hexo@3.3.1
+-- hexo-deployer-sftp@0.1.0
+-- hexo-generator-archive@0.1.4
+-- hexo-generator-category@0.1.3
+-- hexo-generator-index@0.2.1
+-- hexo-generator-tag@0.2.0
+-- hexo-renderer-ejs@0.2.0
+-- hexo-renderer-marked@0.2.11
+-- hexo-renderer-stylus@0.3.3
`-- hexo-server@0.2.0

For BUG

When trying to use the list of alternative configs as documented here: https://hexo.io/docs/configuration.html#Using-an-Alternate-Config, I'm unable to use a list of configs.

I am trying to do this in order to keep my sftp credentials outside the _config.yml which will be visible on GitHub

Running this is ok
hexo config

Running this is ok
hexo config --config _config.yml

Running this is ok
hexo config --config deploy_config.yml

Running with a list outputs an error
hexo config --config _config.yml,deply_config
21:15:11.652 ERROR Local hexo not found in C:\projects\archive\robinteggdotcom-old
11:15:11.654 ERROR Try running: 'npm install hexo --save'

I have tried the npm install command but this does not make any difference

Any help would be much appreciated :)

@NoahDragon NoahDragon added the bug label Apr 17, 2017

@NoahDragon

This comment has been minimized.

Member

NoahDragon commented Apr 17, 2017

When the config file doesn't find, it will throw this error message. I think we need to prompt more clear messages.

@teggr

This comment has been minimized.

teggr commented Apr 18, 2017

I've run the node-inspector on the code and the code appears to be caused by the multi_config_path module being called before the logger has been setup and it then throws an exception.

In the file: https://github.com/hexojs/hexo/blob/master/lib/hexo/index.js
Line 57: https://github.com/hexojs/hexo/blob/master/lib/hexo/index.js#L57

  var multiConfigPath = require('./multi_config_path')(this);
  this.config_path = args.config ? multiConfigPath(base, args.config)
                                 : pathFn.join(base, '_config.yml');
...
  this.log = logger(this.env);

Because the logger is not initialised before the multi_config_path module, then the logger is not in the ctx var.
In the file: https://github.com/hexojs/hexo/blob/master/lib/hexo/multi_config_path.js
Line 66: https://github.com/hexojs/hexo/blob/master/lib/hexo/multi_config_path.js#L66

var log = ctx.log;
...
log.i('Config based on', count, 'files');

@teggr

This comment has been minimized.

teggr commented Apr 18, 2017

Potentially you could initialise the multi_config__path after the logger unless there is another reason not to?

@teggr

This comment has been minimized.

teggr commented Apr 18, 2017

Maybe related to #2499

@NoahDragon

This comment has been minimized.

Member

NoahDragon commented Apr 18, 2017

You are right. I did a quick test, the issue will be resolved if moved the multi_config block after the logger initialization.

I will create a PR shortly.

@coveralls coveralls referenced this issue Apr 18, 2017

Merged

Fix multiple config issue #2518 #2520

2 of 2 tasks complete
@NoahDragon

This comment has been minimized.

Member

NoahDragon commented Apr 19, 2017

PR created #2520

@teggr

This comment has been minimized.

teggr commented Apr 19, 2017

That change looks to be fine. I will test later. Thanks :)

@teggr

This comment has been minimized.

teggr commented Apr 20, 2017

PR #2520 works great locally 👍

NoahDragon added a commit that referenced this issue Apr 20, 2017

Fix multiple config issue #2518 (#2520)
* Update index.js

* Update index.js

* Create _config.json

* Update hexo.js
@NoahDragon

This comment has been minimized.

Member

NoahDragon commented Apr 20, 2017

Thank you. Great, I merged the changes into main repo.

@NoahDragon NoahDragon closed this Apr 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment