Closed KostkaBrukowa closed 2 years ago
Hi, thanks for the PR! Just a few minor things:
Please use imperative mood for commit titles without the branch name, for ex. just
Unescape HTML entities in inspection description
instead of
escape-entities | Adds unescaping html entities
The StringUtil.unescapeXmlEntities
call is not necessary if the text is blank, so I would prefer moving it like this:
return if (text.isNullOrBlank()) " " else StringUtil.unescapeXmlEntities(addMissingPeriod(text))
If you want, I can make the changes and force-push into your branch before I merge, or you can do it yourself; let me know.
I'm also interested in what's the standard for inspection descriptions, I didn't consider the possibility of them having HTML entities. I'll investigate how IDEA renders them to make sure the solution is consistent with that.
Looks like IDEA renders inspection descriptions as HTML, so this solution looks fine but does not cover every case.
StringUtil.unescapeXmlEntities
does not unescape entities with numeric codes except for '
, but I'd say that's fine until someone finds an inspection that uses other numeric codes.
Once this is merged in, I will also strip HTML tags before the entities are unescaped, that should be enough.
Thanks for review! I've changed the line and changed the commit name
Some of the
eslint
errors are formatted as an html code and some of special characters are encoded as html entities.StringUtil.unescapeXmlEntities
converts entities to normal characters.Before (see this
"
):After: