Add Severity#color
#311
Add Severity#color
#311
Conversation
| def color : Colorize::Color | ||
| case self | ||
| in Error then Colorize::ColorANSI::Red | ||
| in Warning then Colorize::ColorANSI::Red |
There was a problem hiding this comment.
what about yellow for warning?
There was a problem hiding this comment.
That was also my choice at the beginning, and by this we come to the interesting topic - do our severities are adequate? I mean - we have just a single rule with :error severity, and the rest of the rules is using either of the remaining two. So, in majority of the cases we gonna have kinda-Ukrainian flag on the CLI (yellow/blue) :P not that it bothers me, but in my eyes what Credo does looks way more adequate:
Refactoring this + the output was going to be (one of) my next PRs actually ;)
There was a problem hiding this comment.
Okay, that makes sense. My latest suggestion is to do Red for Error and LightRed for Warning. If you still think that's a bad idea, let's follow current implementation.
There was a problem hiding this comment.
Yeah, I still do ;)
- light red looks kinda odd
- we'd use the weaker of the two for majority of the issues
- we'd break BC even more
- refactoring is on the way anyway
There was a problem hiding this comment.
Just a small observation: I think ameba shouldn't have an error severity. Everything that's clearly an actual error should be rejected by the compiler. A linter doesn't need to handle that (except for automatic fixing maybe?).
So Rule::Lint::Syntax is the only error severity rule because it catches syntax errors from the parser. I would expect that to stay the only one for said reason.
This PR aims to improve the DX by colouring issue messages with unique color assigned to issue severity.