fix(editor): take a JSX block comment back out on the second toggling (#2121)

In a TSX file, a second Comment with Line Comment on a JSX line wrapped `{/* ... */}` in another comment, leaving `/* {/* ... */} */`, which no longer compiles. The comment tokens depend on the syntax node at a position, and the action read them only at the start of the range, so it could never find the comment already around the range.

Both Comment with Line Comment and Comment with Block Comment now read the token pairs at both ends of the range and past its last line, take the widest match, and fall back to CodeMirror only for the wrapping.

Siblings covered: multiple selections on JSX lines, and selections spanning separate comments such as `/* a */ b /* c */`, which are now left unchanged instead of having delimiters removed across live code.

Co-authored-by: aconeshana <[email protected]>

e2e/commentToggle.spec.ts

@@ -0,0 +1,32 @@
+import { expect, openFromTree, test } from "./fixtures";
+
+// A JSX comment is `{/* … */}` among JSX children, while its own text parses as
+// JavaScript, where the tokens are `/* … */`. Toggling must still take it back out.
+const SRC = `export function App() {
+  return (
+    <div>
+      <b>x</b>
+    </div>
+  );
+}
+`;
+
+test("⌘/ twice on a JSX line gives the line back", async ({ workbench: page }) => {
+  await page.evaluate(async (src) => {
+    const backendModule = "/src/editor/core/backend.ts";
+    const mod = await import(/* @vite-ignore */ backendModule);
+    const backend = mod.docBackend() as { files: Map<string, string> };
+    backend.files.set("/dev/acme-api/App.tsx", src);
+  }, SRC);
+
+  const editor = await openFromTree(page, "App.tsx");
+  const content = editor.locator(".cm-content");
+  await editor.locator(".cm-line", { hasText: "<b>x</b>" }).click();
+
+  await page.keyboard.press("Meta+/");
+  await expect(content).toContainText("{/* <b>x</b> */}");
+
+  await page.keyboard.press("Meta+/");
+  await expect(content).not.toContainText("/*");
+  await expect(editor.locator(".cm-line", { hasText: "<b>x</b>" })).toHaveText("<b>x</b>");
+});

src/editor/core/actions.ts

@@ -1,7 +1,7 @@
 /** Editor actions (ids from keymaps/intellij-macos.ts). */
 import { EditorSelection, type EditorState } from "@codemirror/state";
 import { EditorView } from "@codemirror/view";
-import { indentSelection, moveLineDown, moveLineUp, toggleBlockComment } from "@codemirror/commands";
+import { indentSelection, moveLineDown, moveLineUp } from "@codemirror/commands";
 import { foldEffect, syntaxTree, toggleFold } from "@codemirror/language";
 import { acceptCompletion, completionStatus } from "@codemirror/autocomplete";
 import { executeAction, registerActions, type Action } from "@/platform/actions";
@@ -9,6 +9,7 @@ import { getContext } from "@/platform/context";
 import { editorController, useGotoLine, useTabs } from "./controller";
 import {
   addCaretsToLineEnds,
+  commentBlock,
   commentLine,
   completeStatement,
   deleteLine,
@@ -661,7 +662,7 @@ export function registerEditorActions(): () => void {
       "color scheme",
     ]),
     text("code.commentLine", "Comment with Line Comment", (v) => commentLine(v)),
-    text("code.commentBlock", "Comment with Block Comment", (v) => toggleBlockComment(v)),
+    text("code.commentBlock", "Comment with Block Comment", (v) => commentBlock(v)),
     text("code.duplicateLine", "Duplicate Line or Selection", (v) => duplicateLineOrSelection(v)),
     text("code.deleteLine", "Delete Line", (v) => deleteLine(v)),
     text("code.moveLineUp", "Move Line Up", (v) => moveLineUp(v)),

src/editor/core/cm/comments.ts

@@ -1,5 +1,6 @@
 /**
- * Comment with Line Comment and Enter inside a line comment, as in IDEA.
+ * Comment with Line Comment, Comment with Block Comment and Enter inside a
+ * line comment, as in IDEA.
  */
 import {
   EditorSelection,
@@ -11,7 +12,7 @@ import {
 } from "@codemirror/state";
 import type { Command, EditorView } from "@codemirror/view";
 import { indentString, syntaxTree } from "@codemirror/language";
-import { toggleComment } from "@codemirror/commands";
+import { toggleBlockComment, toggleComment } from "@codemirror/commands";
 import { offsetAtVisibleColumn, visibleColumn } from "./visibleColumn";
 
 /** Code Style › <Language> › Code Generation › Comment Code. */
@@ -76,7 +77,7 @@ export const commentLine: Command = (view) => {
     lineToken(state, doc.line(b.startLine).from + leadingWs(doc.line(b.startLine).text)),
   );
   // Languages without line comments (CSS, HTML…): block comments.
-  if (tokens.some((t) => !t)) return toggleComment(view);
+  if (tokens.some((t) => !t)) return blockCommentLines(view);
 
   let allCommented = true;
   let commentedNonEmpty = false;
@@ -195,6 +196,164 @@ function minIndent(state: EditorState, startLine: number, endLine: number, token
   return min ?? 0;
 }
 
+// --- Block comments ---------------------------------------------------------------
+
+interface CommentPair {
+  open: string;
+  close: string;
+}
+
+/** The block comment token pairs the language offers at `pos`, the more specific first. */
+function commentPairs(state: EditorState, pos: number): CommentPair[] {
+  const pairs: CommentPair[] = [];
+  for (const data of state.languageDataAt<{ block?: CommentPair }>("commentTokens", pos)) {
+    const block = data?.block;
+    if (!block?.open || !block.close) continue;
+    if (!pairs.some((p) => p.open === block.open && p.close === block.close)) pairs.push(block);
+  }
+  return pairs;
+}
+
+const BLOCK_SEARCH = 50;
+
+/** A block comment around a range: the tokens' own positions, and how much of their margin to take. */
+interface BlockComment {
+  open: number;
+  openMargin: number;
+  close: number;
+  closeMargin: number;
+}
+
+/**
+ * The comment `pair` around `from..to`: the tokens just outside the range, or
+ * just inside it when the range takes the tokens in itself.
+ */
+function findBlockComment(state: EditorState, pair: CommentPair, from: number, to: number): BlockComment | null {
+  const doc = state.doc;
+  const before = state.sliceDoc(Math.max(0, from - BLOCK_SEARCH), from);
+  const after = state.sliceDoc(to, Math.min(doc.length, to + BLOCK_SEARCH));
+  const spaceBefore = /\s*$/.exec(before)![0].length;
+  const spaceAfter = /^\s*/.exec(after)![0].length;
+  const start = before.length - spaceBefore;
+  if (
+    before.slice(start - pair.open.length, start) === pair.open &&
+    after.slice(spaceAfter, spaceAfter + pair.close.length) === pair.close
+  )
+    return {
+      open: from - spaceBefore,
+      openMargin: spaceBefore ? 1 : 0,
+      close: to + spaceAfter,
+      closeMargin: spaceAfter ? 1 : 0,
+    };
+  const inside = to - from <= 2 * BLOCK_SEARCH;
+  const head = state.sliceDoc(from, inside ? to : from + BLOCK_SEARCH);
+  const tail = inside ? head : state.sliceDoc(Math.max(from, to - BLOCK_SEARCH), to);
+  const headSpace = /^\s*/.exec(head)![0].length;
+  const tailSpace = /\s*$/.exec(tail)![0].length;
+  const end = tail.length - tailSpace - pair.close.length;
+  if (
+    head.slice(headSpace, headSpace + pair.open.length) === pair.open &&
+    tail.slice(end, end + pair.close.length) === pair.close
+  )
+    return {
+      open: from + headSpace + pair.open.length,
+      openMargin: /\s/.test(head.charAt(headSpace + pair.open.length)) ? 1 : 0,
+      close: to - tailSpace - pair.close.length,
+      closeMargin: /\s/.test(tail.charAt(end - 1)) ? 1 : 0,
+    };
+  return null;
+}
+
+/** The lines the selection touches, from their first non-blank column to their end. */
+function selectedLines(state: EditorState): { from: number; to: number }[] {
+  const ranges: { from: number; to: number }[] = [];
+  const doc = state.doc;
+  for (const r of state.selection.ranges) {
+    const firstLine = doc.lineAt(r.from);
+    let lastLine = r.to <= firstLine.to ? firstLine : doc.lineAt(r.to);
+    if (lastLine.from > firstLine.from && lastLine.from === r.to)
+      lastLine = r.to === firstLine.to + 1 ? firstLine : doc.lineAt(r.to - 1);
+    const last = ranges[ranges.length - 1];
+    if (last && last.to > firstLine.from) last.to = lastLine.to;
+    else ranges.push({ from: firstLine.from + leadingWs(firstLine.text), to: lastLine.to });
+  }
+  return ranges;
+}
+
+/**
+ * Removes the block comment wrapping every one of `ranges`; false when one of
+ * them is not commented (the caller then comments instead).
+ *
+ * The pair to look for is read at both ends of a range and past its last line,
+ * and just outside matching tokens. A comment can span syntax contexts:
+ * a JSX `{/* … *\/}` is the extreme case — it sits in JSX children,
+ * while its own text parses as JavaScript, where the tokens are `/* … *\/`. The
+ * widest pair wins when several fit, so the outer comment is the one removed.
+ */
+function uncommentBlock(view: EditorView, ranges: { from: number; to: number }[]): boolean {
+  const { state } = view;
+  const doc = state.doc;
+  const comments = ranges.map((r) => {
+    const pairs = commentPairs(state, r.from);
+    for (const pos of [r.to, doc.lineAt(r.to).to])
+      for (const pair of commentPairs(state, pos))
+        if (!pairs.some((p) => p.open === pair.open && p.close === pair.close)) pairs.push(pair);
+    let best: { pair: CommentPair; comment: BlockComment } | null = null;
+    for (const pair of pairs) {
+      const comment = findBlockComment(state, pair, r.from, r.to);
+      if (comment)
+        for (const pos of [
+          Math.max(0, comment.open - pair.open.length - 1),
+          Math.min(doc.length, comment.close + pair.close.length + 1),
+        ])
+          for (const outer of commentPairs(state, pos))
+            if (!pairs.some((p) => p.open === outer.open && p.close === outer.close)) pairs.push(outer);
+      if (comment && (!best || comment.open - pair.open.length < best.comment.open - best.pair.open.length))
+        best = { pair, comment };
+    }
+    return best;
+  });
+  // End tokens from separate comments must not be removed across live code.
+  for (const done of comments) {
+    if (!done) continue;
+    const { pair, comment } = done;
+    if (!state.sliceDoc(comment.open, comment.close).includes(pair.close)) continue;
+    let node = syntaxTree(state).resolveInner(comment.open, 1);
+    while (node.parent && !/comment/i.test(node.name)) node = node.parent;
+    // A parser-confirmed nested comment can legitimately contain closing tokens.
+    if (
+      !/comment/i.test(node.name) ||
+      node.from !== comment.open - pair.open.length ||
+      node.to !== comment.close + pair.close.length
+    )
+      return true;
+  }
+  if (comments.some((c) => !c)) return false;
+
+  const changes: ChangeSpec[] = [];
+  for (const done of comments) {
+    const { pair, comment } = done!;
+    changes.push(
+      { from: comment.open - pair.open.length, to: comment.open + comment.openMargin },
+      { from: comment.close - comment.closeMargin, to: comment.close + pair.close.length },
+    );
+  }
+  view.dispatch({ changes, scrollIntoView: true, userEvent: "delete.uncomment" });
+  return true;
+}
+
+/** ⌘⇧/: block comments the selection, or takes the block comment back out. */
+export const commentBlock: Command = (view) => {
+  if (view.state.readOnly) return false;
+  const ranges = view.state.selection.ranges.map((r) => ({ from: r.from, to: r.to }));
+  return uncommentBlock(view, ranges) || toggleBlockComment(view);
+};
+
+/** ⌘/ in a language without line comments (CSS, HTML…): the block comment of the lines. */
+function blockCommentLines(view: EditorView): boolean {
+  return uncommentBlock(view, selectedLines(view.state)) || toggleComment(view);
+}
+
 // --- Enter in a line comment ------------------------------------------------------
 
 /** The start of the line comment the caret is in (after its prefix), else -1. */

src/editor/core/cm/editing.test.ts

@@ -15,7 +15,7 @@ import {
 } from "./editing";
 import { batchLanguage, commentTokens, findLanguageDescription } from "./languages";
 import { toggleCaret } from "./multiCaret";
-import { enterInLineComment, lineCommentStyle } from "./comments";
+import { commentBlock, enterInLineComment, lineCommentStyle } from "./comments";
 import { languages } from "@codemirror/language-data";
 
 /** Brace languages in miniature: one unit deeper after a line ending in `{`. */
@@ -386,3 +386,131 @@ describe("Batch language mode and @REM comment toggle", () => {
     expect(has("labelName", ":sub")).toBe(true);
   });
 });
