Line wrapping breaks custom multiline problem matchers

[enizor]

Related to #32042 & #45107

• VSCode Version: 1.40.2 & 1.41.0-insiders
• OS Version: Windows 1803 (conpty unavailable)

Steps to Reproduce:

1. Create a task outputting multiple long lines that will get wrapped in the terminal
2. Use a custom multiline pattern that should match on a part split into the two lines
3. Run the task with a limietd space for the terminal (so that the terminal will wrap lines) and having it opened beforehand (to avoid Wrapped task output if Terminal not opened #81328)
4. Observe the matched problems

Observation:
Pattern tries to match only on the first part of a wrapped line

Exemple:

"pattern": [
    {
        "kind": "file",
        "regexp": "^## \\+\\+ (WARNING|ERROR) : \\S+ : (.*)",
        "severity": 1,
        "message": 2,
    },
    {
        "kind": "file",
        "regexp": "^## \\+\\+ WARNING|ERROR : see Output file in (\\S+)",
        "file": 1
    }
]

Wrapped output:

## ++ ERROR : test_name : error message
## ++ ERROR : see Output file in c:\a\very\long\absolute\path\to\somewhere\in\the\wo
rkspace\test.traces

Resulting error:
error message is parsed correctly but the file location is parsed as c:\a\very\long\absolute\path\to\somewhere\in\the\wo

Does this issue occur when all extensions are disabled?: Yes

EDIT: The wrapped-line parsing works correctly if a single regexp is used.

[alexr00]

I think there's nothing we can do about this for winpty. The terminal sees that as three lines.
For conpty, I'm seeing this come through as 3 lines as well:

termline: "## ++ ERROR : test_name : error message"
termline: "## ++ ERROR : see Output file in c:\a\very\long\absolute\path\to
termline: "## ++ ERROR : see Output file in c:\a\very\long\absolute\path\to\somewhere\in\the\workspace\test.traces"

@Tyriar to confirm.

[Tyriar]

Hacks on hacks! I guess we could suppress triggering the end of a line here:

vscode/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts

Lines 509 to 515 in d8d87f5

	// Force line data to be sent when the cursor is moved, the main purpose for
	// this is because ConPTY will often not do a line feed but instead move the
	// cursor, in which case we still want to send the current line's data to tasks.
	xterm.parser.addCsiHandler({ final: 'H' }, () => {
	this._onCursorMove();
	return false;
	});

By using the same end of line wrapping trick:

vscode/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts

Lines 1168 to 1170 in d8d87f5

	if (newLine && !newLine.isWrapped) {
	this._sendLineData(buffer, buffer.baseY + buffer.cursorY - 1);
	}

[Tyriar]

@alexr00 could you try add this to conditional to _onCursorMove to see if it fixes since you can repro this easier than me?

vscode/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts

Lines 1166 to 1168 in d8d87f5

	const buffer = this._xterm!.buffer;
	const newLine = buffer.getLine(buffer.baseY + buffer.cursorY);
	if (newLine && !newLine.isWrapped) {

[alexr00]

Updated _onCursorMove:

	private _onCursorMove(): void {
		const buffer = this._xterm!.buffer;
		const newLine = buffer.getLine(buffer.baseY + buffer.cursorY);
		if (newLine && !newLine.isWrapped) {
			this._sendLineData(buffer, buffer.baseY + buffer.cursorY);
		}
	}

But I still see the extra broken line.

[Tyriar]

@alexr00 hmm, the fix might be a little more involved thinking about this more as linefeed hasn't fired at this point. We probably need to change this in xterm.js' window's mode to also run when the cursor is moved:

https://github.com/xtermjs/xterm.js/blob/70babeacb62fe05264d64324ca1f4436997efa1b/src/common/WindowsMode.ts#L9-L26

Does this work just to test out the theory? Below will treat all lines > 10 chars in width as wrapped (and not send line data).

	private _onCursorMove(): void {
		const buffer = this._xterm!.buffer;
		const newLine = buffer.getLine(buffer.baseY + buffer.cursorY);
		// Only send the line when the cursor is < 10
		if (newLine && buffer.cursorX < 10) {
			this._sendLineData(buffer, buffer.baseY + buffer.cursorY);
		}
	}

[alexr00]

@Tyriar that does solve this problem. I now just get the expected two lines and no broken lines.

[Tyriar]

@alexr00 I pulled in the change from xtermjs/xterm.js#2666, can you try it out and see if it's fixed now? (make sure you run yarn to pull in the new xterm version)

[alexr00]

Pulled and yarned, but I'm still seeing the extra broken line in onLineData:

termline: "## ++ ERROR : test_name : error message"
termline: "## ++ ERROR : see Output file in c:\a\very\long\absolute\path\to\somewhere\in\the\worksp
termline: "## ++ ERROR : see Output file in c:\a\very\long\absolute\path\to\somewhere\in\the\workspace\test.traces"

[Tyriar]

Might be related to the order of CSI handlers

[dsanders11]

@alexr00 @Tyriar, is there any hope of progress on this?

It affects the text captured by a problem matcher in weird ways, you don't even need long lines. If your window is small, it will be messed up by the wrapping of the terminal output. However, that only happens if you're viewing the terminal output. If I run a task and let it run without viewing the terminal output, the problem matcher works as expected. If I run the same task but view the terminal output while it runs, the visual wrapping done due to the small window affects the captured output by problem matchers.

It seems like that's a hint in the right direction here? Can the problem matchers be run on the raw output, like when the user hasn't viewed the terminal? That way wrapping in the terminal wouldn't affect anything.

[Tyriar]

There is hope as proper text wrapping support was added to conpty: #91898

[dsanders11]

@Tyriar, thanks for the info. I see that issue is from March, any vague idea on timeline of when it all might land? First half of 2021?

[Tyriar]

@dsanders11 the linked issue says the build after 19041 which is end of 2020, start of 20221. It's not clear yet if we need to do something to adopt it (like removing our heuristics if it works).

[Tyriar]

Closing this off as we don't plan on improving the winpty support and the issue isn't about conpty.