370085 - Support ANSI escape codes a la mIRC codes

Status: RESOLVED FIXED

(Other Applications Graveyard :: ChatZilla, enhancement)

Description

[James Ross]

I've only seen this about twice in all my time in #chatzilla, but apparently mIRC, Trillian and irssi do support these, eggdrop bots sometimes send them, and it would probably be good to parse them ourselves.

An ANSI escape sequence in IRC is identified by the character 0x1B followed by the standard ANSI sequence ("[", then the control operations, then, for our usage, "m"). E.g.

[\0x1B][0m      <-- reset all
[\0x1B][1m      <-- turn bold on
[\0x1B][22m     <-- turn bold off

(Full list at http://en.wikipedia.org/wiki/ANSI_escape_code#graphics)

Comment 1

[James Ross]

Attachment: Simplistic patch

This maps the ANSI colours to mIRC ones, which isn't exactly right; however, I'd much rather we changed the mIRC colours to be exactly the ANSI ones. Also adds italic, overline and strikethrough - the mess in the CSS file is because the 3 lines (over, through, under) are all one CSS property and don't merge (yay, go CSS Working Group), so we have to explicitly declare each combination.

Comment 2

[James Ross]

Comment on attachment 266395 [details] [diff] [review]
Simplistic patch

What the heck, the colours here are close enough (WP says they're not even standardised for ANSI, so it can join the not-standardised mIRC club).

Comment 3

[:Gijs (he/him)]

Comment on attachment 266395 [details] [diff] [review]
Simplistic patch

Sorry this took so long. Theses suck. The patch generally looks OK, but I've inlined some comments below:

>Index: xul/content/mungers.js
>@@ -576,12 +578,15 @@ function mircResetColor (text, container
>     delete data.currFgColor;
>     delete data.currBgColor;
>     delete data.isBold;
>+    delete data.isItalic;
>+    delete data.isOverline;
>+    delete data.isStrikethrough;
>     delete data.isUnderline;
>     delete data.hasColorInfo;

Can we do this some cleverer way? Like returning a new data object with the bits we do want to keep, which seems like almost nothing? :-)

>@@ -602,12 +607,147 @@ function mircReverseColor (text, contain
>         data.currBgColor = tempColor;
>     else
>         delete data.currBgColor;
>     data.hasColorInfo = true;
> }
> 
>+function ansiEscapeSGR(text, containerTag, data)
>+{
>+    if (!client.enableColors ||
>+        (("noANSIColors" in data) && data.noANSIColors) ||
This occurs nowhere else in the patch. How would it be set, and do we need it?

>+        (("noStateChange" in data) && data.noStateChange))
This is used elsewhere in the code, but I can't find evidence of it being set. What am I missing? Is this only here for plugin support or the like?

> <snip>
>+            case 0: // Reset/normal.
>+                delete data.currFgColor;
>+                delete data.currBgColor;
>+                delete data.isBold;
>+                delete data.isItalic;
>+                delete data.isOverline;
>+                delete data.isStrikethrough;
>+                delete data.isUnderline;
>+                delete data.hasColorInfo;
>+                break;

Can we make this use the same bits as the mircResetColor? Why and/or why not?

> <snip>

>Index: xul/content/output-base.css

Could you add a comment before this mess indicating why it is messy (reference the bug comment if you don't feel like repeating yourself :-) )
 
> <snip messy CSS>

r+ with the above addressed. Whether you think I need to look at a new patch is up to you and possibly the answers to the questions :-)

Comment 4

[James Ross]

noStateChange used to be used for msgIsImportant (http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/extensions/irc/xul/content/static.js&rev=1.256&mark=1197#1180) and is the specific version for ANSI codes (like "dontLogURLs" for the linkifier and "noMircColors" for mIRC colours, etc.). Anything that can change the state during parsing should support these - they're useful for scripts (my input preview needs noStateChange for example).

Comment 5

[James Ross]

Attachment: Updated patch

I put the two blocks of deletes into a utility function any munger can use.

Comment 6

[:Gijs (he/him)]

Comment on attachment 404510 [details] [diff] [review]
Updated patch

r=me, thanks!

Comment 7

[James Ross]

Checked in --> FIXED.