Open sprusr opened 8 years ago
This should probably strip all control codes, not just colors.
\x02
: bold text\x03
: colored text (already handled with arguments, :+1:)\x1D
: italic text\x1F
: underlined text\x16
: swap background and foreground colors ("reverse video")\x0F
: reset all formattingHey @dgw thanks for the suggestion. Initially I thought it wouldn't be necessary to handle these, as they represent characters that won't be displayed, but I suppose it depends on the situation. Will make the changes suggested, thanks again.
If only worrying about characters that are displayed, there's no reason to remove \x03
from colors; just removing the digits would suffice. :wink:
Thanks for updating the PR. I figured (and should have explained initially) that it's best to remove all of the control codes in case of misbehaving fonts, or other weirdness that could arise from having non-printing characters in the notifications. There are a lot of system configurations out there, and as long as we're stripping one kind of control character…
:100::+1:
:+1: However, this probably conflicts #627 as that PR moves code around.
My apologies for the number of commits this is taking for a single regex! I haven't made much contribution to open source projects before, so thank you for being so helpful along the way.
Don't worry about that, @sprusr, we all have to start somewhere :-)
The only thing is that before this gets merged, it'll be good to squash the commits into 1.
Fixes erming/shout#628