feat(unity): open scripts from external editor arguments (#2296)

* feat(app): support Unity external editor navigation

* docs(unity): explain manual external editor setup

src/features/app/__tests__/windows.test.ts

@@ -6,9 +6,14 @@ const projectCurrent = vi.fn();
 const windowOpenProject = vi.fn();
 const windowOpenFiles = vi.fn();
 const windowTakeLaunch = vi.fn();
+const windowCompleteLaunch = vi.fn();
 const wslResolve = vi.fn();
 const cancelClose = vi.fn();
 const notify = vi.fn();
+const listen = vi.fn();
+vi.mock("@tauri-apps/api/webviewWindow", () => ({
+  getCurrentWebviewWindow: () => ({ listen }),
+}));
 vi.mock("@/platform/remote", async (original) => ({
   ...(await original<typeof import("@/platform/remote")>()),
   isRemoteWindow: vi.fn(() => false),
@@ -36,6 +41,7 @@ vi.mock("@/platform/ipc", async (orig) => {
       windowOpenProject: (...a: unknown[]) => windowOpenProject(...a),
       windowOpenFiles: (...a: unknown[]) => windowOpenFiles(...a),
       windowTakeLaunch: () => windowTakeLaunch(),
+      windowCompleteLaunch: () => windowCompleteLaunch(),
       wslResolve: (p: string) => wslResolve(p),
       lightEditOpen: (p: string) => lightEditOpen(p),
       fsReadDir: (p: string) => fsReadDir(p),
@@ -47,18 +53,20 @@ vi.mock("@/platform/project", async (orig) => {
   return { ...real, openProject: (p: string) => openProject(p) };
 });
 const diffs: unknown[] = [];
-vi.mock("@/features/vcs/diffInput", () => ({ openVcsDiff: (data: unknown) => void diffs.push(data) }));
+const openDiff = vi.fn(async (data: unknown) => void diffs.push(data));
+vi.mock("@/features/vcs/diffInput", () => ({ openVcsDiff: (data: unknown) => openDiff(data) }));
 
 const {
   openProjectInWindow,
   consumeLaunch,
+  installLaunchHandling,
   openFiles,
   openPaths,
   openLightEditFileAsProject,
   openProjectWithPolicy,
   windowActions,
 } = await import("../windows");
-const { useProject } = await import("@/platform/project");
+const { useProject, startupLaunchHandled } = await import("@/platform/project");
 const { provideEditorService, provideNotificationService } = await import("@/workbench/contributions");
 const { isRemoteWindow, connectRemote } = await import("@/platform/remote");
 const { IpcError } = await import("@/platform/ipc");
@@ -66,11 +74,14 @@ provideNotificationService({ notify });
 
 const opened: string[] = [];
 let activePath: string | null = null;
-provideEditorService({
-  openFile: async (path, options) =>
+const openEditorFile = vi.fn<import("@/workbench/contributions").EditorService["openFile"]>(
+  async (path, options) =>
     void opened.push(
       `${path}${options?.focus ? " (focus)" : ""}${options?.selection ? ` @${options.selection.line}:${options.selection.col}` : ""}`,
     ),
+);
+provideEditorService({
+  openFile: openEditorFile,
   openInput: async () => {},
   activePath: () => activePath,
   activeCursor: () => null,
@@ -81,18 +92,29 @@ provideEditorService({
 const ok = (data: unknown) => ({ status: "ok", data });
 const projectState = (root: string) => ({ project: { root, name: root, trusted: true, isGit: false, settings: {} } });
 
+function deferred() {
+  let resolve!: () => void;
+  const promise = new Promise<void>((done) => (resolve = done));
+  return { promise, resolve };
+}
+
 beforeEach(() => {
   for (const f of [windowOpenProject, windowOpenFiles, windowTakeLaunch, wslResolve]) f.mockReset();
+  windowTakeLaunch.mockResolvedValue(null);
+  windowCompleteLaunch.mockReset().mockResolvedValue(ok(null));
   wslResolve.mockResolvedValue(null);
   vi.mocked(connectRemote).mockClear();
   for (const f of [lightEditOpen, fsReadDir, openProject]) f.mockClear();
   cancelClose.mockClear();
   notify.mockClear();
+  listen.mockReset();
+  openEditorFile.mockClear();
+  openDiff.mockClear();
   vi.mocked(isRemoteWindow).mockReturnValue(false);
   opened.length = 0;
   diffs.length = 0;
   activePath = null;
-  useProject.setState({ project: null, lightEdit: false, settings: null } as never);
+  useProject.setState({ project: null, lightEdit: false, settings: null, opening: false, openingPath: null } as never);
 });
 
 describe("openProjectInWindow", () => {
@@ -272,12 +294,160 @@ describe("Open Containing Folder as Project", () => {
 });
 
 describe("launch", () => {
+  it("subscribes before taking a launch received while the listener is registering", async () => {
+    const registered = deferred();
+    const unlisten = vi.fn();
+    listen.mockImplementationOnce(async () => {
+      await registered.promise;
+      windowTakeLaunch.mockResolvedValueOnce({ project: "/p", files: ["/p/late.cs"] });
+      return unlisten;
+    });
+    const dispose = installLaunchHandling();
+    try {
+      await vi.waitFor(() => expect(listen).toHaveBeenCalledOnce());
+      expect(windowTakeLaunch).not.toHaveBeenCalled();
+      registered.resolve();
+      await startupLaunchHandled();
+      expect(opened).toEqual(["/p/late.cs (focus)"]);
+    } finally {
+      registered.resolve();
+      await startupLaunchHandled();
+      dispose();
+    }
+    expect(unlisten).toHaveBeenCalledOnce();
+  });
+
+  it("does not consume a launch after disposal while the listener is registering", async () => {
+    const registered = deferred();
+    const unlisten = vi.fn();
+    listen.mockImplementationOnce(async () => {
+      await registered.promise;
+      return unlisten;
+    });
+    windowTakeLaunch.mockResolvedValueOnce({ project: "/p", files: ["/p/a.cs"] });
+    const dispose = installLaunchHandling();
+    await vi.waitFor(() => expect(listen).toHaveBeenCalledOnce());
+    dispose();
+    registered.resolve();
+    await startupLaunchHandled();
+    expect(unlisten).toHaveBeenCalledOnce();
+    expect(windowTakeLaunch).not.toHaveBeenCalled();
+    expect(opened).toEqual([]);
+  });
+
+  it("serializes launch events while their project is opening", async () => {
+    const openedProject = deferred();
+    openProject.mockImplementationOnce(async (path) => {
+      await openedProject.promise;
+      const state = projectState(path);
+      useProject.setState({ ...state, lightEdit: false } as never);
+      return state.project;
+    });
+    let onLaunch: (() => void) | undefined;
+    listen.mockImplementationOnce(async (_event, handler: () => void) => {
+      onLaunch = handler;
+      return () => {};
+    });
+    windowTakeLaunch
+      .mockResolvedValueOnce({ project: "/p", files: ["/p/a.cs"] })
+      .mockResolvedValueOnce({ files: ["/p/b.cs"], positions: [{ path: "/p/b.cs", line: 10, column: 3 }] });
+    const dispose = installLaunchHandling();
+    try {
+      await vi.waitFor(() => expect(openProject).toHaveBeenCalledOnce());
+      onLaunch!();
+      openedProject.resolve();
+      await startupLaunchHandled();
+      await consumeLaunch();
+      expect(lightEditOpen).not.toHaveBeenCalled();
+      expect(opened).toEqual(["/p/a.cs (focus)", "/p/b.cs (focus) @10:3"]);
+      expect(openProject).toHaveBeenCalledOnce();
+    } finally {
+      openedProject.resolve();
+      await startupLaunchHandled();
+      dispose();
+    }
+  });
+
+  it("waits for an existing project restore before opening a received file", async () => {
+    useProject.setState({ opening: true, openingPath: "/p" });
+    windowTakeLaunch.mockResolvedValueOnce({
+      files: ["/p/a.cs"],
+      positions: [{ path: "/p/a.cs", line: 8, column: 2 }],
+    });
+    const request = consumeLaunch();
+    try {
+      await vi.waitFor(() => expect(windowTakeLaunch).toHaveBeenCalledOnce());
+      await new Promise((resolve) => setTimeout(resolve, 0));
+      expect(opened).toEqual([]);
+      expect(lightEditOpen).not.toHaveBeenCalled();
+      expect(windowCompleteLaunch).not.toHaveBeenCalled();
+    } finally {
+      useProject.setState({ ...projectState("/p"), opening: false, openingPath: null } as never);
+      await request;
+    }
+    expect(openProject).not.toHaveBeenCalled();
+    expect(lightEditOpen).not.toHaveBeenCalled();
+    expect(opened).toEqual(["/p/a.cs (focus) @8:2"]);
+    expect(windowCompleteLaunch).toHaveBeenCalledOnce();
+  });
+
+  it("keeps the open project and applies later locations in order", async () => {
+    useProject.setState(projectState("/p") as never);
+    for (const line of [5, 8, 12]) {
+      windowTakeLaunch.mockResolvedValueOnce({
+        project: "/p",
+        files: ["/p/a.cs"],
+        positions: [{ path: "/p/a.cs", line, column: 0 }],
+      });
+    }
+    await Promise.all([consumeLaunch(), consumeLaunch(), consumeLaunch()]);
+    expect(openProject).not.toHaveBeenCalled();
+    expect(opened).toEqual(["/p/a.cs (focus) @5:0", "/p/a.cs (focus) @8:0", "/p/a.cs (focus) @12:0"]);
+  });
+
+  it("reports a missing file and still handles the next launch", async () => {
+    useProject.setState(projectState("/p") as never);
+    openEditorFile.mockRejectedValueOnce(new Error("File not found"));
+    windowTakeLaunch.mockResolvedValueOnce({ files: ["/p/missing.cs"] }).mockResolvedValueOnce({ files: ["/p/a.cs"] });
+    await Promise.all([consumeLaunch(), consumeLaunch()]);
+    expect(notify).toHaveBeenCalledWith(expect.objectContaining({ level: "error", body: "File not found" }));
+    expect(opened).toEqual(["/p/a.cs (focus)"]);
+    expect(windowCompleteLaunch).toHaveBeenCalledTimes(2);
+  });
+
+  it.each([
+    new Error("Project unavailable"),
+    new IpcError({ kind: "cancelled", message: "Project opening cancelled" }),
+  ])("completes a failed or cancelled project launch and handles the next: %s", async (error) => {
+    openProject.mockRejectedValueOnce(error);
+    windowTakeLaunch
+      .mockResolvedValueOnce({ project: "/p", files: ["/p/a.cs"] })
+      .mockResolvedValueOnce({ project: "/q", files: ["/q/b.cs"] });
+    await Promise.all([consumeLaunch(), consumeLaunch()]);
+    expect(windowCompleteLaunch).toHaveBeenCalledTimes(2);
+    expect(opened).toEqual(["/q/b.cs (focus)"]);
+    expect(useProject.getState().project?.root).toBe("/q");
+  });
+
+  it("keeps handling launches after completing a launch fails", async () => {
+    useProject.setState(projectState("/p") as never);
+    windowCompleteLaunch.mockRejectedValueOnce(new Error("Connection interrupted"));
+    windowTakeLaunch.mockResolvedValueOnce({ files: ["/p/a.cs"] }).mockResolvedValueOnce({ files: ["/p/b.cs"] });
+    const first = consumeLaunch();
+    const second = consumeLaunch();
+    await expect(first).rejects.toThrow("Connection interrupted");
+    await second;
+    expect(windowCompleteLaunch).toHaveBeenCalledTimes(2);
+    expect(opened).toEqual(["/p/a.cs (focus)", "/p/b.cs (focus)"]);
+  });
+
   it("opens the pending project once", async () => {
     windowTakeLaunch.mockResolvedValueOnce({ project: "/p/launch", files: [] }).mockResolvedValueOnce(null);
     await consumeLaunch();
     await consumeLaunch();
     expect(openProject).toHaveBeenCalledTimes(1);
     expect(openProject).toHaveBeenCalledWith("/p/launch");
+    expect(windowCompleteLaunch).toHaveBeenCalledOnce();
   });
 
   it("opens launched files in LightEdit without a project", async () => {
@@ -310,6 +480,7 @@ describe("launch", () => {
     windowTakeLaunch.mockResolvedValueOnce({ project: null, files: [], diff: { left: "/x/a.txt", right: "/x/b.txt" } });
     await consumeLaunch();
     expect(lightEditOpen).toHaveBeenCalledWith("/x/b.txt");
+    expect(windowCompleteLaunch).toHaveBeenCalledOnce();
     expect(diffs).toEqual([
       {
         path: "/x/b.txt",
@@ -324,6 +495,7 @@ describe("launch", () => {
     windowTakeLaunch.mockResolvedValueOnce({ project: null, files: [], diff: { left: "/p/a.txt", right: "/p/b.txt" } });
     await consumeLaunch();
     expect(lightEditOpen).not.toHaveBeenCalled();
+    expect(windowCompleteLaunch).toHaveBeenCalledOnce();
     expect(diffs).toEqual([
       {
         path: "/p/b.txt",
@@ -332,6 +504,90 @@ describe("launch", () => {
       },
     ]);
   });
+
+  it("completes a failed CLI diff and handles the next file launch", async () => {
+    useProject.setState(projectState("/p") as never);
+    openDiff.mockRejectedValueOnce(new Error("Diff file unavailable"));
+    windowTakeLaunch
+      .mockResolvedValueOnce({ files: [], diff: { left: "/p/a.txt", right: "/p/b.txt" } })
+      .mockResolvedValueOnce({ files: ["/p/next.cs"], positions: [{ path: "/p/next.cs", line: 8, column: 2 }] });
+    const first = consumeLaunch();
+    const next = consumeLaunch();
+    await expect(first).rejects.toThrow("Diff file unavailable");
+    await next;
+    expect(windowCompleteLaunch).toHaveBeenCalledTimes(2);
+    expect(opened).toEqual(["/p/next.cs (focus) @8:2"]);
+  });
+
+  it("applies a later source location after a pending diff", async () => {
+    useProject.setState(projectState("/p") as never);
+    windowTakeLaunch
+      .mockResolvedValueOnce({ files: [], diff: { left: "/p/a.txt", right: "/p/b.txt" } })
+      .mockResolvedValueOnce({ files: ["/p/next.cs"], positions: [{ path: "/p/next.cs", line: 8, column: 2 }] });
+    await Promise.all([consumeLaunch(), consumeLaunch()]);
+    expect(diffs).toHaveLength(1);
+    expect(opened).toEqual(["/p/next.cs (focus) @8:2"]);
+    expect(openDiff.mock.invocationCallOrder[0]).toBeLessThan(openEditorFile.mock.invocationCallOrder[0]!);
+    expect(windowCompleteLaunch).toHaveBeenCalledTimes(2);
+  });
+
+  it.each([
+    { project: null, diffBeforeFiles: false },
+    { project: "/p", diffBeforeFiles: false },
+    { project: null, diffBeforeFiles: true },
+    { project: "/p", diffBeforeFiles: true },
+  ])(
+    "preserves the last request's focus for $project (diff first: $diffBeforeFiles)",
+    async ({ project, diffBeforeFiles }) => {
+      windowTakeLaunch.mockResolvedValueOnce({
+        project,
+        diffBeforeFiles,
+        files: ["/p/a.cs"],
+        positions: [{ path: "/p/a.cs", line: 8, column: 2 }],
+        diff: { left: "/p/left.txt", right: "/p/right.txt" },
+      });
+      await consumeLaunch();
+      expect(opened).toEqual(["/p/a.cs (focus) @8:2"]);
+      expect(diffs).toHaveLength(1);
+      const fileCall = openEditorFile.mock.invocationCallOrder[0]!;
+      const diffCall = openDiff.mock.invocationCallOrder[0]!;
+      expect(diffBeforeFiles ? diffCall : fileCall).toBeLessThan(diffBeforeFiles ? fileCall : diffCall);
+      expect(windowCompleteLaunch).toHaveBeenCalledOnce();
+    },
+  );
+
+  it("opens later files when their preceding diff fails", async () => {
+    useProject.setState(projectState("/p") as never);
+    openDiff.mockRejectedValueOnce(new Error("Diff file unavailable"));
+    windowTakeLaunch.mockResolvedValueOnce({
+      files: ["/p/next.cs"],
+      positions: [{ path: "/p/next.cs", line: 8, column: 2 }],
+      diff: { left: "/p/a.txt", right: "/p/b.txt" },
+      diffBeforeFiles: true,
+    });
+    await consumeLaunch();
+    expect(opened).toEqual(["/p/next.cs (focus) @8:2"]);
+    expect(notify).toHaveBeenCalledWith(expect.objectContaining({ level: "error", body: "Diff file unavailable" }));
+    expect(windowCompleteLaunch).toHaveBeenCalledOnce();
+  });
+
+  it.each([false, true])(
+    "does not open a diff when its project launch is cancelled (diff first: %s)",
+    async (diffBeforeFiles) => {
+      openProject.mockRejectedValueOnce(new IpcError({ kind: "cancelled", message: "Project opening cancelled" }));
+      windowTakeLaunch.mockResolvedValueOnce({
+        project: "/p",
+        diffBeforeFiles,
+        files: ["/p/a.cs"],
+        diff: { left: "/p/left.txt", right: "/p/right.txt" },
+      });
+      await consumeLaunch();
+      expect(opened).toEqual([]);
+      expect(diffs).toEqual([]);
+      expect(lightEditOpen).not.toHaveBeenCalled();
+      expect(windowCompleteLaunch).toHaveBeenCalledOnce();
+    },
+  );
 });
 
 describe("Attach", () => {

src/features/app/windows.ts

@@ -65,10 +65,10 @@ async function openHere(path: string): Promise<boolean> {
   }
 }
 
-/** Opens `files` as editor tabs of this window, focusing the last. */
 /** Caret positions of files opened from the command line (`--line` / `--column`), 0-based. */
 type Positions = ReadonlyArray<{ path: string; line: number; column: number }>;
 
+/** Opens `files` as editor tabs of this window, focusing the last. */
 async function openInEditor(files: readonly string[], positions: Positions = []): Promise<void> {
   for (const [i, path] of files.entries()) {
     const at = positions.find((p) => p.path === path);
@@ -77,7 +77,7 @@ async function openInEditor(files: readonly string[], positions: Positions = [])
         focus: i === files.length - 1,
         ...(at ? { selection: { line: at.line, col: at.column } } : {}),
       })
-      .catch((e) => console.warn(`could not open ${path}`, e));
+      .catch((e) => openFailed(path, e));
   }
 }
 
@@ -213,25 +213,53 @@ async function pickFiles(title: string): Promise<string[]> {
 }
 
 /** Takes this window's pending launch (if any) and opens its project, files or diff pair. */
-export async function consumeLaunch(): Promise<void> {
+let launchQueue: Promise<void> = Promise.resolve();
+export function consumeLaunch(): Promise<void> {
+  const request = launchQueue.then(consumePendingLaunch);
+  launchQueue = request.catch(() => {});
+  return request;
+}
+
+async function consumePendingLaunch(): Promise<void> {
   if (!isTauri()) return;
   const launch = await commands.windowTakeLaunch().catch(() => null);
-  const files = launch?.files ?? [];
-  const positions = launch?.positions ?? [];
-  if (launch?.project) {
-    if (await openHere(launch.project)) await openInEditor(files, positions);
-  } else if (files.length) await openFilesHere(files, positions);
-  else if (launch?.diff) await openCliDiff(launch.diff);
-  else if (launch?.remote) {
-    const { target, path } = launch.remote;
-    await connectRemote(target, path).catch((e) =>
-      notifications().notify({
-        level: "error",
-        title: t("app.window.openFailed", { name: basename(path) }),
-        body: errorText(e),
-        key: `app.open:${path}`,
-      }),
-    );
+  if (!launch) return;
+  try {
+    // Session restoration may already be opening this window's project.
+    if (useProject.getState().opening) {
+      await new Promise<void>((resolve) => {
+        const off = useProject.subscribe((state) => {
+          if (!state.opening) {
+            off();
+            resolve();
+          }
+        });
+      });
+    }
+    const files = launch.files ?? [];
+    const positions = launch.positions ?? [];
+    if (launch.project && !(await openHere(launch.project))) return;
+    if (launch.diff && launch.diffBeforeFiles) {
+      const diff = launch.diff;
+      await openCliDiff(diff).catch((e) => openFailed(diff.right, e));
+    }
+    if (launch.project) {
+      await openInEditor(files, positions);
+    } else if (files.length) await openFilesHere(files, positions);
+    else if (launch.remote) {
+      const { target, path } = launch.remote;
+      await connectRemote(target, path).catch((e) =>
+        notifications().notify({
+          level: "error",
+          title: t("app.window.openFailed", { name: basename(path) }),
+          body: errorText(e),
+          key: `app.open:${path}`,
+        }),
+      );
+    }
+    if (launch.diff && !launch.diffBeforeFiles) await openCliDiff(launch.diff);
+  } finally {
+    await call(commands.windowCompleteLaunch());
   }
 }
 
@@ -517,14 +545,19 @@ export function installLaunchHandling(): () => void {
   if (!isTauri()) return () => {};
   let disposed = false;
   let unlisten: (() => void) | undefined;
-  const launch = consumeLaunch();
+  const launch = import("@tauri-apps/api/webviewWindow").then(async ({ getCurrentWebviewWindow }) => {
+    const off = await getCurrentWebviewWindow().listen(LAUNCH_EVENT, () => {
+      if (!disposed) void consumeLaunch().catch(console.warn);
+    });
+    if (disposed) off();
+    else {
+      unlisten = off;
+      // Subscribe before taking pending files so launches during startup aren't missed.
+      await consumeLaunch();
+    }
+  });
   provideStartupLaunch(launch);
   holdSplash(launch);
-  void import("@tauri-apps/api/webviewWindow").then(async ({ getCurrentWebviewWindow }) => {
-    const off = await getCurrentWebviewWindow().listen(LAUNCH_EVENT, () => void consumeLaunch());
-    if (disposed) off();
-    else unlisten = off;
-  });
   return () => {
     disposed = true;
     unlisten?.();