+ command like arg: --config-file [./config.json]#613
Uh oh!
There was an error while loading. Please reload this page.
Conversation
| start.action((opts) =>{ | ||
| let argv = extend({}, opts,{version: program.version() }) | ||
| let configFile = argv['configFile'] || './config.json' |
There was a problem hiding this comment.
Might want to use path.join here with __dirname is that seems to be consistent with much of the code base
There was a problem hiding this comment.
I'm happy to do that; was just making a minimal change to master. Does that buy us anything over just a relative path to config.json? I made pretty much the reverse change in shex.js when I made it able to load examples from a URL I suspect the only change is for node-solid-server is in debugging output.
There was a problem hiding this comment.
No, looks good, happy to merge!
melvincarvalho commented Oct 19, 2017
Generally, looks good! I think also solves an issue raised in |
dmitrizagidulin commented Oct 20, 2017
re relative path - we constantly run into trouble with relative paths with people who install solid server globally (npm i -g) |
melvincarvalho commented Oct 20, 2017
@dmitrizagidulin good point! However the defaults for data and db are already ./data and ./.db respectively, right? So ./config as a default seems consistent, for a quick change. If I got that right. We could perhaps cleanup the config a bit more later, e.g. in line with #436, alphabetizing the help etc. |
ericprud commented Oct 22, 2017
Not necessarily critical path for this PR, but what is the right behavior for servers installed with |
timbl left a comment
timbl left a comment There was a problem hiding this comment.
Looks good to me. (I think to not allow relative paths will occasionally drive people (like me) crazy, too. )
This added arg allows you to override "
./config.json". It's kinda lame in that it doesn't actually use the default 'cause config.json is read beforerequire('commander').parse(process.argv)is run so you have to include the default a la: