preview: keep the words around a complete untrusted wrap

core/client.js

@@ -1,3 +1,3 @@
 // Generated from src/client by `npm run build` (scripts/build-client.mjs). Do not edit by hand.
 window.__ModuleLoader__.load({
   id: 'ds-bot',
@@ -2419,10 +2419,12 @@
   for (let previous; previous !== out; ) {
     previous = out;
     const open = out.indexOf("<untrusted-message");
-    if (open < 0) break;
     const close = out.lastIndexOf("</untrusted-message>");
-    const inner = close > open ? out.slice(out.indexOf(">", open) + 1, close) : out;
-    out = stripFence(inner);
+    if (open < 0 || close <= open) break;
+    const head = out.slice(0, open);
+    const inner = out.slice(out.indexOf(">", open) + 1, close);
+    const tail = out.slice(close + "</untrusted-message>".length);
+    out = stripFence(head) + inner + stripFence(tail);
   }
   return stripFence(out).replace(/\s+/g, " ").trim();
 }

core/src/shared/preview.js

@@ -27,16 +27,20 @@ export function stripFence(text) {
 }
 
 // The words of a stored line as a preview shows them: inner text of a wrap, no marker of any shape,
-// and one line, because a row is one line.
+// and one line, because a row is one line. A wrap in the middle of a line loses its markers in place,
+// so the sentence around it survives: a reply that quotes a remote line and then answers it reads as
+// the reply, not as the quote.
 export function visibleText(text) {
   let out = String(text ?? '')
   for (let previous; previous !== out;) {
     previous = out
     const open = out.indexOf('<untrusted-message')
-    if (open < 0) break
     const close = out.lastIndexOf('</untrusted-message>')
-    const inner = close > open ? out.slice(out.indexOf('>', open) + 1, close) : out
-    out = stripFence(inner)
+    if (open < 0 || close <= open) break
+    const head = out.slice(0, open)
+    const inner = out.slice(out.indexOf('>', open) + 1, close)
+    const tail = out.slice(close + '</untrusted-message>'.length)
+    out = stripFence(head) + inner + stripFence(tail)
   }
   return stripFence(out).replace(/\s+/g, ' ').trim()
 }

core/test/preview-fence-words.test.js

@@ -0,0 +1,36 @@
+// A preview is still the line someone said (#1347): the fence is data, but so are the words on either
+// side of it. A complete wrap that sits in the middle of a line must lose its markers and keep the
+// sentence around it — including the reply that quotes a remote line and then answers it.
+import { test } from 'node:test'
+import assert from 'node:assert/strict'
+import { visibleText } from '../src/shared/preview.js'
+import { previewOf, readableLine } from '../src/client/text.js'
+
+test('words before and after a complete wrap stay in the line', () => {
+  assert.equal(visibleText('He said <untrusted-message from="Coder">quoted</untrusted-message> then left'), 'He said quoted then left')
+  assert.equal(visibleText('before <untrusted-message from="Coder">a</untrusted-message>'), 'before a')
+  assert.equal(visibleText('<untrusted-message from="Coder">a</untrusted-message> after'), 'a after')
+})
+
+test('every complete wrap in one line is unwrapped, not only the outermost pair', () => {
+  assert.equal(visibleText('a <untrusted-message from="C">one</untrusted-message> b <untrusted-message from="D">two</untrusted-message> c'), 'a one b two c')
+})
+
+test('a wrap on its own still reads as just its words', () => {
+  assert.equal(visibleText('<untrusted-message from="Coder">\nhello there\n</untrusted-message>'), 'hello there')
+})
+
+test('a broken marker fragment still loses the marker and keeps the words', () => {
+  assert.equal(visibleText('kick from="User"> B test 4 >'), 'kick B test 4 >')
+  assert.equal(visibleText('before <untrusted-message from="C">x'), 'before x')
+})
+
+test('a reply that quotes a remote line shows the reply, not only the quote', () => {
+  const reply = 'He said <untrusted-message from="Coder">\nplease rebase the branch\n</untrusted-message> and I answered that it is done.'
+  const line = readableLine(reply)
+  assert.equal(line, 'He said please rebase the branch and I answered that it is done.')
+  assert.doesNotMatch(line, /untrusted|from="/)
+
+  const list = { projectionsBySession: { s1: { values: { turnOutline: [{ turn: 1, response: reply }] } } }, byId: {} }
+  assert.equal(previewOf(list, 's1'), line)
+})

patches/ds-bot/0008-client-bundle.patch

@@ -1828,7 +1828,7 @@ diff --git a/client.js b/client.js
    const skip = [
      ...[...value.matchAll(LITERAL)].map((match) => ({ start: match.index, end: match.index + match[0].length })),
      ...referenceSpans(value, references)
-@@ -1445,24 +1445,86 @@ function mentionSpans(text, bots, { references = [] } = {}) {
+@@ -1445,24 +1445,88 @@ function mentionSpans(text, bots, { references = [] } = {}) {
    }
    return spans;
  }
@@ -1877,10 +1877,12 @@ diff --git a/client.js b/client.js
 +  for (let previous; previous !== out; ) {
 +    previous = out;
 +    const open = out.indexOf("<untrusted-message");
-+    if (open < 0) break;
 +    const close = out.lastIndexOf("</untrusted-message>");
-+    const inner = close > open ? out.slice(out.indexOf(">", open) + 1, close) : out;
-+    out = stripFence(inner);
++    if (open < 0 || close <= open) break;
++    const head = out.slice(0, open);
++    const inner = out.slice(out.indexOf(">", open) + 1, close);
++    const tail = out.slice(close + "</untrusted-message>".length);
++    out = stripFence(head) + inner + stripFence(tail);
 +  }
 +  return stripFence(out).replace(/\s+/g, " ").trim();
 +}

patches/ds-bot/files/src/shared/preview.js

@@ -27,16 +27,20 @@ export function stripFence(text) {
 }
 
 // The words of a stored line as a preview shows them: inner text of a wrap, no marker of any shape,
-// and one line, because a row is one line.
+// and one line, because a row is one line. A wrap in the middle of a line loses its markers in place,
+// so the sentence around it survives: a reply that quotes a remote line and then answers it reads as
+// the reply, not as the quote.
 export function visibleText(text) {
   let out = String(text ?? '')
   for (let previous; previous !== out;) {
     previous = out
     const open = out.indexOf('<untrusted-message')
-    if (open < 0) break
     const close = out.lastIndexOf('</untrusted-message>')
-    const inner = close > open ? out.slice(out.indexOf('>', open) + 1, close) : out
-    out = stripFence(inner)
+    if (open < 0 || close <= open) break
+    const head = out.slice(0, open)
+    const inner = out.slice(out.indexOf('>', open) + 1, close)
+    const tail = out.slice(close + '</untrusted-message>'.length)
+    out = stripFence(head) + inner + stripFence(tail)
   }
   return stripFence(out).replace(/\s+/g, ' ').trim()
 }

patches/ds-bot/files/test/preview-fence-words.test.js

@@ -0,0 +1,36 @@
+// A preview is still the line someone said (#1347): the fence is data, but so are the words on either
+// side of it. A complete wrap that sits in the middle of a line must lose its markers and keep the
+// sentence around it — including the reply that quotes a remote line and then answers it.
+import { test } from 'node:test'
+import assert from 'node:assert/strict'
+import { visibleText } from '../src/shared/preview.js'
+import { previewOf, readableLine } from '../src/client/text.js'
+
+test('words before and after a complete wrap stay in the line', () => {
+  assert.equal(visibleText('He said <untrusted-message from="Coder">quoted</untrusted-message> then left'), 'He said quoted then left')
+  assert.equal(visibleText('before <untrusted-message from="Coder">a</untrusted-message>'), 'before a')
+  assert.equal(visibleText('<untrusted-message from="Coder">a</untrusted-message> after'), 'a after')
+})
+
+test('every complete wrap in one line is unwrapped, not only the outermost pair', () => {
+  assert.equal(visibleText('a <untrusted-message from="C">one</untrusted-message> b <untrusted-message from="D">two</untrusted-message> c'), 'a one b two c')
+})
+
+test('a wrap on its own still reads as just its words', () => {
+  assert.equal(visibleText('<untrusted-message from="Coder">\nhello there\n</untrusted-message>'), 'hello there')
+})
+
+test('a broken marker fragment still loses the marker and keeps the words', () => {
+  assert.equal(visibleText('kick from="User"> B test 4 >'), 'kick B test 4 >')
+  assert.equal(visibleText('before <untrusted-message from="C">x'), 'before x')
+})
+
+test('a reply that quotes a remote line shows the reply, not only the quote', () => {
+  const reply = 'He said <untrusted-message from="Coder">\nplease rebase the branch\n</untrusted-message> and I answered that it is done.'
+  const line = readableLine(reply)
+  assert.equal(line, 'He said please rebase the branch and I answered that it is done.')
+  assert.doesNotMatch(line, /untrusted|from="/)
+
+  const list = { projectionsBySession: { s1: { values: { turnOutline: [{ turn: 1, response: reply }] } } }, byId: {} }
+  assert.equal(previewOf(list, 's1'), line)
+})