-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor Colorize::Mode
enum
#11663
Refactor Colorize::Mode
enum
#11663
Conversation
Co-Authored-By: r00ster91 <r00ster91@protonmail.com>
Co-authored-by: Sijawusz Pur Rahnama <sija@sija.pl>
Co-authored-by: Caspian Baska <email@caspian.computer>
end | ||
|
||
private def each_code(mode : Colorize::Mode) | ||
yield '1' if mode.bold? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know the result would be the same, but for readability I wish it was explicitly covering all enum members.
yield '1' if mode.bold? || mode.bright?
Maybe compiler could be (is) smart enough to make it noop, but humans would see all cases covered.
Or maybe just comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the compiler can easily optimize this.
I'll add a comment to the member definition. That's probably more useful than here in this internal implementation.
Extracted from #7690
This is mostly an internal refactoring. Only effect on the API is that invalid symbols are now a compile time error instead of runtime error. That could be considered a breaking change, but it's an improvement for error detection.