Send cli open command to correct window if multiple windows are open

Closes #177

Author: pol-rivero

app/src/lib/ipc-shared.ts

@@ -73,6 +73,7 @@ export type RequestChannels = {
   'quit-app': () => void
   'open-repository-in-new-window': (path: string) => void
   'set-window-title': (title: string) => void
+  'set-window-selected-repository': (path: string | null) => void
   'minimize-window': () => void
   'maximize-window': () => void
   'unmaximize-window': () => void

app/src/main-process/app-window.ts

@@ -37,6 +37,14 @@ export class AppWindow {
   private _rendererReadyTime: number | null = null
   private isDownloadingUpdate: boolean = false
 
+  /**
+   * The path of the repository currently selected in this window, as reported
+   * by the renderer, or null if no repository is selected. Used to route
+   * `open-repository` actions to the window that already has the repository
+   * open.
+   */
+  private _selectedRepositoryPath: string | null = null
+
   private minWidth = 960
   private minHeight = 660
 
@@ -331,6 +339,19 @@ export class AppWindow {
     this.window.setTitle(title)
   }
 
+  /** The path of the repository currently selected in this window, if any. */
+  public get selectedRepositoryPath(): string | null {
+    return this._selectedRepositoryPath
+  }
+
+  public setSelectedRepositoryPath(path: string | null) {
+    this._selectedRepositoryPath = path
+  }
+
+  public hasSelectedRepositoryPath(): this is AppWindowWithSelectedRepository {
+    return this.isLoaded && this.selectedRepositoryPath !== null
+  }
+
   /** Selects all the windows web contents */
   public selectAllWindowContents() {
     this.window.webContents.selectAll()
@@ -609,6 +630,10 @@ export class AppWindow {
   }
 }
 
+export type AppWindowWithSelectedRepository = AppWindow & {
+  selectedRepositoryPath: string
+}
+
 const trySetUpdaterGuid = async (url: string) => {
   try {
     const id = await getUpdaterGUID()

app/src/main-process/main.ts

@@ -11,6 +11,7 @@ import {
   WebContents,
 } from 'electron'
 import * as Fs from 'fs'
+import * as Path from 'path'
 
 import { AppWindow } from './app-window'
 import { buildDefaultMenu, getAllMenuItems } from './menu'
@@ -189,7 +190,34 @@ function getTargetWindow() {
     return focusedAppWindow
   }
 
-  return getAppWindows()[0] ?? null
+  return getAppWindows().at(0) ?? null
+}
+
+function normalizeRepositoryPath(path: string) {
+  // Strip trailing separator
+  const normalized = Path.normalize(path).replace(/[\\/]+$/, '')
+  // Windows paths are case-insensitive
+  return __WIN32__ ? normalized.toLowerCase() : normalized
+}
+
+function findWindowForRepositoryPath(rawTargetPath: string): AppWindow | null {
+  const targetPath = normalizeRepositoryPath(rawTargetPath)
+  const allWindows = getAppWindows().filter(w => w.hasSelectedRepositoryPath())
+  const windowsSortedFromMostSpecificToLeast = allWindows.sort(
+    (a, b) => b.selectedRepositoryPath.length - a.selectedRepositoryPath.length
+  )
+
+  for (const window of windowsSortedFromMostSpecificToLeast) {
+    const candidatePath = normalizeRepositoryPath(window.selectedRepositoryPath)
+    if (
+      targetPath === candidatePath ||
+      targetPath.startsWith(candidatePath + Path.sep)
+    ) {
+      return window
+    }
+  }
+
+  return null // fallback to getLoadedTargetWindow()
 }
 
 function getLoadedTargetWindow() {
@@ -250,22 +278,23 @@ if (!handlingSquirrelEvent) {
   const gotSingleInstanceLock = app.requestSingleInstanceLock()
   isDuplicateInstance = !gotSingleInstanceLock
 
-  app.on('second-instance', (event, args, workingDirectory) => {
-    // Someone tried to run a second instance, we should focus our window.
-    const targetWindow = getTargetWindow()
-    if (targetWindow) {
-      if (targetWindow.isMinimized()) {
-        targetWindow.restore()
+  app.on('second-instance', async (event, args, workingDirectory) => {
+    const handledAction = await handleCommandLineArguments(args)
+    if (handledAction) {
+      return
+    }
+    const mainWindow = getTargetWindow()
+    if (mainWindow) {
+      if (mainWindow.isMinimized()) {
+        mainWindow.restore()
       }
 
-      if (!targetWindow.isVisible()) {
-        targetWindow.show()
+      if (!mainWindow.isVisible()) {
+        mainWindow.show()
       }
 
-      targetWindow.focus()
+      mainWindow.focus()
     }
-
-    handleCommandLineArguments(args)
   })
 
   if (isDuplicateInstance) {
@@ -311,7 +340,7 @@ if (__DARWIN__) {
   })
 }
 
-async function handleCommandLineArguments(argv: string[]) {
+async function handleCommandLineArguments(argv: string[]): Promise<boolean> {
   const args = parseCommandLineArgs(argv, {
     boolean: ['protocol-launcher'],
   })
@@ -347,30 +376,41 @@ async function handleCommandLineArguments(argv: string[]) {
 
     if (matchingUrl) {
       handleAppURL(matchingUrl)
-      return
+      return true
     } else if (__WIN32__) {
       log.error(`Encountered --protocol-launcher without app url`)
-      return
+      return false
     }
     // If --protocol-launcher is present we always want to bail and not
     // risk a smuggled cli switch
   }
 
   if (typeof args['cli-open'] === 'string') {
     handleCLIAction({ kind: 'open-repository', path: args['cli-open'] })
+    return true
   } else if (typeof args['cli-clone'] === 'string') {
     handleCLIAction({
       kind: 'clone-url',
       url: args['cli-clone'],
       branch:
         typeof args['cli-branch'] === 'string' ? args['cli-branch'] : undefined,
     })
+    return true
   }
 
-  return
+  return false
 }
 
 function handleCLIAction(action: CLIAction) {
+  if (action.kind === 'open-repository') {
+    const existingWindow = findWindowForRepositoryPath(action.path)
+    if (existingWindow !== null) {
+      existingWindow.revealAndFocus()
+      existingWindow.sendCLIAction(action)
+      return
+    }
+  }
+
   onDidLoad(window => {
     // This manual focus call _shouldn't_ be necessary, but is for Chrome on
     // macOS. See https://github.com/desktop/desktop/issues/973.
@@ -623,6 +663,10 @@ app.on('ready', () => {
     getAppWindowFromWebContents(event.sender)?.setTitle(title)
   )
 
+  ipcMain.on('set-window-selected-repository', (event, path: string | null) =>
+    getAppWindowFromWebContents(event.sender)?.setSelectedRepositoryPath(path)
+  )
+
   ipcMain.on('minimize-window', event =>
     getAppWindowFromWebContents(event.sender)?.minimizeWindow()
   )

app/src/ui/app.tsx

@@ -73,6 +73,7 @@ import {
   uninstallWindowsCLI,
   openRepositoryInNewWindow,
   setWindowTitle,
+  setWindowSelectedRepository,
 } from './main-process-proxy'
 import { DiscardChanges } from './discard-changes'
 import { Welcome } from './welcome'
@@ -1120,12 +1121,20 @@ export class App extends React.Component<IAppProps, IAppState> {
     })
 
     this.updateWindowTitle()
+    this.updateSelectedRepository()
   }
 
   public componentDidUpdate(prevProps: IAppProps, prevState: IAppState): void {
     if (this.getWindowTitle(prevState) !== this.getWindowTitle()) {
       this.updateWindowTitle()
     }
+
+    if (
+      this.getSelectedRepositoryPath(prevState) !==
+      this.getSelectedRepositoryPath()
+    ) {
+      this.updateSelectedRepository()
+    }
   }
 
   private onDocumentFocus = (event: FocusEvent) => {
@@ -1149,6 +1158,16 @@ export class App extends React.Component<IAppProps, IAppState> {
     setWindowTitle(this.getWindowTitle())
   }
 
+  private getSelectedRepositoryPath(
+    state: IAppState = this.state
+  ): string | null {
+    return state.selectedState?.repository.path ?? null
+  }
+
+  private updateSelectedRepository() {
+    setWindowSelectedRepository(this.getSelectedRepositoryPath())
+  }
+
   /**
    * Manages keyboard shortcuts specific to macOS.
    * - adds Shift+F10 to open the context menus (like on Windows so macOS

app/src/ui/main-process-proxy.ts

@@ -182,6 +182,16 @@ export const openRepositoryInNewWindow = sendProxy(
 /** Tell the main process to update the current window title */
 export const setWindowTitle = sendProxy('set-window-title', 1)
 
+/**
+ * Tell the main process which repository (if any) is currently selected in this
+ * window. This allows the main process to route CLI/second-instance
+ * `open-repository` actions to the window that already has the repository open.
+ */
+export const setWindowSelectedRepository = sendProxy(
+  'set-window-selected-repository',
+  1
+)
+
 /** Subscribes to auto updater error events originating from the main process */
 export function onAutoUpdaterError(
   errorHandler: (evt: Electron.IpcRendererEvent, error: Error) => void

app/test/unit/ipc-contract-test.ts

@@ -71,6 +71,7 @@ describe('IPC channel contract', () => {
     'uninstall-windows-cli',
     'open-repository-in-new-window',
     'set-window-title',
+    'set-window-selected-repository',
     'restart-app',
   ] as const