Fix path completion appending instead of replacing existing path

src/path-completion-item-provider.ts

@@ -2,14 +2,49 @@ import vscode from "vscode"
 import path from "path"
 import fs from "fs"
 import child_process from "child_process"
-import _ from "lodash"
 import util from "util"
-const exec = util.promisify(child_process.exec)
+const execFile = util.promisify(child_process.execFile)
 
-function parseAbsolutePatternFromLine(line: vscode.TextLine) {
-  const text = line.text.split("#", 2)[0]
-  const match = text.match(/^\s*(\/\S*)/)
-  return match?.[1]
+/**
+ * Locate the path segment under the cursor for an absolute pattern like
+ * `/src/comp|onents/foo.ts`.
+ *
+ * Returns the directory to list (`/src/`) along with the start and end
+ * character offsets of the segment being completed (`components`), so the
+ * completion replaces the segment instead of being inserted into it.
+ */
+export function parsePathSegmentAtCursor(
+  lineText: string,
+  cursorCharacter: number,
+):
+  | {
+      directory: string
+      segmentStart: number
+      segmentEnd: number
+      followedBySlash: boolean
+    }
+  | undefined {
+  const patternStart = lineText.search(/\S/)
+  if (patternStart === -1 || lineText[patternStart] !== "/") {
+    return undefined
+  }
+  const beforeCursor = lineText.slice(patternStart, cursorCharacter)
+  // cursor must be inside the pattern (the first whitespace-delimited token).
+  if (beforeCursor.length === 0 || /[\s#]/.test(beforeCursor)) {
+    return undefined
+  }
+  const lastSlash = beforeCursor.lastIndexOf("/")
+  const directory = beforeCursor.slice(0, lastSlash + 1)
+  const segmentStart = patternStart + lastSlash + 1
+  const rest = lineText.slice(cursorCharacter)
+  const segmentLength = rest.search(/[\s#/]|$/)
+  const segmentEnd = cursorCharacter + segmentLength
+  return {
+    directory,
+    segmentStart,
+    segmentEnd,
+    followedBySlash: lineText[segmentEnd] === "/",
+  }
 }
 
 function findRepositoryRoot(startPath: string) {
@@ -28,6 +63,31 @@ function findRepositoryRoot(startPath: string) {
   }
 }
 
+async function findIgnoredPaths(
+  repositoryRoot: string,
+  paths: string[],
+): Promise<Set<string>> {
+  // ignore git directory explicitly because git check-ignore won't.
+  const ignoredPaths = new Set<string>([path.join(repositoryRoot, ".git")])
+  if (paths.length === 0) {
+    return ignoredPaths
+  }
+  const stdout = await execFile("git", ["check-ignore", "--", ...paths], {
+    cwd: repositoryRoot,
+  }).then(
+    (result) => result.stdout,
+    // git-check-ignore exits 1 when there are no matching ignored files, so
+    // we'll get an exception in our common flow.
+    (e: { stdout?: string }) => e.stdout ?? "",
+  )
+  for (const ignoredPath of stdout.split("\n")) {
+    if (ignoredPath !== "") {
+      ignoredPaths.add(ignoredPath)
+    }
+  }
+  return ignoredPaths
+}
+
 export class PathCompletionItemProvider
   implements vscode.CompletionItemProvider
 {
@@ -38,70 +98,62 @@ export class PathCompletionItemProvider
   async provideCompletionItems(
     document: vscode.TextDocument,
     position: vscode.Position,
-  ): Promise<
-    | vscode.CompletionItem[]
-    | vscode.CompletionList<vscode.CompletionItem>
-    | null
-    | undefined
-  > {
+  ): Promise<vscode.CompletionItem[]> {
     const line = document.lineAt(position.line)
+    const segment = parsePathSegmentAtCursor(line.text, position.character)
+    if (segment == null) {
+      return []
+    }
 
     const repositoryRoot = findRepositoryRoot(document.uri.fsPath)
     if (repositoryRoot == null) {
       this.outputChannel.appendLine("Could not find repository root from file")
       return []
     }
 
-    const pattern = parseAbsolutePatternFromLine(line)
-    if (pattern == null) {
+    const directoryPath = path.join(repositoryRoot, segment.directory)
+    const files = await fs.promises
+      .readdir(directoryPath, { withFileTypes: true })
+      .catch(() => null)
+    if (files == null) {
       return []
     }
 
-    const patternPath = path.join(repositoryRoot, pattern)
-    let files = []
-    try {
-      files = fs.readdirSync(patternPath, { withFileTypes: true })
-    } catch (e) {
-      return []
-    }
+    const ignoredPaths = await findIgnoredPaths(
+      repositoryRoot,
+      files.map((x) => path.join(directoryPath, x.name)),
+    )
 
-    const ignoredPaths = new Set<string>()
-    // ignore git directory explicitly because git check-ignore won't.
-    const gitDirectory = path.join(repositoryRoot, ".git")
-    ignoredPaths.add(gitDirectory)
+    // Replace the whole segment under the cursor so selecting a completion
+    // updates an existing path rather than inserting into the middle of it.
+    const range = new vscode.Range(
+      position.line,
+      segment.segmentStart,
+      position.line,
+      segment.segmentEnd,
+    )
 
-    const fileArgs = files.map((x) => path.join(patternPath, x.name)).join(" ")
-    try {
-      const { stdout } = await exec(`git check-ignore ${fileArgs}`, {
-        cwd: repositoryRoot,
-      })
-      for (const ignoredPath of stdout.split("\n")) {
-        if (ignoredPath != null) {
-          ignoredPaths.add(ignoredPath)
-        }
-      }
-      // git-check-ignore errors when there are no matching ignored files.
-      // so we'll get an exception in our common flow.
-    } catch {}
-
-    const completionItems = files
-      .filter((x) => {
-        const absolutePath = path.join(patternPath, x.name)
-        return !ignoredPaths.has(absolutePath)
-      })
+    return files
+      .filter((x) => !ignoredPaths.has(path.join(directoryPath, x.name)))
       .map((x): vscode.CompletionItem => {
         const isDirectory = x.isDirectory()
-        const kind = isDirectory
-          ? vscode.CompletionItemKind.Folder
-          : vscode.CompletionItemKind.File
-
-        return {
-          label: x.name,
-          sortText: `${isDirectory ? "a" : "b"}:${x.name}`,
-          kind,
+        const item = new vscode.CompletionItem(
+          x.name,
+          isDirectory
+            ? vscode.CompletionItemKind.Folder
+            : vscode.CompletionItemKind.File,
+        )
+        item.range = range
+        item.sortText = `${isDirectory ? "a" : "b"}:${x.name}`
+        if (isDirectory && !segment.followedBySlash) {
+          // add the trailing slash and keep completing into the directory.
+          item.insertText = `${x.name}/`
+          item.command = {
+            title: "Trigger suggest",
+            command: "editor.action.triggerSuggest",
+          }
         }
+        return item
       })
-
-    return _.sortBy(completionItems, (x) => x.kind + "?" + x.label)
   }
 }

test/suite/path-completion-item-provider.test.ts

@@ -0,0 +1,51 @@
+import assert from "assert"
+import { parsePathSegmentAtCursor } from "../../src/path-completion-item-provider"
+
+function parse(lineWithCursor: string) {
+  const cursor = lineWithCursor.indexOf("|")
+  return parsePathSegmentAtCursor(lineWithCursor.replace("|", ""), cursor)
+}
+
+suite("parsePathSegmentAtCursor", () => {
+  test("completes a new segment at the end of a path", () => {
+    assert.deepStrictEqual(parse("/src/|"), {
+      directory: "/src/",
+      segmentStart: 5,
+      segmentEnd: 5,
+      followedBySlash: false,
+    })
+  })
+  test("replaces the rest of an existing segment", () => {
+    assert.deepStrictEqual(parse("/src/comp|onents @owner"), {
+      directory: "/src/",
+      segmentStart: 5,
+      segmentEnd: 15,
+      followedBySlash: false,
+    })
+  })
+  test("replaces a segment in the middle of a path", () => {
+    assert.deepStrictEqual(parse("  /src/co|mponents/Button.tsx @owner"), {
+      directory: "/src/",
+      segmentStart: 7,
+      segmentEnd: 17,
+      followedBySlash: true,
+    })
+  })
+  test("includes dots and dashes in the segment", () => {
+    assert.deepStrictEqual(parse("/src/my-fi|le.test.ts"), {
+      directory: "/src/",
+      segmentStart: 5,
+      segmentEnd: 20,
+      followedBySlash: false,
+    })
+  })
+  test("ignores non-absolute patterns", () => {
+    assert.strictEqual(parse("src/|"), undefined)
+    assert.strictEqual(parse("*.js|"), undefined)
+  })
+  test("ignores the cursor outside the pattern", () => {
+    assert.strictEqual(parse("/src/ @own|er"), undefined)
+    assert.strictEqual(parse("# /src/|"), undefined)
+    assert.strictEqual(parse("|/src/"), undefined)
+  })
+})