Throw on bad config file #228
Conversation
markdownlint.js
Outdated
| @@ -68,7 +68,7 @@ function readConfiguration(userConfigFile) { | |||
| markdownlint.readConfigSync(userConfigFile, configFileParsers); | |||
| config = require('deep-extend')(config, userConfig); | |||
| } catch (error) { | |||
| console.warn('Cannot read or parse config file ' + userConfigFile + ': ' + error.message); | |||
| throw 'Cannot read or parse config file ' + userConfigFile + ': ' + error.message; | |||
There was a problem hiding this comment.
Please add tests to cover the new behavior. I'm thinking one for missing file and one for a invalid file?
Also, why not a change more like: https://github.com/igorshubovych/markdownlint-cli/blob/master/markdownlint.js#L190-L193? I'm not sure I see where the throw gets turned into an error.
There was a problem hiding this comment.
The thing I don't like about letting the process run by setting an exitCode but not calling exit() is it accumulates further output to stdout/err. If you then write tests, you get more than the minimal output. E.g.
[Expected: 1; Actual: 2]
- `Cannot read or parse config file 'non-existent-file-path.yaml': ENOENT: no such file or directory, open 'non-existent-file-path.yaml'␊
- md043-config.md:13 MD012/no-multiple-blanks Multiple consecutive blank lines␊I'll use it of course to keep consistency. Didn't see the similar case on L190-L193 before.
There was a problem hiding this comment.
Good point, future refactoring... :)
…ests for both cases
Closes #105.