Linter's html field support #455
Conversation
|
@noseglid, could you, please, help me with this PR? It looks like the build and all tests are good but Travis is stalling for no obvious (at least for me) reason. Have you ever encountered such a problem in this project before? Or did I break something? |
| @@ -31,13 +31,15 @@ class Linter { | |||
| } | |||
| this.linter.setMessages(messages.map(match => ({ | |||
| type: match.type || 'Error', | |||
| text: match.message || 'Error from build', | |||
| text: !match.message && !match.html_message ? 'Error from build' : match.message, | |||
| html: match.html_message, | |||
There was a problem hiding this comment.
If message and html_message is set, it will set both to the linter which is an error.
There was a problem hiding this comment.
Yes, you're right. Here I saw two options: either we check it on our side and throw an error or let Linter make all the checks and throw errors. Now, other checks are made by Linter (for instance, that filePath is non-empty) so I decided to leave it this way. Although, if you think it must be the builder's responsibility to check the duplication, I'll add it. I think both attitudes have pros and cons and I have no strict preference other than being consistent in this question.
In case of duplication Linter throws the following error: 'Got both html and text fields on Linter Response, expecting only one'
There was a problem hiding this comment.
And, of course, there's a third option - just ignore either message or html_message when both are set. It will reduce quantity of errors seen by the end user, but it will also hide problems in the provider code. Maybe it's better to show an error and let the provider developer know that he or she did something wrong.
Some enhancement would be to ignore message if it equals to html_message and only use the latter. I think html_message must be preferred because in case of html-tags absence it behaves the same way as message.
What do you think?
There was a problem hiding this comment.
I don't think we should invoke the linter API incorrectly. Since message is the most used I think it should have precedence. So if both are set, don't send html_message to linter. Does that sound okay to you?
There was a problem hiding this comment.
Sure, that sounds reasonable as long as it is documented. I'll make the necessary changes.
|
I think it just stalled a little bit on travis's end. I restarted it and it passed. Sometimes weird stuff happens :) |
|
Done with making |
|
Sorry for taking so long to finish this one off. Summer, y'know :) This is a great PR. Thanks for the work you've put into this! |
|
No worries at all. And thanks for merging! I hope build providers will make good use of the feature and come up with some cool stuff. |
Added support for the Linter's message
htmlfield viahtml_message.fixes #428