Merge branch 'development' into releases/3.4.16

Author: tidy-dev

.github/workflows/ci.yml

@@ -38,7 +38,7 @@ on:
       APPLE_APPLICATION_CERT_PASSWORD:
 
 env:
-  NODE_VERSION: 20.17.0
+  NODE_VERSION: 20.18.1
 
 jobs:
   lint:

.node-version

@@ -1 +1 @@
-20.17.0
+20.18.1

.nvmrc

@@ -1 +1 @@
-v20.17.0
+v20.18.1

.tool-versions

@@ -1,2 +1,2 @@
 python 3.9.5
-nodejs 20.17.0
+nodejs 20.18.1

app/.npmrc

@@ -1,3 +1,3 @@
 runtime = electron
 disturl = https://electronjs.org/headers
-target = 32.1.2
+target = 34.0.1

app/package.json

@@ -33,7 +33,7 @@
     "desktop-trampoline": "desktop/desktop-trampoline#v0.9.10",
     "dexie": "^3.2.2",
     "dompurify": "^2.5.4",
-    "dugite": "3.0.0-rc6",
+    "dugite": "3.0.0-rc10",
     "electron-window-state": "^5.0.3",
     "event-kit": "^2.0.0",
     "focus-trap-react": "^8.1.0",
@@ -72,7 +72,7 @@
     "winston": "^3.6.0"
   },
   "devDependencies": {
-    "electron-devtools-installer": "^3.2.1",
+    "electron-devtools-installer": "^4.0.0",
     "temp": "^0.8.3",
     "webpack-hot-middleware": "^2.10.0"
   }

app/src/cli/main.ts

@@ -1,21 +1,31 @@
 import { join, resolve } from 'path'
 import parse from 'minimist'
