From 7970b5c1e35726103ede881f5840453ad954a3e5 Mon Sep 17 00:00:00 2001
From: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
Date: Sun, 7 Aug 2022 06:05:42 -0700
Subject: [PATCH] Fix edge cases in link creation

---
 .../src/atlas/WebglCharAtlas.ts               |  2 +-
 demo/client.ts                                | 14 +++++++++++++
 demo/index.html                               |  1 +
 src/browser/OscLinkProvider.ts                | 20 +++++++++++++++----
 src/common/InputHandler.ts                    |  3 ++-
 5 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts
index 182b3b3e..13dceae6 100644
--- a/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts
+++ b/addons/xterm-addon-webgl/src/atlas/WebglCharAtlas.ts
@@ -356,7 +356,7 @@ export class WebglCharAtlas implements IDisposable {
 
   private _drawToCache(codeOrChars: number | string, bg: number, fg: number, ext: number): IRasterizedGlyph {
     const chars = typeof codeOrChars === 'number' ? String.fromCharCode(codeOrChars) : codeOrChars;
-    console.log('_drawToCache', chars, ext);
+
     this.hasCanvasChanged = true;
 
     // Allow 1 cell width per character, with a minimum of 2 (CJK), plus some padding. This is used
diff --git a/demo/client.ts b/demo/client.ts
index 64c694f5..8f858b39 100644
--- a/demo/client.ts
+++ b/demo/client.ts
@@ -189,6 +189,7 @@ if (document.location.pathname === '/test') {
   document.getElementById('powerline-symbol-test').addEventListener('click', powerlineSymbolTest);
   document.getElementById('underline-test').addEventListener('click', underlineTest);
   document.getElementById('ansi-colors').addEventListener('click', ansiColorsTest);
+  document.getElementById('osc-hyperlinks').addEventListener('click', addAnsiHyperlink);
   document.getElementById('add-decoration').addEventListener('click', addDecoration);
   document.getElementById('add-overview-ruler').addEventListener('click', addOverviewRuler);
 }
@@ -827,6 +828,19 @@ function ansiColorsTest() {
   }
 }
 
+function addAnsiHyperlink() {
+  term.write('\n\n\r');
+  term.writeln(`Regular link with no id:`);
+  term.writeln('\x1b]8;;https://github.com\x07GitHub\x1b]8;;\x07');
+  term.writeln('\x1b]8;;https://xtermjs.org\x07https://xtermjs.org\x1b]8;;\x07\x1b[C<- null cell');
+  term.writeln(`\nShared ID links:`);
+  term.writeln('╔════╗ ╔════╗');
+  term.writeln('║\x1b]8;;https://github.com\x07GitH\x1b]8;;\x07║ ║    ║');
+  term.writeln('║\x1b]8;;https://github.com\x07ub\x1b]8;;\x07  ║ ║    ║');
+  term.writeln('╚════╝ ╚════╝');
+  term.write('\x1b[3A\x1b[8C\x1b]8;;https://xtermjs.org\x07xter\x1b[B\x1b[4Dm.js\x1b]8;;\x07\x1b[2B\x1b[12D');
+}
+
 function addDecoration() {
   term.options['overviewRulerWidth'] = 15;
   const marker = term.registerMarker(1);
diff --git a/demo/index.html b/demo/index.html
index 836084b3..c38cb007 100644
--- a/demo/index.html
+++ b/demo/index.html
@@ -79,6 +79,7 @@
               <dd><button id="powerline-symbol-test" title="Write powerline symbol characters to the terminal (\ue0a0+)">Powerline symbol test</button></dd>
               <dd><button id="underline-test" title="Write text with Kitty's extended underline sequences">Underline test</button></dd>
 	            <dd><button id="ansi-colors" title="Write a wide range of ansi colors">Ansi colors test</button></dd>
+	            <dd><button id="osc-hyperlinks" title="Write some OSC 8 hyperlinks">Ansi hyperlinks test</button></dd>
 
               <dt>Decorations</dt>
               <dd><button id="add-decoration" title="Add a decoration to the terminal">Decoration</button></dd>
diff --git a/src/browser/OscLinkProvider.ts b/src/browser/OscLinkProvider.ts
index c7594479..7fee62af 100644
--- a/src/browser/OscLinkProvider.ts
+++ b/src/browser/OscLinkProvider.ts
@@ -28,12 +28,14 @@ export class OscLinkProvider implements ILinkProvider {
     let currentStart = -1;
     let finishLink = false;
     for (let x = 0; x < lineLength; x++) {
-      if (!line.hasContent(x)) {
+      // Minor optimization, only check for content if there isn't a link in case the link ends with
+      // a null cell
+      if (currentStart === -1 && !line.hasContent(x)) {
         continue;
       }
 
       line.loadCell(x, cell);
-      if (cell.extended.urlId) {
+      if (cell.hasExtendedAttrs() && cell.extended.urlId) {
         if (currentStart === -1) {
           currentStart = x;
           currentLinkId = cell.extended.urlId;
@@ -55,8 +57,15 @@ export class OscLinkProvider implements ILinkProvider {
             text,
             // These ranges are 1-based
             range: {
-              start: { x: currentStart + 1, y },
-              end: { x: x + 1, y }
+              start: {
+                x: currentStart + 1,
+                y
+              },
+              end: {
+                // Offset end x if it's a link that ends on the last cell in the line
+                x: x + (!finishLink && x === lineLength - 1 ? 1 : 0),
+                y
+              }
             },
             activate(e, text) {
               console.log('activate!', text);
@@ -64,6 +73,9 @@ export class OscLinkProvider implements ILinkProvider {
             // TODO: Embedder API to handle hover
           });
         }
+        currentStart = -1;
+        currentLinkId = -1;
+        finishLink = false;
       }
     }
     // TODO: Handle fetching and returning other link ranges to underline other links with the same id
diff --git a/src/common/InputHandler.ts b/src/common/InputHandler.ts
index b613a24a..e6d154b9 100644
--- a/src/common/InputHandler.ts
+++ b/src/common/InputHandler.ts
@@ -2937,15 +2937,16 @@ export class InputHandler extends Disposable implements IInputHandler {
       id = parsedParams[idParamIndex].slice(3) || undefined;
     }
     this._currentHyperlink = { id, uri };
+    console.log('start hyperlink');
     this._curAttrData.extended = this._curAttrData.extended.clone();
     this._curAttrData.extended.urlId = 1;
     this._curAttrData.updateExtended();
-    console.log('hasExtendedAttrs?', this._curAttrData.hasExtendedAttrs());
     this._onStartHyperlink.fire(this._currentHyperlink);
     return true;
   }
 
   private _finishHyperlink(): boolean {
+    console.log('finish hyperlink');
     this._curAttrData.extended = this._curAttrData.extended.clone();
     this._curAttrData.extended.urlId = 0;
     this._curAttrData.updateExtended();