Link provider underline draws 1 extra character when after wide characters

[Tyriar]

I'm guessing this is in xterm.js as it appears to be purely a rendering issue, the link works file and excludes the ]:

[RishabhKothaari]

@Tyriar
I did a bunch of work on this issue. Currently, xterm.js fails to detect some links as the regex cannot capture all the cases and the same is true for rendering the underlines.

https://drive.google.com/open?id=19M6FJwpbNzRuMbjHE3QeC9BN0mPHg7yi

Rendering happens on

xterm.js/src/browser/renderer/LinkRenderLayer.ts

Lines 56 to 78 in f2ed019

	private _onShowLinkUnderline(e: ILinkifierEvent): void {
	if (e.fg === INVERTED_DEFAULT_COLOR) {
	this._ctx.fillStyle = this._colors.background.css;
	} else if (e.fg && is256Color(e.fg)) {
	// 256 color support
	this._ctx.fillStyle = this._colors.ansi[e.fg].css;
	} else {
	this._ctx.fillStyle = this._colors.foreground.css;
	}
	if (e.y1 === e.y2) {
	// Single line link
	this._fillBottomLineAtCells(e.x1, e.y1, e.x2 - e.x1);
	} else {
	// Multi-line link
	this._fillBottomLineAtCells(e.x1, e.y1, e.cols - e.x1);
	for (let y = e.y1 + 1; y < e.y2; y++) {
	this._fillBottomLineAtCells(0, y, e.cols);
	}
	this._fillBottomLineAtCells(0, e.y2, e.x2);
	}
	this._state = e;
	}

xterm.js/src/browser/renderer/BaseRenderLayer.ts

Lines 160 to 165 in 3a5def9

	protected _fillBottomLineAtCells(x: number, y: number, width: number = 1): void {
	this._ctx.fillRect(
	x * this._scaledCellWidth,
	(y + 1) * this._scaledCellHeight - window.devicePixelRatio - 1 /* Ensure it's drawn within the cell */,
	width * this._scaledCellWidth,
	window.devicePixelRatio);

all of this happens using based on range created using

xterm.js/addons/xterm-addon-web-links/src/WebLinkProvider.ts

Lines 24 to 71 in 9817754

	public static computeLink(position: IBufferCellPosition, regex: RegExp, terminal: Terminal, handler: (event: MouseEvent, uri: string) => void): ILink | undefined {
	const rex = new RegExp(regex.source, (regex.flags || '') + 'g');
	const [line, startLineIndex] = LinkComputer._translateBufferLineToStringWithWrap(position.y - 1, false, terminal);
	let match;
	let stringIndex = -1;
	while ((match = rex.exec(line)) !== null) {
	const text = match[1];
	if (!text) {
	// something matched but does not comply with the given matchIndex
	// since this is most likely a bug the regex itself we simply do nothing here
	console.log('match found without corresponding matchIndex');
	break;
	}
	// Get index, match.index is for the outer match which includes negated chars
	// therefore we cannot use match.index directly, instead we search the position
	// of the match group in text again
	// also correct regex and string search offsets for the next loop run
	stringIndex = line.indexOf(text, stringIndex + 1);
	rex.lastIndex = stringIndex + text.length;
	if (stringIndex < 0) {
	// invalid stringIndex (should not have happened)
	break;
	}
	let endX = stringIndex + text.length;
	let endY = startLineIndex + 1;
	while (endX > terminal.cols) {
	endX -= terminal.cols;
	endY++;
	}
	const range = {
	start: {
	x: stringIndex + 1,
	y: startLineIndex + 1
	},
	end: {
	x: endX,
	y: endY
	}
	};
	return { range, text, activate: handler };

I would like to work on this further. Can you let me know how can I reproduce the issue on xterm.js to test rendering without modifying the regex to handle links like
https://www.google.com/[我是学生].txt

Also, I feel WebLink addon must detect links with a regex similar to the vscode terminal.

[Tyriar]

Can you let me know how can I reproduce the issue on xterm.js

Maybe the only way right now is to it within VS Code as described in #2818 (comment) as it pulls in a bunch of monaco code to do the link detection. I'm pretty sure the bug is in xterm.js though as everything else seems to work, just not to rendering part. Alternatively you could try use a custom link matcher which matches on these characters to see if it can trigger the bug in the renderers.

Also, I feel WebLink addon must detect links with a regex similar to the vscode terminal.

The link support in VS Code only works because it builds on a lot of code inside the editor, in an effort to keep the web links addon simple and avoid needing to attribute, I thought it best to keep it regex based as it was before.