From 98aa084089f1f78a84de51c503a5b96a46b96775 Mon Sep 17 00:00:00 2001
From: Domagoj Kriskovic <dom.kriskovic@overleaf.com>
Date: Mon, 19 Jun 2023 13:23:47 +0200
Subject: [PATCH] [cm6+rt] getParentNode account for commands with arguments
 (#13345)

* [cm6+rt] getParentNode account for commands with arguments

* add explanation comment

* added test

* prettier

* check commandAncestor in same position

* using ancestorOfNodeWithToRange

* function rename

GitOrigin-RevId: bb837d956d029c1026dab03825634f24d5bc3905
---
 .../features/source-editor/commands/ranges.ts  | 18 +++++-------------
 .../utils/tree-operations/ancestors.ts         | 14 ++++++++++++++
 .../source-editor/commands/ranges.test.ts      | 16 ++++++++++++++++
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/services/web/frontend/js/features/source-editor/commands/ranges.ts b/services/web/frontend/js/features/source-editor/commands/ranges.ts
index 7bfa401e4d..c79207a431 100644
--- a/services/web/frontend/js/features/source-editor/commands/ranges.ts
+++ b/services/web/frontend/js/features/source-editor/commands/ranges.ts
@@ -11,6 +11,7 @@ import {
   ancestorOfNodeWithType,
   isUnknownCommandWithName,
 } from '../utils/tree-query'
+import { lastAncestorAtEndPosition } from '../utils/tree-operations/ancestors'
 
 export const wrapRanges =
   (
@@ -216,19 +217,10 @@ function getParentNode(
   let node: SyntaxNode | undefined | null = null
   if (typeof position === 'number') {
     node = tree?.resolveInner(position, assoc)?.parent
-    // HACK: Spaces after UnknownCommands (and other commands without arguments)
-    // are included in the Command node. So we have to adjust for that here.
-    const preceedingCharacter = state.sliceDoc(
-      Math.max(0, position - 1),
-      position
-    )
-    if (
-      preceedingCharacter === ' ' &&
-      ['UnknownCommand', 'Item', 'Left', 'Right'].some(name =>
-        node?.type.is(name)
-      )
-    ) {
-      node = ancestorOfNodeWithType(node, 'Command')?.parent
+
+    const ancestorAtEndPos = lastAncestorAtEndPosition(node, position)
+    if (ancestorAtEndPos?.parent) {
+      node = ancestorAtEndPos.parent
     }
   } else {
     node = position?.parent
diff --git a/services/web/frontend/js/features/source-editor/utils/tree-operations/ancestors.ts b/services/web/frontend/js/features/source-editor/utils/tree-operations/ancestors.ts
index 01038c4a6c..b698f0c056 100644
--- a/services/web/frontend/js/features/source-editor/utils/tree-operations/ancestors.ts
+++ b/services/web/frontend/js/features/source-editor/utils/tree-operations/ancestors.ts
@@ -99,6 +99,20 @@ export const ancestorOfNodeWithType = (
   return null
 }
 
+export const lastAncestorAtEndPosition = (
+  node: SyntaxNode | null | undefined,
+  to: number
+): SyntaxNode | null => {
+  for (let ancestor = node; ancestor; ancestor = ancestor.parent) {
+    if (ancestor.parent?.to === to) {
+      continue
+    } else if (ancestor.to === to) {
+      return ancestor
+    }
+  }
+  return null
+}
+
 export const descendantsOfNodeWithType = (
   node: SyntaxNode,
   type: string | number,
diff --git a/services/web/test/frontend/features/source-editor/commands/ranges.test.ts b/services/web/test/frontend/features/source-editor/commands/ranges.test.ts
index 9a847e7a99..b7fd567f3b 100644
--- a/services/web/test/frontend/features/source-editor/commands/ranges.test.ts
+++ b/services/web/test/frontend/features/source-editor/commands/ranges.test.ts
@@ -178,6 +178,22 @@ describe('toggleRanges', function () {
         cm.applyCommand(BOLD_COMMAND)
         expect(cm).line(1).to.equal('\\noindent \\textbf{<My paragraph>}')
       })
+
+      it('still formats after unknown command with arguments', function () {
+        const cm = new CodemirrorTestSession(['\\foo{test}<My paragraph>'])
+        cm.applyCommand(BOLD_COMMAND)
+        expect(cm).line(1).to.equal('\\foo{test}\\textbf{<My paragraph>}')
+      })
+
+      it('still formats after known command with arguments', function () {
+        const cm1 = new CodemirrorTestSession(['\\cite{foo}<text>'])
+        cm1.applyCommand(BOLD_COMMAND)
+        expect(cm1).line(1).to.equal('\\cite{foo}\\textbf{<text>}')
+
+        const cm2 = new CodemirrorTestSession(['\\href{url}{title}<text>'])
+        cm2.applyCommand(BOLD_COMMAND)
+        expect(cm2).line(1).to.equal('\\href{url}{title}\\textbf{<text>}')
+      })
     })
   })
 })