-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
Replace manual color codes with sopel.formatting #1516
Conversation
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 present to you, @dgw's traditional nitpick review.
If you're not put off by the idea of getting really anal about whether whitespace is included in formatted spans or not, there are some spots in the reddit module that you could fix up, too. 😉
if u.is_mod: | ||
message = message + ' | 05Mod' | ||
message = message + ' | ' + bold(color('Mod', colors.GREEN)) |
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.
This is technically changing the color, not just replacing it with formatting functions.
That said… green is the color Reddit uses for displaying moderators' names, so… Consider this a start on the devilish festivities for v6.6.6, since we're changing behavior (however slightly) in a patch release. 😛
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.
Correct on all counts.
It does change behavior, but if this breaks something... whoever built something that parses sopel's output was doing very silly things anyway, and the changes to spacing and colon bolding would also likely break it.
b36cadb
to
844321f
Compare
Cleaner, and makes source display work correctly in editors that don't like non-printing characters.