-import { execFile, ExecFileException } from 'child_process'
+import { execFile, spawn } from 'child_process'
 
 const run = (...args: Array<string>) => {
-  function cb(e: ExecFileException | null, stderr: string) {
+  function cb(e: unknown | null, stderr?: string) {
     if (e) {
       console.error(`Error running command ${args}`)
-      console.error(stderr)
-      process.exit(e.code)
+      console.error(stderr ?? `${e}`)
+      process.exit(
+        typeof e === 'object' && 'code' in e && typeof e.code === 'number'
+          ? e.code
+          : 1
+      )
     }
   }
 
   if (process.platform === 'darwin') {
     execFile('open', ['-n', join(__dirname, '../../..'), '--args', ...args], cb)
   } else if (process.platform === 'win32') {
     const exeName = `GitHubDesktop${__DEV__ ? '-dev' : ''}.exe`
-    execFile(join(__dirname, `../../${exeName}`), args, cb)
+    spawn(join(__dirname, `../../${exeName}`), args, {
+      detached: true,
+      stdio: 'ignore',
+    })
+      .on('error', cb)
+      .on('exit', code => (process.exitCode = code ?? process.exitCode))
+      .unref()
   } else {
     throw new Error('Unsupported platform')
   }

app/src/lib/custom-integration.ts

@@ -1,8 +1,7 @@
-import { ChildProcess, SpawnOptions, spawn } from 'child_process'
 import { parseCommandLineArgv } from 'windows-argv-parser'
 import stringArgv from 'string-argv'
 import { promisify } from 'util'
-import { exec } from 'child_process'
+import { exec, spawn, SpawnOptions } from 'child_process'
 import { access, lstat } from 'fs/promises'
 import * as fs from 'fs'
 
@@ -118,7 +117,9 @@ export async function validateCustomIntegrationPath(
 
     return { isValid: isExecutableFile || !!bundleID, bundleID }
   } catch (e) {
-    log.error(`Failed to validate path: ${path}`, e)
+    if (e.code !== 'ENOENT') {
+      log.error(`Failed to validate path: ${path}`, e)
+    }
     return { isValid: false }
   }
 }
@@ -183,15 +184,13 @@ export function migratedCustomIntegration(
  * on Windows, where we need to wrap the command and arguments in quotes when
  * the shell option is enabled.
  *
- * @param command Command to spawn
+ * @param cmd Command to spawn
  * @param args Arguments to pass to the command
  * @param options Options to pass to spawn (optional)
  * @returns The ChildProcess object returned by spawn
  */
-export function spawnCustomIntegration(
-  command: string,
+export const spawnCustomIntegration = (
+  cmd: string,
   args: readonly string[],
-  options?: SpawnOptions
-): ChildProcess {
-  return options ? spawn(command, args, options) : spawn(command, args)
-}
+  opts?: SpawnOptions
+) => spawn(cmd, args, { stdio: 'ignore', detached: true, ...opts })

app/src/lib/editors/launch.ts

@@ -5,116 +5,82 @@ import {
   expandTargetPathArgument,
   ICustomIntegration,
   parseCustomIntegrationArguments,
-  spawnCustomIntegration,
 } from '../custom-integration'
 
-/**
- * Open a given file or folder in the desired external editor.
- *
- * @param fullPath A folder or file path to pass as an argument when launching the editor.
- * @param editor The external editor to launch.
- */
-export async function launchExternalEditor(
-  fullPath: string,
-  editor: FoundEditor
-): Promise<void> {
-  const editorPath = editor.path
+async function launchEditor(
+  editorPath: string,
+  args: readonly string[],
+  editorName: string,
+  spawnAsDarwinApp: boolean
+) {
   const exists = await pathExists(editorPath)
   const label = __DARWIN__ ? 'Settings' : 'Options'
   if (!exists) {
     throw new ExternalEditorError(
-      `Could not find executable for '${editor.editor}' at path '${editor.path}'.  Please open ${label} and select an available editor.`,
+      `Could not find executable for ${editorName} at path '${editorPath}'. Please open ${label} and select an available editor.`,
       { openPreferences: true }
     )
   }
 
-  const opts: SpawnOptions = {
-    // Make sure the editor processes are detached from the Desktop app.
-    // Otherwise, some editors (like Notepad++) will be killed when the
-    // Desktop app is closed.
-    detached: true,
-  }
-
-  try {
-    if (__DARWIN__) {
-      // In macOS we can use `open`, which will open the right executable file
-      // for us, we only need the path to the editor .app folder.
-      spawn('open', ['-a', editorPath, fullPath], opts)
-    } else {
-      spawn(editorPath, [fullPath], opts)
+  return new Promise<void>((resolve, reject) => {
+    const opts: SpawnOptions = {
+      // Make sure the editor processes are detached from the Desktop app.
+      // Otherwise, some editors (like Notepad++) will be killed when the
+      // Desktop app is closed.
+      detached: true,
+      stdio: 'ignore',
     }
-  } catch (error) {
-    log.error(`Error while launching ${editor.editor}`, error)
-    if (error?.code === 'EACCES') {
-      throw new ExternalEditorError(
-        `GitHub Desktop doesn't have the proper permissions to start '${editor.editor}'. Please open ${label} and try another editor.`,
-        { openPreferences: true }
-      )
-    } else {
-      throw new ExternalEditorError(
-        `Something went wrong while trying to start '${editor.editor}'. Please open ${label} and try another editor.`,
-        { openPreferences: true }
-      )
-    }
-  }
+
+    const child = spawnAsDarwinApp
+      ? spawn('open', ['-a', editorPath, ...args], opts)
+      : spawn(editorPath, args, opts)
+
+    child.on('error', reject)
+    child.on('spawn', resolve)
+    child.unref() // Don't wait for editor to exit
+  }).catch((e: unknown) => {
+    log.error(
+      `Error while launching ${editorName}`,
+      e instanceof Error ? e : undefined
+    )
+    throw new ExternalEditorError(
+      e && typeof e === 'object' && 'code' in e && e.code === 'EACCES'
+        ? `GitHub Desktop doesn't have the proper permissions to start ${editorName}. Please open ${label} and try another editor.`
+        : `Something went wrong while trying to start ${editorName}. Please open ${label} and try another editor.`,
+      { openPreferences: true }
+    )
+  })
 }
 
+/**
+ * Open a given file or folder in the desired external editor.
+ *
+ * @param fullPath A folder or file path to pass as an argument when launching the editor.
+ * @param editor The external editor to launch.
+ */
+export const launchExternalEditor = (fullPath: string, editor: FoundEditor) =>
+  launchEditor(editor.path, [fullPath], `'${editor.editor}'`, __DARWIN__)
+
 /**
  * Open a given file or folder in the desired custom external editor.
  *
  * @param fullPath A folder or file path to pass as an argument when launching the editor.
  * @param customEditor The external editor to launch.
  */
-export async function launchCustomExternalEditor(
+export const launchCustomExternalEditor = (
   fullPath: string,
   customEditor: ICustomIntegration
-): Promise<void> {
-  const editorPath = customEditor.path
-  const exists = await pathExists(editorPath)
-  const label = __DARWIN__ ? 'Settings' : 'Options'
-  if (!exists) {
-    throw new ExternalEditorError(
-      `Could not find executable for custom editor at path '${customEditor.path}'.  Please open ${label} and select an available editor.`,
-      { openPreferences: true }
-    )
-  }
-
-  const opts: SpawnOptions = {
-    // Make sure the editor processes are detached from the Desktop app.
-    // Otherwise, some editors (like Notepad++) will be killed when the
-    // Desktop app is closed.
-    detached: true,
-  }
-
+) => {
   const argv = parseCustomIntegrationArguments(customEditor.arguments)
 
   // Replace instances of RepoPathArgument with fullPath in customEditor.arguments
   const args = expandTargetPathArgument(argv, fullPath)
 
-  try {
-    if (__DARWIN__ && customEditor.bundleID) {
-      // In macOS we can use `open` if it's an app (i.e. if we have a bundleID),
-      // which will open the right executable file for us, we only need the path
-      // to the editor .app folder.
-      spawnCustomIntegration('open', ['-a', editorPath, ...args], opts)
-    } else {
-      spawnCustomIntegration(editorPath, args, opts)
-    }
-  } catch (error) {
-    log.error(
-      `Error while launching custom editor at path ${customEditor.path} with arguments ${args}`,
-      error
-    )
-    if (error?.code === 'EACCES') {
-      throw new ExternalEditorError(
-        `GitHub Desktop doesn't have the proper permissions to start custom editor at path ${customEditor.path}. Please open ${label} and try another editor.`,
-        { openPreferences: true }
-      )
-    } else {
-      throw new ExternalEditorError(
-        `Something went wrong while trying to start custom editor at path ${customEditor.path}. Please open ${label} and try another editor.`,
-        { openPreferences: true }
-      )
-    }
-  }
+  // In macOS we can use `open` if it's an app (i.e. if we have a bundleID),
+  // which will open the right executable file for us, we only need the path
+  // to the editor .app folder.
+  const spawnAsDarwinApp = __DARWIN__ && customEditor.bundleID !== undefined
+  const editorName = `custom editor at path '${customEditor.path}'`
+
+  return launchEditor(customEditor.path, args, editorName, spawnAsDarwinApp)
 }

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

@@ -151,29 +151,6 @@ export class AppWindow {
       autoUpdater.removeAllListeners()
       terminateDesktopNotifications()
     })
-
-    if (__WIN32__) {
-      // workaround for known issue with fullscreen-ing the app and restoring
-      // is that some Chromium API reports the incorrect bounds, so that it
-      // will leave a small space at the top of the screen on every other
-      // maximize
-      //
-      // adapted from https://github.com/electron/electron/issues/12971#issuecomment-403956396
-      //
-      // can be tidied up once https://github.com/electron/electron/issues/12971
-      // has been confirmed as resolved
-      this.window.once('ready-to-show', () => {
-        this.window.on('unmaximize', () => {
-          setTimeout(() => {
-            const bounds = this.window.getBounds()
-            bounds.width += 1
-            this.window.setBounds(bounds)
-            bounds.width -= 1
-            this.window.setBounds(bounds)
-          }, 5)
-        })
-      })
-    }
   }
 
   public load() {
@@ -226,7 +203,7 @@ export class AppWindow {
     registerWindowStateChangedEvents(this.window)
     this.window.loadURL(encodePathAsUrl(__dirname, 'index.html'))
 
-    nativeTheme.addListener('updated', (event: string, userInfo: any) => {
+    nativeTheme.addListener('updated', () => {
       ipcWebContents.send(this.window.webContents, 'native-theme-updated')
     })