ErrorList applies black colour on ESC sequence reset

[mpheath]

ESC[0m and ESC[m apply SCE_ERR_ES_BLACK when it should reset to default. View text-formatting which states:

Value	Description	Behavior
0	Default	Returns all attributes to the default state prior to modification

0 is not a colour. It is a reset to default state. StyleFromSequence() treats everything as a colour so it treats 0 as SCE_ERR_ES_BLACK .

The issue is that SCE_ERR_ES_BLACK is not visible if the background colour is black. So example:

ESC[31m Red colour ESC[m Cannot see this black coloured text if the background is black

This image posted shows the issue as CRLF are visible except for those after ESC[0m which are black on a black background.

Patch code:

diff --git a/lexers/LexErrorList.cxx b/lexers/LexErrorList.cxx
index 22f831ec..f8fbd447 100644
--- a/lexers/LexErrorList.cxx
+++ b/lexers/LexErrorList.cxx
@@ -371,7 +371,9 @@ void ColouriseErrorListLine(
 				return;
 			case 'm':	// Colour command
 				styler.ColourTo(endSeqPosition, SCE_ERR_ESCSEQ);
-				portionStyle = StyleFromSequence(startSeq+2);
+				// ESC[m or ESC[0m reset to default style else get the colour style.
+				portionStyle = (startSeq[2] == 'm' || (startSeq[2] == '0' && startSeq[3] == 'm')) ?
+				               SCE_ERR_DEFAULT : StyleFromSequence(startSeq+2);
 				break;
 			case 'K':	// Erase to end of line -> ignore
 				styler.ColourTo(endSeqPosition, SCE_ERR_ESCSEQ);

Updated LexErrorList.cxx and test files:

fix.zip

[nyamatongwe]

The interpretation of ESC[m or ESC[0m likely belongs inside StyleFromSequence. While not necessary for now, there could be a need to interpret more complex SGR sequences which include ESC[0 like ESC[0;44m.

[mpheath]

I do consider ESC[0m with the optional 0 as special with setting SCE_ERR_DEFAULT.

The following does similar reset with 0 though would be a different styleID as to the blue background.

echo ESC[31m fg:Red ESC[0;44m bg:Blue ESC[m

The blue background has default foreground text.

echo ESC[31m fg:Red ESC[44m bg:Blue ESC[m

The blue background has red foreground text due to omitting the 0 as to the inheritance of the previous escape. That means the previous escape may need to be remembered by the lexer.

To do the complex is at a minimum 256 styleIDs AFAIK. This might be beyond the limits of a styleID based lexer. That is for 16 colours, imagine next when 256 colours is requested with needing 65536 styleIDs.

I do not know what to do about the complex idea suggested.

The next idea planned after this issue is to disable the RecogniseErrorListLine() function with lexer.errorlist.escape.sequences=2 so ErrorList can be just be a ANSI Escape lexer for downstream projects to use. As the image shows, some Notepad++ users are using ErrorList as a log lexer.

---

Edit: Considering the default colour and how default can be foreground or background colour, that may mean 1 + 17 * 17 = 290 styleIDs.

[nerdfever]

There's a larger but probably related issue in that Notepad++ issue. Unformatted text (prior to any ANSI escape sequences) appears in red instead of the default foreground color. And text between ESC[0m (reset) and ESC[1;31m (bold red) appears in default foreground color. (It should appear bold red).

At least in Notepad++. That issue shows a 15 byte file that demonstrates the problem.

(NP++ is also broken when editing text that contain ANSI escape sequences, but that might be unrelated to Scintilla. Might not.)

[mpheath]

@nerdfever ErrorList recognizes the text at the beginning of the lines in Red as GCC errors.

Details

See line 181 with the colons in the pattern <filename>:<line>:<message>

lexilla/lexers/LexErrorList.cxx

Lines 179 to 188 in 2ea69c4

	} else {
	// Look for one of the following formats:
	// GCC: <filename>:<line>:<message>
	// Microsoft: <filename>(<line>) :<message>
	// Common: <filename>(<line>): warning|error|note|remark|catastrophic|fatal
	// Common: <filename>(<line>) warning|error|note|remark|catastrophic|fatal
	// Microsoft: <filename>(<line>,<column>)<message>
	// CTags: <identifier>\t<filename>\t<message>
	// Lua 5 traceback: \t<filename>:<line>:<message>
	// Lua 5.1: <exe>: <filename>:<line>:<message>

and many more error patterns exist before the else keyword. That is why the lexer is called ErrorList as in the error patterns, not Log or any other name.

The misuse of the ErrorList lexer as a Log lexer is gross. The spilling of your issue into this issue is not helping as you are trying to broaden the scope. This issue is only about ESC[0m and the style it sets.

An issue has not been created yet at Lexilla to discuss anything else in your issue. I have mentioned in my previous comment about the next idea with a property setting though unless I create an issue with a patch for it then it still just an idea at this moment.

[mpheath]

@nyamatongwe

The interpretation of ESC[m or ESC[0m likely belongs inside StyleFromSequence.

This modification of StyleFromSequence() can interpret ESC[m or ESC[0m:

Details

int StyleFromSequence(const char *seq) noexcept {
	int bold = 0;
	int style = 0;
	while (!SequenceEnd(*seq)) {
		if (Is0To9(*seq)) {
			int base = *seq - '0';
			if (Is0To9(seq[1])) {
				base = base * 10;
				base += seq[1] - '0';
				seq++;
			}
			if (base == 0) {
				// Reset to default.
				style = 0;
				bold = 0;
			} else if (base == 1) {
				// Set style as bright.
				bold = 1;
			} else if (base >= 30 && base <= 37) {
				// Set dim style which starts at 40.
				style = base + 10;
			} else if (base >= 90 && base <= 97) {
				// Set bright style which starts at 48.
				style = base - 42;
			}
		}
		seq++;
	}
	// Set dim style as bright style if bold is set.
	if (bold && (style >= 40 && style <= 47)) {
		style += 8;
	}
	// Return the style which if 0 will be SCE_ERR_DEFAULT style.
	return style;
}

Instead of colour which could base to 0 and be added to SCE_ERR_ES_BLACK, this uses style which aligns values with the SCE_ERR_ES_* constants and if style ends as 0 then that is style SCE_ERR_DEFAULT. This removes the need of the previous patch code though the test files in the zip are still needed.

[nyamatongwe]

This change breaks the basic bold command ESC[1m which now resets to SCE_ERR_DEFAULT (0) instead of SCE_ERR_ES_DARK_GRAY (48) which was interpreted by SciTE as bold black.

Here is an example of running gcc -fdiagnostics-color before and after the change and in a command prompt window. The escape sequence here is actually ESC[01m but that is equivalent to ESC[1m.

Diagnostic.zip

[mpheath]

This change breaks the basic bold command ESC[1m which now resets to SCE_ERR_DEFAULT (0) instead of SCE_ERR_ES_DARK_GRAY (48) which was interpreted by SciTE as bold black.

IMO ESC[1m would be a bold default style if ErrorList had a style for it. Black was incorrect for ESC[0m so wondering what a resolve for ESC[1m might be to fix. Could set to SCE_ERR_ES_DARK_GRAY though might be considered an ugly fix as a bold default colour seems better suited and the later might need an additional style to be set.

As a comparison, the new Escape Sequence lexer that I am working on basically uses default like the other colours so it styles as bold default.

Details

Escape Sequence lexer ESC[1m is bold default.

[nyamatongwe]

Patch to fix regression for ESC[1m.

+++ b/lexers/LexErrorList.cxx
@@ -461,6 +461,9 @@ int StyleFromSequence(const char *seq) noexcept {
 			} else if (base == 1) {
 				// Set style as bright.
 				bold = 1;
+				if (style == 0) {
+					style = 40;
+				}
 			} else if (base >= 30 && base <= 37) {
 				// Set dim style which starts at 40.
 				style = base + 10;

[mpheath]

IMO, try the patch and see how it works. This issue is about ESC[0m. If ESC[1m is a concern later then it can be discussed later.

[nyamatongwe]

The commit for this issue caused a regression. Thus, either the change gets undone or a fix is applied over the change.