+
+describe("toggle comment where the tokens depend on the syntax context", () => {
+  // A JSX comment is `{/* … */}` in JSX children, but its own text parses as
+  // JavaScript, where the tokens are `/* … */`.
+  const wrap = (body: string) => `const a = (\n  <div>\n    ${body}\n  </div>\n);`;
+
+  it("takes a JSX line comment back out, with the caret", async () => {
+    const tsx = await languages.find((l) => l.name === "TSX")!.load();
+    const v = make(wrap("|</div>"), tsx);
+    commentLine(v);
+    expect(v.state.doc.line(3).text).toBe("    {/* </div> */}");
+    commentLine(v);
+    expect(show(v)).toBe(wrap("|</div>"));
+  });
+
+  it("takes a JSX line comment back out, with the line selected", async () => {
+    const tsx = await languages.find((l) => l.name === "TSX")!.load();
+    const v = make(wrap("[</div>]"), tsx);
+    commentLine(v);
+    expect(v.state.doc.line(3).text).toBe("    {/* </div> */}");
+    commentLine(v);
+    expect(v.state.doc.line(3).text).toBe("    </div>");
+  });
+
+  it("takes a JSX block comment back out after ⌘⇧/", async () => {
+    const tsx = await languages.find((l) => l.name === "TSX")!.load();
+    const v = make(wrap("[</div>]"), tsx);
+    commentBlock(v);
+    expect(v.state.doc.line(3).text).toBe("    {/* </div> */}");
+    commentBlock(v);
+    expect(show(v)).toBe(wrap("[</div>]"));
+  });
+
+  it("takes a block comment back out in a language with no line comment", async () => {
+    const html = await languages.find((l) => l.name === "HTML")!.load();
+    const v = make("<section>\n  |<p>hi</p>\n</section>", html);
+    commentLine(v);
+    expect(v.state.doc.line(2).text).toBe("  <!-- <p>hi</p> -->");
+    commentLine(v);
+    expect(show(v)).toBe("<section>\n  |<p>hi</p>\n</section>");
+  });
+
+  it("wraps several selected JSX lines once and unwraps them again", async () => {
+    const tsx = await languages.find((l) => l.name === "TSX")!.load();
+    const v = make("const a = (\n  <div>\n    [<b>x</b>\n    <i>y</i>]\n  </div>\n);", tsx);
+    commentBlock(v);
+    expect(v.state.doc.toString()).toContain("{/* <b>x</b>\n    <i>y</i> */}");
+    commentBlock(v);
+    expect(v.state.doc.toString()).toBe("const a = (\n  <div>\n    <b>x</b>\n    <i>y</i>\n  </div>\n);");
+  });
+});
+
+describe("comment toggle regressions", () => {
+  const cases = [
+    ["TSX", "const a = <div>\n  [<b>x</b>\n  <i>y</i>]\n</div>;"],
+    ["TSX", "const a = <div>{\n  [value +\n  other]\n}</div>;"],
+    ["TSX", "[const a = 1;\nconst b = 2;]"],
+    ["HTML", "<section>\n  [<b>x</b>\n  <i>y</i>]\n</section>"],
+    ["CSS", "[a { color: red; }\nb { color: blue; }]"],
+    ["Vue", "<template>\n[<b>x</b>\n<i>y</i>]\n</template>"],
+  ];
+
+  for (const [action, command] of [
+    ["line", commentLine],
+    ["block", commentBlock],
+  ] as const) {
+    it.each(cases)(`${action} comments round trip in %s: %s`, async (language, input) => {
+      const ext = await languages.find((l) => l.name === language)!.load();
+      const v = make(input, ext);
+      const original = v.state.doc.toString();
+      expect(command(v)).toBe(true);
+      expect(v.state.doc.toString()).not.toBe(original);
+      expect(command(v)).toBe(true);
+      expect(v.state.doc.toString()).toBe(original);
+    });
+
+    it.each(["TSX", "HTML", "CSS", "Vue"])(`${action} comments round trip with multiple ranges in %s`, async (name) => {
+      const ext = await languages.find((l) => l.name === name)!.load();
+      let body = name === "CSS" ? "[a { color: red; }]\n[b { color: blue; }]" : "[<b>x</b>]\n[<i>y</i>]";
+      if (action === "line") body = body.replaceAll("[", "|").replaceAll("]", "");
+      const input = name === "TSX" ? `const a = <div>\n${body}\n</div>;` : body;
+      const v = make(input, ext);
+      const original = v.state.doc.toString();
+      expect(command(v)).toBe(true);
+      expect(command(v)).toBe(true);
+      expect(v.state.doc.toString()).toBe(original);
+      expect(v.state.selection.ranges).toHaveLength(2);
+    });
+
+    it.each([
+      ["CSS", "/*", "*/"],
+      ["JavaScript", "/*", "*/"],
+      ["HTML", "<!--", "-->"],
+      ["TSX", "{/*", "*/}"],
+    ])(`${action} comments do not join separate comments across live code in %s`, async (name, open, close) => {
+      const ext = await languages.find((l) => l.name === name)!.load();
+      for (const body of [`[${open} a ${close} b ${open} c ${close}]`, `${open} [a ${close} b ${open} c] ${close}`]) {
+        const input = name === "TSX" ? `const a = <div>\n${body}\n</div>;` : body;
+        const v = make(input, ext);
+        const original = v.state.doc.toString();
+        if (name === "JavaScript" && action === "line") {
+          commentLine(v);
+          commentLine(v);
+        } else command(v);
+        expect(v.state.doc.toString()).toBe(original);
+      }
+    });
+  }
+
+  it("keeps a line-only language's block command unavailable", async () => {
+    const ext = await languages.find((l) => l.name === "Python")!.load();
+    const v = make("[a = 1\nb = 2]", ext);
+    const original = v.state.doc.toString();
+    expect(commentBlock(v)).toBe(false);
+    expect(v.state.doc.toString()).toBe(original);
+    expect(commentLine(v)).toBe(true);
+    expect(v.state.doc.toString()).not.toBe(original);
+    expect(commentLine(v)).toBe(true);
+    expect(v.state.doc.toString()).toBe(original);
+  });
+
+  it("uncomments a parser-confirmed nested block comment", async () => {
+    const ext = await languages.find((l) => l.name === "Rust")!.load();
+    const v = make("[/* a /* b */ c */]", ext);
+    expect(commentBlock(v)).toBe(true);
+    expect(v.state.doc.toString()).toBe("a /* b */ c");
+  });
+});

src/editor/core/cm/editing.ts

@@ -659,7 +659,7 @@ export function completeStatement(language: string): Command {
 
 // --- comments ------------------------------------------------------------------------
 
-export { commentLine } from "./comments";
+export { commentBlock, commentLine } from "./comments";
 
 // --- smart keys ------------------------------------------------------------------
 

src/features/project-tree/fs.ts

@@ -225,6 +225,17 @@ export function sampleFs(root: string): MemoryFs {
       f("build.gradle.kts"),
       f("settings.gradle.kts"),
       f("README.md"),
+      f(
+        "App.tsx",
+        `export function App() {
+  return (
+    <div>
+      <b>x</b>
+    </div>
+  );
+}
+`,
+      ),
       ...Array.from({ length: 10_000 }, (_, i) => f(`generated/item${i + 1}.json`)),
     ]),
     root,