provide exit code for integration with other processes #7
Conversation
|
Hi, sorry I haven't had a chance to look at this. I'll look at this soon! |
|
Hi @Mukimov, I just pushed a big change to |
lib/server.js
Outdated
| // the directories contain multiple configs (I think this is also a bug | ||
| // in stylelint) | ||
| console.info(`Changing process directory to ${folder}`); | ||
| process.chdir(folder); |
There was a problem hiding this comment.
I removed this part. You can check it with 'vanilla' stylelint output, it is different with stylelint_d output. I think it should be the same.
There was a problem hiding this comment.
I don't understand, do you mean the file glob or the console.info?
There was a problem hiding this comment.
I mean process.chdir
Here is "vanilla" stylelint project/styles/button.scss output:
project/styles/components/button.scss
248:3 × Expected height to come before margin-right order/properties-alphabetical-order
And stylelint_d project/styles/button.scss output:
button.scss
248:3 × Expected height to come before margin-right order/properties-alphabetical-order
File path is not the same as in stylelint output.
There was a problem hiding this comment.
Oh I see. Good point, but I think it's outside the scope of this PR, so I made a separate issue: #8.
lib/server.js
Outdated
| @@ -173,8 +173,7 @@ function connHandler(conn) { | |||
| if (formatter === 'string') { | |||
| // If the formatter is a string, we just send the raw output | |||
| // from stylelint, since it's formatted for us already. | |||
| result = data.output; | |||
| write(conn, result); | |||
| write(conn, JSON.stringify(data)); | |||
There was a problem hiding this comment.
I'd feel more comfortable if we didn't use JSON.stringify on the plain data, since if the resulting data is too large it could cause issues for JSON.stringify, and rather sent split up like we do below, that way we get around any issues JSON.stringify may have with this data.
| @@ -53,7 +53,7 @@ if (args.stdin) { | |||
|
|
|||
| function lint(args) { | |||
| var command = args._[0]; | |||
| var format = args.formatter; | |||
| var format = args.formatter || 'string'; | |||
There was a problem hiding this comment.
We don't want to assume a format here since the format assumption is taken care of in server.js.
There was a problem hiding this comment.
But default format in stylelint is string, So usually people wants string output, not json.
There was a problem hiding this comment.
Ah sorry, that was ambiguous. The default formatter, which is string, is handled in server.js, not in stylelint_d.js. For now, this is okay.
bin/stylelint_d.js
Outdated
| }); | ||
|
|
||
| if (result.some(function(d) { return d.errored; })) { | ||
| process.exitCode = 1; |
There was a problem hiding this comment.
It looks like stylelint returns specific error codes for specific reasons, so we should try to follow those as well:
1: Something unknown went wrong.
2: At least one rule with an "error"-level severity triggered at least one warning.
78: There was some problem with the configuration file.
80: A file glob was passed, but it found no files.
|
Hi @Mukimov, I'm really sorry for not replying back after your changes. A few things have changed since April when the last commit here was, and I'm planning to get this added in the next week or so. I will send a message here when it's complete so you can test it out and review it. I'll also see what I can do about assigning credit in the commit(s) so that you are attributed properly as a contributor to the repository. |
|
I will merge the other PR in one week if I don't hear anything. |
No description provided.