diff --git a/src/InputHandler.ts b/src/InputHandler.ts index e1223e7e..34131a15 100644 --- a/src/InputHandler.ts +++ b/src/InputHandler.ts @@ -57,7 +57,7 @@ export class InputHandler implements IInputHandler { this._terminal.y++; if (this._terminal.y > this._terminal.scrollBottom) { this._terminal.y--; - this._terminal.scroll(); + this._terminal.scroll(true); } } else { if (ch_width === 2) // FIXME: check for xterm behavior diff --git a/src/Linkifier.test.ts b/src/Linkifier.test.ts index 2450ffb9..c6e59d84 100644 --- a/src/Linkifier.test.ts +++ b/src/Linkifier.test.ts @@ -119,6 +119,12 @@ describe('Linkifier', () => { it('should match a link immediately after a link at the end of a text node', done => { assertLinkifiesRow('foo barbaz', /bar|baz/, 'foo barbaz', done); }); + it('should not duplicate text after a unicode character (wrapped in a span)', done => { + // This is a regression test for an issue that came about when using + // an oh-my-zsh theme that added the large blue diamond unicode + // character (U+1F537) which caused the path to be duplicated. See #642. + assertLinkifiesRow('echo \'🔷foo\'', /foo/, 'echo \'🔷foo\'', done); + }); }); describe('validationCallback', () => { diff --git a/src/Linkifier.ts b/src/Linkifier.ts index f99cdaa4..bc4949b1 100644 --- a/src/Linkifier.ts +++ b/src/Linkifier.ts @@ -200,7 +200,7 @@ export class Linkifier { * Linkifies a row given a specific handler. * @param {HTMLElement} row The row to linkify. * @param {LinkMatcher} matcher The link matcher for this line. - * @return The link element if it was added, otherwise undefined. + * @return The link element(s) that were added. */ private _doLinkifyRow(row: HTMLElement, matcher: LinkMatcher): HTMLElement[] { // Iterate over nodes as we want to consider text nodes @@ -235,8 +235,21 @@ export class Linkifier { element.innerHTML = ''; element.appendChild(linkElement); } + } else if (node.childNodes.length > 1) { + // Matches part of string in an element with multiple child nodes + for (let j = 0; j < node.childNodes.length; j++) { + const childNode = node.childNodes[j]; + const childSearchIndex = childNode.textContent.indexOf(uri); + if (childSearchIndex !== -1) { + // Match found in currentNode + this._replaceNodeSubstringWithNode(childNode, linkElement, uri, childSearchIndex); + // Don't need to count nodesAdded by replacing the node as this + // is a child node, not a top-level node. + break; + } + } } else { - // Matches part of string + // Matches part of string in a single text node const nodesAdded = this._replaceNodeSubstringWithNode(node, linkElement, uri, searchIndex); // No need to consider the new nodes i += nodesAdded; @@ -308,25 +321,26 @@ export class Linkifier { * @return The number of nodes to skip when searching for the next uri. */ private _replaceNodeSubstringWithNode(targetNode: Node, newNode: Node, substring: string, substringIndex: number): number { - let node = targetNode; - if (node.nodeType !== 3/*Node.TEXT_NODE*/) { - node = node.childNodes[0]; + // If the targetNode is a non-text node with a single child, make the child + // the new targetNode. + if (targetNode.childNodes.length === 1) { + targetNode = targetNode.childNodes[0]; } // The targetNode will be either a text node or a . The text node // (targetNode or its only-child) needs to be replaced with newNode plus new // text nodes potentially on either side. - if (node.childNodes.length === 0 && node.nodeType !== 3/*Node.TEXT_NODE*/) { + if (targetNode.nodeType !== 3/*Node.TEXT_NODE*/) { throw new Error('targetNode must be a text node or only contain a single text node'); } - const fullText = node.textContent; + const fullText = targetNode.textContent; if (substringIndex === 0) { // Replace with const rightText = fullText.substring(substring.length); const rightTextNode = this._document.createTextNode(rightText); - this._replaceNode(node, newNode, rightTextNode); + this._replaceNode(targetNode, newNode, rightTextNode); return 0; } @@ -334,7 +348,7 @@ export class Linkifier { // Replace with const leftText = fullText.substring(0, substringIndex); const leftTextNode = this._document.createTextNode(leftText); - this._replaceNode(node, leftTextNode, newNode); + this._replaceNode(targetNode, leftTextNode, newNode); return 0; } @@ -343,7 +357,7 @@ export class Linkifier { const leftTextNode = this._document.createTextNode(leftText); const rightText = fullText.substring(substringIndex + substring.length); const rightTextNode = this._document.createTextNode(rightText); - this._replaceNode(node, leftTextNode, newNode, rightTextNode); + this._replaceNode(targetNode, leftTextNode, newNode, rightTextNode); return 1; } } diff --git a/src/Renderer.ts b/src/Renderer.ts index f0f50f61..7d8c6f85 100644 --- a/src/Renderer.ts +++ b/src/Renderer.ts @@ -353,9 +353,8 @@ export class Renderer { const endCol = viewportCappedStartRow === viewportCappedEndRow ? end[0] : this._terminal.cols; documentFragment.appendChild(this._createSelectionElement(viewportCappedStartRow, startCol, endCol)); // Draw middle rows - for (let i = viewportCappedStartRow + 1; i < viewportCappedEndRow; i++) { - documentFragment.appendChild(this._createSelectionElement(i, 0, this._terminal.cols)); - } + const middleRowsCount = viewportCappedEndRow - viewportCappedStartRow - 1; + documentFragment.appendChild(this._createSelectionElement(viewportCappedStartRow + 1, 0, this._terminal.cols, middleRowsCount)); // Draw final row if (viewportCappedStartRow !== viewportCappedEndRow) { // Only draw viewportEndRow if it's not the same as viewporttartRow @@ -371,9 +370,9 @@ export class Renderer { * @param colStart The start column. * @param colEnd The end columns. */ - private _createSelectionElement(row: number, colStart: number, colEnd: number): HTMLElement { + private _createSelectionElement(row: number, colStart: number, colEnd: number, rowCount: number = 1): HTMLElement { const element = document.createElement('div'); - element.style.height = `${this._terminal.charMeasure.height}px`; + element.style.height = `${rowCount * this._terminal.charMeasure.height}px`; element.style.top = `${row * this._terminal.charMeasure.height}px`; element.style.left = `${colStart * this._terminal.charMeasure.width}px`; element.style.width = `${this._terminal.charMeasure.width * (colEnd - colStart)}px`; diff --git a/src/SelectionManager.ts b/src/SelectionManager.ts index e2ba3756..930f09d1 100644 --- a/src/SelectionManager.ts +++ b/src/SelectionManager.ts @@ -184,12 +184,24 @@ export class SelectionManager extends EventEmitter { // Get middle rows for (let i = start[1] + 1; i <= end[1] - 1; i++) { - result.push(translateBufferLineToString(this._buffer.get(i), true)); + const bufferLine = this._buffer.get(i); + const lineText = translateBufferLineToString(bufferLine, true); + if (bufferLine.isWrapped) { + result[result.length - 1] += lineText; + } else { + result.push(lineText); + } } // Get final row if (start[1] !== end[1]) { - result.push(translateBufferLineToString(this._buffer.get(end[1]), true, 0, end[0])); + const bufferLine = this._buffer.get(end[1]); + const lineText = translateBufferLineToString(bufferLine, true, 0, end[0]); + if (bufferLine.isWrapped) { + result[result.length - 1] += lineText; + } else { + result.push(lineText); + } } // Format string by replacing non-breaking space chars with regular spaces @@ -291,6 +303,9 @@ export class SelectionManager extends EventEmitter { return; } + // Tell the browser not to start a regular selection + event.preventDefault(); + // Reset drag scroll state this._dragScrollAmount = 0; diff --git a/src/xterm.js b/src/xterm.js index 8f2d64e3..b485069a 100644 --- a/src/xterm.js +++ b/src/xterm.js @@ -782,7 +782,8 @@ Terminal.loadAddon = function(addon, callback) { Terminal.prototype.updateCharSizeStyles = function() { this.charSizeStyleElement.textContent = `.xterm-wide-char{width:${this.charMeasure.width * 2}px;}` + - `.xterm-normal-char{width:${this.charMeasure.width}px;}`; + `.xterm-normal-char{width:${this.charMeasure.width}px;}` + + `.xterm-rows > div{height:${this.charMeasure.height}px;}`; } /** @@ -1119,8 +1120,10 @@ Terminal.prototype.showCursor = function() { /** * Scroll the terminal down 1 row, creating a blank line. + * @param {boolean} isWrapped Whether the new line is wrapped from the previous + * line. */ -Terminal.prototype.scroll = function() { +Terminal.prototype.scroll = function(isWrapped) { var row; // Make room for the new row in lines @@ -1147,10 +1150,10 @@ Terminal.prototype.scroll = function() { if (row === this.lines.length) { // Optimization: pushing is faster than splicing when they amount to the same behavior - this.lines.push(this.blankLine()); + this.lines.push(this.blankLine(undefined, isWrapped)); } else { // add our new line - this.lines.splice(row, 0, this.blankLine()); + this.lines.splice(row, 0, this.blankLine(undefined, isWrapped)); } if (this.scrollTop !== 0) { @@ -2129,8 +2132,9 @@ Terminal.prototype.eraseLine = function(y) { /** * Return the data array of a blank line * @param {number} cur First bunch of data for each "blank" character. + * @param {boolean} isWrapped Whether the new line is wrapped from the previous line. */ -Terminal.prototype.blankLine = function(cur) { +Terminal.prototype.blankLine = function(cur, isWrapped) { var attr = cur ? this.eraseAttr() : this.defAttr; @@ -2139,6 +2143,12 @@ Terminal.prototype.blankLine = function(cur) { , line = [] , i = 0; + // TODO: It is not ideal that this is a property on an array, a buffer line + // class should be added that will hold this data and other useful functions. + if (isWrapped) { + line.isWrapped = isWrapped; + } + for (; i < this.cols; i++) { line[i] = ch; }