Merge pull request #3085 from jramosg/uncomment-with-not-first-column

Uncomment with not first column
Fixes #3081

Author: jramosg

CHANGELOG.md

@@ -5,6 +5,7 @@ Changes to Calva.
 ## [Unreleased]
 
 - Fix: [Toggle comments off, indent glitch](https://github.com/BetterThanTomorrow/calva/issues/3078)
+- Fix: [Toggle comments off fails when first line doesn't start with ;;](https://github.com/BetterThanTomorrow/calva/issues/3081)
 - Fix: [Toggle Line Comment can break structure for unformatted partial multiline selections](https://github.com/BetterThanTomorrow/calva/issues/3083)
 - Fix: [Clojure-lsp not starting when offline](https://github.com/BetterThanTomorrow/calva/issues/1299)
 

src/comment-prefix.ts

@@ -1,5 +1,21 @@
 /** Matches one or more leading semicolons (`;`, `;;`, `;;;`, etc.) */
-export const commentPrefixPattern = /^;+/;
+const commentPrefixPattern = /^;+/;
+
+/**
+ * Returns the first position from `candidatePositions` where a comment prefix
+ * (one or more semicolons) starts, or `undefined` if none match.
+ */
+export function findCommentPrefixStart(
+  lineText: string,
+  candidatePositions: number[]
+): number | undefined {
+  for (const pos of candidatePositions) {
+    if (commentPrefixPattern.test(lineText.slice(pos))) {
+      return pos;
+    }
+  }
+  return undefined;
+}
 
 /**
  * Calculates the end index for removing a comment prefix (semicolons + trailing space)
@@ -8,20 +24,20 @@ export const commentPrefixPattern = /^;+/;
  * if the line is otherwise empty after the prefix).
  *
  * @param lineText - The full text of the line
- * @param firstNonWhitespace - The index of the first non-whitespace character in the line
+ * @param removalStart - The index at which the comment prefix starts
  * @returns The end index for removal, or `undefined` if no comment prefix is found
  */
 export function calculateCommentPrefixRemovalEnd(
   lineText: string,
-  firstNonWhitespace: number
+  removalStart: number
 ): number | undefined {
-  const remainder = lineText.slice(firstNonWhitespace);
+  const remainder = lineText.slice(removalStart);
   const match = remainder.match(commentPrefixPattern);
   if (!match) {
     return undefined;
   }
 
-  let removalEnd = firstNonWhitespace + match[0].length;
+  let removalEnd = removalStart + match[0].length;
   if (removalEnd < lineText.length && lineText[removalEnd] === ' ') {
     let spaceRunEnd = removalEnd;
     // If the line is otherwise empty after the comment prefix, remove all trailing spaces as well

src/edit.ts

@@ -6,9 +6,9 @@ import * as select from './select';
 import * as printer from './printer';
 import { _semiColonWouldBreakStructureWhere } from './cursor-doc/paredit';
 import * as format from './calva-fmt/src/format';
-import { calculateCommentPrefixRemovalEnd, commentPrefixPattern } from './comment-prefix';
+import { calculateCommentPrefixRemovalEnd, findCommentPrefixStart } from './comment-prefix';
 
-export { commentPrefixPattern } from './comment-prefix';
+type CandidatesMap = Map<number, number[]>;
 
 // Relies on that `when` claus guards this from being called
 // when the cursor is before the comment marker
@@ -58,19 +58,39 @@ function getAffectedLineNumbers(editor: vscode.TextEditor, lineCount: number): n
     .sort((a, b) => a - b);
 }
 
+/**
+ * Builds candidate positions where a comment prefix might start on a line:
+ * first non-whitespace, then any selection start columns past that point.
+ */
+function commentCandidatePositions(
+  firstNonWhitespace: number,
+  lineNum: number,
+  selections?: readonly vscode.Selection[]
+): number[] {
+  const positions = [firstNonWhitespace];
+  if (selections) {
+    for (const sel of selections) {
+      if (sel.start.line === lineNum && sel.start.character > firstNonWhitespace) {
+        positions.push(sel.start.character);
+      }
+    }
+  }
+  return positions;
+}
+
 function areAllNonEmptyTargetLinesCommented(
   document: vscode.TextDocument,
-  lineNumbers: number[]
+  lineNumbers: number[],
+  candidatesMap: CandidatesMap
 ): boolean {
   const nonEmptyLines = lineNumbers.filter(
     (lineNum) => !document.lineAt(lineNum).isEmptyOrWhitespace
   );
   return (
     nonEmptyLines.length > 0 &&
     nonEmptyLines.every((lineNum) => {
-      const line = document.lineAt(lineNum);
-      const lineText = line.text.slice(line.firstNonWhitespaceCharacterIndex);
-      return commentPrefixPattern.test(lineText);
+      const candidates = candidatesMap.get(lineNum) ?? [];
+      return findCommentPrefixStart(document.lineAt(lineNum).text, candidates) !== undefined;
     })
   );
 }
@@ -401,31 +421,38 @@ async function reformatEnclosingFormsForLines(
 async function updateLineComments(
   editor: vscode.TextEditor,
   affectedLineNumbers: number[],
-  shouldUncomment: boolean
+  shouldUncomment: boolean,
+  candidatesMap: CandidatesMap
 ) {
   const descendingLineNumbers = affectedLineNumbers.sort((a, b) => b - a);
 
   await editor.edit(
     (editBuilder) => {
       for (const lineNum of descendingLineNumbers) {
         const line = editor.document.lineAt(lineNum);
-        const firstNonWhitespace = line.firstNonWhitespaceCharacterIndex;
         const lineText = line.text;
 
         if (shouldUncomment) {
-          const removalEnd = calculateCommentPrefixRemovalEnd(lineText, firstNonWhitespace);
+          const removalStart = findCommentPrefixStart(lineText, candidatesMap.get(lineNum) ?? []);
+          if (removalStart === undefined) {
+            continue;
+          }
+          const removalEnd = calculateCommentPrefixRemovalEnd(lineText, removalStart);
           if (removalEnd === undefined) {
             continue;
           }
 
           editBuilder.delete(
             new vscode.Range(
-              new vscode.Position(lineNum, firstNonWhitespace),
+              new vscode.Position(lineNum, removalStart),
               new vscode.Position(lineNum, removalEnd)
             )
           );
         } else {
-          editBuilder.insert(new vscode.Position(lineNum, firstNonWhitespace), ';; ');
+          editBuilder.insert(
+            new vscode.Position(lineNum, line.firstNonWhitespaceCharacterIndex),
+            ';; '
+          );
         }
       }
     },
@@ -444,9 +471,10 @@ async function updateLineComments(
 async function toggleCommentsThenReformatEnclosingForms(
   editor: vscode.TextEditor,
   affectedLineNumbers: number[],
-  shouldUncomment: boolean
+  shouldUncomment: boolean,
+  candidatesMap: CandidatesMap
 ) {
-  await updateLineComments(editor, affectedLineNumbers, shouldUncomment);
+  await updateLineComments(editor, affectedLineNumbers, shouldUncomment, candidatesMap);
   await reformatEnclosingFormsForLines(editor, affectedLineNumbers);
 }
 
@@ -475,9 +503,19 @@ export async function toggleLineCommentCommand() {
     return;
   }
 
+  const candidatesMap = new Map<number, number[]>();
+  for (const lineNum of affectedLineNumbers) {
+    const line = document.lineAt(lineNum);
+    candidatesMap.set(
+      lineNum,
+      commentCandidatePositions(line.firstNonWhitespaceCharacterIndex, lineNum, editor.selections)
+    );
+  }
+
   const allNonEmptyLinesCommented = areAllNonEmptyTargetLinesCommented(
     document,
-    affectedLineNumbers
+    affectedLineNumbers,
+    candidatesMap
   );
 
   const isSingleSelection = editor.selections.length === 1;
@@ -489,7 +527,8 @@ export async function toggleLineCommentCommand() {
   await toggleCommentsThenReformatEnclosingForms(
     editor,
     affectedLineNumbers,
-    allNonEmptyLinesCommented
+    allNonEmptyLinesCommented,
+    candidatesMap
   );
 }
 

src/extension-test/integration/suite/toggle-comment-test.ts

@@ -315,4 +315,28 @@ suite(suiteName, () => {
       '(x• (j (y |;; (a b c)|•     ))• z)'
     );
   });
+
+  it('should uncomment partial-selection comments round-trip (issue #3081)', async () => {
+    const editor = vscode.window.activeTextEditor;
+
+    // Simple partial selection
+    const simple = '(a |(b c•      d)|•   e)';
+    await performToggle(editor, simple);
+    const simpleCommented = textNotationFromDocAndSelections(editor.document, editor.selections);
+    assert.equal(simpleCommented, '(a |;; (b c•   ;;    d)|•   e)');
+    await vscode.commands.executeCommand('calva.toggleLineComment');
+    await new Promise((resolve) => setTimeout(resolve, pauseMs));
+    const simpleUncommented = textNotationFromDocAndSelections(editor.document, editor.selections);
+    assert.equal(simpleUncommented, simple);
+
+    // Nested partial selection
+    const nested = '(a |(b c•      (d e•         f)•      g)|•   e)';
+    await performToggle(editor, nested);
+    const nestedCommented = textNotationFromDocAndSelections(editor.document, editor.selections);
+    assert.equal(nestedCommented, '(a |;; (b c•   ;;    (d e•   ;;       f)•   ;;    g)|•   e)');
+    await vscode.commands.executeCommand('calva.toggleLineComment');
+    await new Promise((resolve) => setTimeout(resolve, pauseMs));
+    const nestedUncommented = textNotationFromDocAndSelections(editor.document, editor.selections);
+    assert.equal(nestedUncommented, nested);
+  });
 });