Merge branch 'development' into if-it-smells-like-a-bundle-and-looks-like-a-bundle

app/package.json

@@ -3,7 +3,7 @@
   "productName": "GitHub Desktop",
   "bundleID": "com.github.GitHubClient",
   "companyName": "GitHub, Inc.",
-  "version": "2.9.11-beta1",
+  "version": "2.9.11",
   "main": "./main.js",
   "repository": {
     "type": "git",
@@ -34,7 +34,6 @@
     "dugite": "^1.104.0",
     "electron-window-state": "^5.0.3",
     "event-kit": "^2.0.0",
-    "file-url": "^2.0.2",
     "focus-trap-react": "^8.1.0",
     "fs-admin": "^0.19.0",
     "fs-extra": "^9.0.1",

app/src/lib/directory-exists.ts

@@ -0,0 +1,14 @@
+import { stat } from 'fs/promises'
+
+/**
+ * Helper method to stat a path and check both that it exists and that it's
+ * a directory.
+ */
+export const directoryExists = async (path: string) => {
+  try {
+    const s = await stat(path)
+    return s.isDirectory()
+  } catch (e) {
+    return false
+  }
+}

app/src/lib/exec-file.ts

@@ -0,0 +1,10 @@
+import { execFile as execFileOrig } from 'child_process'
+import { promisify } from 'util'
+
+/**
+ * A version of execFile which returns a Promise rather than the traditional
+ * callback approach of `child_process.execFile`.
+ *
+ * See `child_process.execFile` for more information
+ */
+export const execFile = promisify(execFileOrig)

app/src/lib/git/rebase.ts

@@ -104,7 +104,7 @@ export async function getRebaseInternalState(
     )
 
     if (targetBranch.startsWith('refs/heads/')) {
-      targetBranch = targetBranch.substr(11).trim()
+      targetBranch = targetBranch.substring(11).trim()
     }
 
     baseBranchTip = await FSE.readFile(

app/src/lib/git/rev-parse.ts

@@ -2,6 +2,7 @@ import * as Path from 'path'
 
 import { git } from './core'
 import { RepositoryDoesNotExistErrorCode } from 'dugite'
+import { directoryExists } from '../directory-exists'
 
 /**
  * Get the absolute path to the top level working directory.
@@ -80,3 +81,41 @@ export async function isBareRepository(path: string): Promise<boolean> {
 export async function isGitRepository(path: string): Promise<boolean> {
   return (await getTopLevelWorkingDirectory(path)) !== null
 }
+
+/**
+ * Attempts to fulfill the work of isGitRepository and isBareRepository while
+ * requiring only one Git process to be spawned.
+ *
+ * Returns 'bare', 'regular', or 'missing' if the repository couldn't be
+ * found.
+ */
+export async function getRepositoryType(
+  path: string
+): Promise<'bare' | 'regular' | 'missing'> {
+  if (!(await directoryExists(path))) {
+    return 'missing'
+  }
+
+  try {
+    const result = await git(
+      ['rev-parse', '--is-bare-repository'],
+      path,
+      'getRepositoryType',
+      { successExitCodes: new Set([0, 128]) }
+    )
+
+    if (result.exitCode === 0) {
+      return result.stdout.trim() === 'true' ? 'bare' : 'regular'
+    }
+    return 'missing'
+  } catch (err) {
+    // This could theoretically mean that the Git executable didn't exist but
+    // in reality it's almost always going to be that the process couldn't be
+    // launched inside of `path` meaning it didn't exist. This would constitute
+    // a race condition given that we stat the path before executing Git.
+    if (err.code === 'ENOENT') {
+      return 'missing'
+    }
+    throw err
+  }
+}

app/src/lib/http.ts

@@ -84,9 +84,9 @@ async function deserialize<T>(response: Response): Promise<T> {
  * @param path The resource path (should be relative to the root of the server)
  */
 export function getAbsoluteUrl(endpoint: string, path: string): string {
-  let relativePath = path[0] === '/' ? path.substr(1) : path
+  let relativePath = path[0] === '/' ? path.substring(1) : path
   if (relativePath.startsWith('api/v3/')) {
-    relativePath = relativePath.substr(7)
+    relativePath = relativePath.substring(7)
   }
 
   // Our API endpoints are a bit sloppy in that they don't typically

app/src/lib/is-git-on-path.ts

@@ -1,70 +1,33 @@
-import { spawn, SpawnOptionsWithoutStdio } from 'child_process'
 import * as Path from 'path'
+import { execFile } from './exec-file'
 
-function captureCommandOutput(
-  command: string,
-  args: string[],
-  options: SpawnOptionsWithoutStdio = {}
-): Promise<string | undefined> {
-  return new Promise<string | undefined>((resolve, reject) => {
-    const cp = spawn(command, args, options)
-
-    cp.on('error', error => {
-      log.warn(`Unable to spawn ${command}`, error)
-      resolve(undefined)
-    })
-
-    const chunks = new Array<Buffer>()
-    let total = 0
-
-    cp.stdout.on('data', (chunk: Buffer) => {
-      chunks.push(chunk)
-      total += chunk.length
-    })
-
-    cp.on('close', function (code) {
-      if (code !== 0) {
-        resolve(undefined)
-      } else {
-        resolve(
-          Buffer.concat(chunks, total)
-            .toString()
-            .replace(/\r?\n[^]*/m, '')
-        )
-      }
-    })
-  })
-}
-
-export function findGitOnPath(): Promise<string | undefined> {
-  // adapted from http://stackoverflow.com/a/34953561/1363815
+const findOnPath = (program: string) => {
   if (__WIN32__) {
-    const windowsRoot = process.env.SystemRoot || 'C:\\Windows'
-    const wherePath = Path.join(windowsRoot, 'System32', 'where.exe')
-
-    // `where` will list _all_ PATH components where the executable
-    // is found, one per line, and return 0, or print an error and
-    // return 1 if it cannot be found
-    log.info(`calling captureCommandOutput(where git)`)
-    return captureCommandOutput(wherePath, ['git'], { cwd: windowsRoot })
+    const cwd = process.env.SystemRoot || 'C:\\Windows'
+    const cmd = Path.join(cwd, 'System32', 'where.exe')
+    return execFile(cmd, [program], { cwd })
   }
-
-  if (__DARWIN__ || __LINUX__) {
-    // `which` will print the path and return 0 when the executable
-    // is found under PATH, or return 1 if it cannot be found
-    return captureCommandOutput('which', ['git'])
-  }
-
-  return Promise.resolve(undefined)
+  return execFile('which', [program])
 }
 
-export async function isGitOnPath(): Promise<boolean> {
+/** Attempts to locate the path to the system version of Git */
+export const findGitOnPath = () =>
+  // `where` (i.e on Windows) will list _all_ PATH components where the
+  // executable is found, one per line, and return 0, or print an error and
+  // return 1 if it cannot be found.
+  //
+  // `which` (i.e. on macOS and Linux) will print the path and return 0
+  // when the executable is found under PATH, or return 1 if it cannot be found
+  findOnPath('git')
+    .then(({ stdout }) => stdout.split(/\r?\n/, 1)[0])
+    .catch(err => {
+      log.warn(`Failed trying to find Git on PATH`, err)
+      return undefined
+    })
+
+/** Returns a value indicating whether Git was found in the system's PATH */
+export const isGitOnPath = async () =>
   // Modern versions of macOS ship with a Git shim that guides you through
   // the process of setting everything up. We trust this is available, so
   // don't worry about looking for it here.
-  if (__DARWIN__) {
-    return Promise.resolve(true)
-  }
-
-  return (await findGitOnPath()) !== undefined
-}
+  __DARWIN__ || (await findGitOnPath()) !== undefined

app/src/lib/parse-app-url.ts

@@ -101,7 +101,7 @@ export function parseAppURL(url: string): URLActionType {
   }
 
   // Trim the trailing / from the URL
-  const parsedPath = pathName.substr(1)
+  const parsedPath = pathName.substring(1)
 
   if (actionName === 'openrepo') {
     const pr = getQueryStringValue(query, 'pr')

app/src/lib/patch-formatter.ts

@@ -191,7 +191,7 @@ export function formatPatch(
         // is concerned which means that we should treat it as if it's still
         // in the old file so we'll convert it to a context line.
         if (line.type === DiffLineType.Delete) {
-          hunkBuf += ` ${line.text.substr(1)}\n`
+          hunkBuf += ` ${line.text.substring(1)}\n`
           oldCount++
           newCount++
         } else {
@@ -280,10 +280,10 @@ export function formatPatchToDiscardChanges(
       } else if (selection.isSelected(absoluteIndex)) {
         // Reverse the change (if it was an added line, treat it as removed and vice versa).
         if (line.type === DiffLineType.Add) {
-          hunkBuf += `-${line.text.substr(1)}\n`
+          hunkBuf += `-${line.text.substring(1)}\n`
           newCount++
         } else if (line.type === DiffLineType.Delete) {
-          hunkBuf += `+${line.text.substr(1)}\n`
+          hunkBuf += `+${line.text.substring(1)}\n`
           oldCount++
         } else {
           assertNever(line.type, `Unsupported line type ${line.type}`)
@@ -296,7 +296,7 @@ export function formatPatchToDiscardChanges(
           // so we just print it untouched on the diff.
           oldCount++
           newCount++
-          hunkBuf += ` ${line.text.substr(1)}\n`
+          hunkBuf += ` ${line.text.substring(1)}\n`
         } else if (line.type === DiffLineType.Delete) {
           // An unselected removed line has no impact on this patch since it's not
           // found on the current working copy of the file, so we can ignore it.

app/src/lib/path.ts

@@ -1,16 +1,14 @@
 import * as Path from 'path'
-import fileUrl from 'file-url'
 import { realpath } from 'fs-extra'
+import { pathToFileURL } from 'url'
 
 /**
  * Resolve and encode the path information into a URL.
  *
  * @param pathSegments array of path segments to resolve
  */
-export function encodePathAsUrl(...pathSegments: string[]): string {
-  const path = Path.resolve(...pathSegments)
-  return fileUrl(path)
-}
+export const encodePathAsUrl = (...pathSegments: string[]) =>
+  pathToFileURL(Path.resolve(...pathSegments)).toString()
 
 /**
  * Resolve one or more path sequences into an absolute path underneath

app/src/lib/progress/git.ts

@@ -253,8 +253,8 @@ export function parse(line: string): IGitProgressInfo | null {
     return null
   }
 
-  const title = line.substr(0, titleLength)
-  const progressText = line.substr(title.length + 2).trim()
+  const title = line.substring(0, titleLength)
+  const progressText = line.substring(title.length + 2).trim()
 
   if (!progressText.length) {
     return null

app/src/lib/status-parser.ts

@@ -72,11 +72,11 @@ export function parsePorcelainStatus(
 
   while ((field = queue.shift())) {
     if (field.startsWith('# ') && field.length > 2) {
-      entries.push({ kind: 'header', value: field.substr(2) })
+      entries.push({ kind: 'header', value: field.substring(2) })
       continue
     }
 
-    const entryKind = field.substr(0, 1)
+    const entryKind = field.substring(0, 1)
 
     if (entryKind === ChangedEntryType) {
       entries.push(parseChangedEntry(field))
@@ -159,7 +159,7 @@ function parseUnmergedEntry(field: string): IStatusEntry {
 }
 
 function parseUntrackedEntry(field: string): IStatusEntry {
-  const path = field.substr(2)
+  const path = field.substring(2)
   return {
     kind: 'entry',
     // NOTE: We return ?? instead of ? here to play nice with mapStatus,

app/src/lib/stores/git-store.ts

@@ -191,7 +191,7 @@ export class GitStore extends BaseStore {
       log.debug(
         `reconciling history - adding ${
           commits.length
-        } commits before merge base ${mergeBase.substr(0, 8)}`
+        } commits before merge base ${mergeBase.substring(0, 8)}`
       )
 
       // rebuild the local history state by combining the commits _before_ the
@@ -528,7 +528,7 @@ export class GitStore extends BaseStore {
         // strip out everything related to the remote because this
         // is likely to be a tracked branch locally
         // e.g. `main`, `develop`, etc
-        return match.substr(remoteNamespace.length)
+        return match.substring(remoteNamespace.length)
       }
     }
 

app/src/lib/stores/helpers/branch-pruner.ts

@@ -226,7 +226,7 @@ export class BranchPruner {
         continue
       }
 
-      const branchName = branchCanonicalRef.substr(branchRefPrefix.length)
+      const branchName = branchCanonicalRef.substring(branchRefPrefix.length)
 
       if (options.deleteBranch) {
         const isDeleted = await gitStore.performFailableOperation(() =>

app/src/lib/text-token-parser.ts

@@ -162,7 +162,7 @@ export class Tokenizer {
     }
 
     this.flush()
-    const id = parseInt(maybeIssue.substr(1), 10)
+    const id = parseInt(maybeIssue.substring(1), 10)
     if (isNaN(id)) {
       return null
     }
@@ -198,7 +198,7 @@ export class Tokenizer {
     }
 
     this.flush()
-    const name = maybeMention.substr(1)
+    const name = maybeMention.substring(1)
     const url = `${getHTMLURL(repository.endpoint)}/${name}`
     this._results.push({ kind: TokenType.Link, text: maybeMention, url })
     return { nextIndex }

app/src/lib/wrap-rich-text-commit-message.ts

@@ -67,8 +67,8 @@ export function wrapRichTextCommitMessage(
         // We always hard-wrap text, it'd be nice if we could attempt
         // to break at word boundaries in the future but that's too
         // complex for now.
-        summary.push(text(token.text.substr(0, remainder)))
-        overflow.push(text(token.text.substr(remainder)))
+        summary.push(text(token.text.substring(0, remainder)))
+        overflow.push(text(token.text.substring(remainder)))
       } else if (token.kind === TokenType.Emoji) {
         // Can't hard-wrap inside an emoji
         overflow.push(token)
@@ -79,8 +79,8 @@ export function wrapRichTextCommitMessage(
         // text showing otherwise we'll end up with weird links like "h"
         // or "@"
         if (!token.text.startsWith('#') && remainder > 5) {
-          summary.push(link(token.text.substr(0, remainder), token.text))
-          overflow.push(link(token.text.substr(remainder), token.text))
+          summary.push(link(token.text.substring(0, remainder), token.text))
+          overflow.push(link(token.text.substring(remainder), token.text))
         } else {
           overflow.push(token)
         }

app/src/models/diff/diff-line.ts

@@ -36,6 +36,6 @@ export class DiffLine {
 
   /** The content of the line, i.e., without the line type marker. */
   public get content(): string {
-    return this.text.substr(1)
+    return this.text.substring(1)
   }
 }

app/src/ui/add-repository/add-existing-repository.tsx

@@ -1,8 +1,7 @@
 import * as React from 'react'
 import * as Path from 'path'
 import { Dispatcher } from '../dispatcher'
-import { isGitRepository } from '../../lib/git'
-import { isBareRepository } from '../../lib/git'
+import { getRepositoryType } from '../../lib/git'
 import { Button } from '../lib/button'
 import { TextBox } from '../lib/text-box'
 import { Row } from '../lib/row'
@@ -71,28 +70,39 @@ export class AddExistingRepository extends React.Component<
   }
 
   public async componentDidMount() {
-    const pathToCheck = this.state.path
-    // We'll only have a path at this point if the dialog was opened with a path
-    // to prefill.
-    if (pathToCheck.length < 1) {
+    const { path } = this.state
+
+    if (path.length !== 0) {
+      await this.validatePath(path)
+    }
+  }
+
+  private async updatePath(path: string) {
+    this.setState({ path, isRepository: false })
+    await this.validatePath(path)
+  }
+
+  private async validatePath(path: string) {
+    if (path.length === 0) {
+      this.setState({
+        isRepository: false,
+        isRepositoryBare: false,
+        showNonGitRepositoryWarning: false,
+      })
       return
     }
 
-    const isRepository = await isGitRepository(pathToCheck)
-    // The path might have changed while we were checking, in which case we
-    // don't care about the result anymore.
-    if (this.state.path !== pathToCheck) {
-      return
-    }
+    const type = await getRepositoryType(path)
 
-    const isBare = await isBareRepository(this.state.path)
-    if (isBare === true) {
-      this.setState({ isRepositoryBare: true })
-      return
-    }
+    const isRepository = type !== 'missing'
+    const isRepositoryBare = type === 'bare'
+    const showNonGitRepositoryWarning = !isRepository || isRepositoryBare
 
-    this.setState({ isRepository, showNonGitRepositoryWarning: !isRepository })
-    this.setState({ isRepositoryBare: false })
+    this.setState(state =>
+      path === state.path
+        ? { isRepository, isRepositoryBare, showNonGitRepositoryWarning }
+        : null
+    )
   }
 
   private renderWarning() {
@@ -165,9 +175,9 @@ export class AddExistingRepository extends React.Component<
   }
 
   private onPathChanged = async (path: string) => {
-    const isRepository = await isGitRepository(path)
-
-    this.setState({ path, isRepository })
+    if (this.state.path !== path) {
+      this.updatePath(path)
+    }
   }
 
   private showFilePicker = async () => {
@@ -179,15 +189,7 @@ export class AddExistingRepository extends React.Component<
       return
     }
 
-    const isRepository = await isGitRepository(path)
-    const isRepositoryBare = await isBareRepository(path)
-
-    this.setState({
-      path,
-      isRepository,
-      showNonGitRepositoryWarning: !isRepository || isRepositoryBare,
-      isRepositoryBare,
-    })
+    this.updatePath(path)
   }
 
   private resolvedPath(path: string): string {

app/src/ui/app.tsx

@@ -1528,7 +1528,7 @@ export class App extends React.Component<IAppProps, IAppState> {
           />
         )
       case PopupType.About:
-        const version = __DEV__ ? __SHA__.substr(0, 10) : getVersion()
+        const version = __DEV__ ? __SHA__.substring(0, 10) : getVersion()
 
         return (
           <About

app/src/ui/autocompletion/autocompleting-text-input.tsx

@@ -351,7 +351,8 @@ export abstract class AutocompletingTextInput<
       originalText.substr(0, range.start - 1) + autoCompleteText + ' '
 
     const newText =
-      textWithAutoCompleteText + originalText.substr(range.start + range.length)
+      textWithAutoCompleteText +
+      originalText.substring(range.start + range.length)
 
     element.value = newText
 

app/src/ui/autocompletion/emoji-autocompletion-provider.tsx

@@ -104,9 +104,11 @@ export class EmojiAutocompletionProvider
 
     return (
       <div className="title">
-        {emoji.substr(0, hit.matchStart)}
-        <mark>{emoji.substr(hit.matchStart, hit.matchLength)}</mark>
-        {emoji.substr(hit.matchStart + hit.matchLength)}
+        {emoji.substring(0, hit.matchStart)}
+        <mark>
+          {emoji.substring(hit.matchStart, hit.matchStart + hit.matchLength)}
+        </mark>
+        {emoji.substring(hit.matchStart + hit.matchLength)}
       </div>
     )
   }

app/src/ui/create-branch/create-branch-dialog.tsx

@@ -146,7 +146,7 @@ export class CreateBranch extends React.Component<
       return (
         <p>
           Your new branch will be based on the commit '{targetCommit.summary}' (
-          {targetCommit.sha.substr(0, 7)}) from your repository.
+          {targetCommit.sha.substring(0, 7)}) from your repository.
         </p>
       )
     } else if (tip.kind === TipState.Detached) {
@@ -154,7 +154,7 @@ export class CreateBranch extends React.Component<
         <p>
           You do not currently have any branch checked out (your HEAD reference
           is detached). As such your new branch will be based on your currently
-          checked out commit ({tip.currentSha.substr(0, 7)}
+          checked out commit ({tip.currentSha.substring(0, 7)}
           ).
         </p>
       )

app/src/ui/diff/text-diff.tsx

@@ -861,7 +861,7 @@ export class TextDiff extends React.Component<ITextDiffProps, ITextDiffState> {
         if (i === 0 && range.head.ch > 0) {
           lineContent.push(line)
         } else {
-          lineContent.push(line.substr(1))
+          lineContent.push(line.substring(1))
         }
       }
 

app/src/ui/dispatcher/error-handlers.ts

@@ -656,6 +656,6 @@ function getRemoteMessage(stderr: string) {
   return stderr
     .split(/\r?\n/)
     .filter(x => x.startsWith(needle))
-    .map(x => x.substr(needle.length))
+    .map(x => x.substring(needle.length))
     .join('\n')
 }

app/src/ui/toolbar/branch-dropdown.tsx

@@ -141,7 +141,7 @@ export class BranchDropdown extends React.Component<
         b => !b.isDesktopForkRemoteBranch
       )
     } else if (tip.kind === TipState.Detached) {
-      title = `On ${tip.currentSha.substr(0, 7)}`
+      title = `On ${tip.currentSha.substring(0, 7)}`
       tooltip = 'Currently on a detached HEAD'
       icon = OcticonSymbol.gitCommit
       description = 'Detached HEAD'

app/static/darwin/github.sh

@@ -5,8 +5,8 @@ function realpath() {
   /usr/bin/perl -e "use Cwd;print Cwd::abs_path(@ARGV[0])" "$0";
 }
 
-CONTENTS="$(dirname "$(dirname "$(dirname "$(dirname "$(realpath "$0")")")")")"
-BINARY_NAME="$(ls "$CONTENTS/MacOS/")"
+CONTENTS="$(command dirname "$(command dirname "$(command dirname "$(command dirname "$(realpath "$0")")")")")"
+BINARY_NAME="$(TERM=dumb command ls "$CONTENTS/MacOS/")"
 ELECTRON="$CONTENTS/MacOS/$BINARY_NAME"
 CLI="$CONTENTS/Resources/app/cli.js"
 

app/test/unit/branch-pruner-test.ts

@@ -229,5 +229,5 @@ async function getBranchesFromGit(repository: Repository) {
   return gitOutput.stdout
     .split('\n')
     .filter(s => s.length > 0)
-    .map(s => s.substr(2))
+    .map(s => s.substring(2))
 }

app/test/unit/custom-theme-test.ts

@@ -9,15 +9,13 @@ describe('CustomTheme', () => {
     it('sets the first line to body.theme-high-contrast {', () => {
       const customTheme = CustomThemeDefaults[ApplicationTheme.HighContrast]
       const customThemeStyles = buildCustomThemeStyles(customTheme)
-      expect(customThemeStyles.split('\n')[0]).toBe(
-        'body.theme-high-contrast {'
-      )
+      expect(customThemeStyles).toStartWith('body.theme-high-contrast {\n')
     })
 
     it('sets the last line to }', () => {
       const customTheme = CustomThemeDefaults[ApplicationTheme.HighContrast]
       const customThemeStyles = buildCustomThemeStyles(customTheme)
-      expect(customThemeStyles.substr(-1, 1)).toBe('}')
+      expect(customThemeStyles).toEndWith('}')
     })
 
     it('prefaces all variable lines with --', () => {
@@ -31,8 +29,7 @@ describe('CustomTheme', () => {
         if (trimmedLine === '') {
           continue
         }
-        const firstTwoChar = trimmedLine.substr(0, 2)
-        expect(firstTwoChar).toBe('--')
+        expect(trimmedLine).toStartWith('--')
       }
     })
 
@@ -47,8 +44,7 @@ describe('CustomTheme', () => {
         if (trimmedLine === '') {
           continue
         }
-        const firstTwoChar = trimmedLine.substr(-1, 1)
-        expect(firstTwoChar).toBe(';')
+        expect(trimmedLine).toEndWith(';')
       }
     })
   })

app/test/unit/wrap-rich-text-commit-message-test.ts

@@ -46,14 +46,14 @@ describe('wrapRichTextCommitMessage', () => {
     expect(body.length).toBe(2)
 
     expect(summary[0].kind).toBe(TokenType.Text)
-    expect(summary[0].text).toBe(summaryText.substr(0, 72))
+    expect(summary[0].text).toBe(summaryText.substring(0, 72))
     expect(summary[1].kind).toBe(TokenType.Text)
     expect(summary[1].text).toBe('…')
 
     expect(body[0].kind).toBe(TokenType.Text)
     expect(body[0].text).toBe('…')
     expect(body[1].kind).toBe(TokenType.Text)
-    expect(body[1].text).toBe(summaryText.substr(72))
+    expect(body[1].text).toBe(summaryText.substring(72))
   })
 
   it('hard wraps text longer than 72 chars and joins it with the body', async () => {
@@ -66,12 +66,12 @@ describe('wrapRichTextCommitMessage', () => {
     expect(body.length).toBe(4)
 
     expect(summary[0].kind).toBe(TokenType.Text)
-    expect(summary[0].text).toBe(summaryText.substr(0, 72))
+    expect(summary[0].text).toBe(summaryText.substring(0, 72))
     expect(summary[1].kind).toBe(TokenType.Text)
     expect(summary[1].text).toBe('…')
 
     expect(body[0].text).toBe('…')
-    expect(body[1].text).toBe(summaryText.substr(72))
+    expect(body[1].text).toBe(summaryText.substring(72))
     expect(body[2].text).toBe('\n\n')
     expect(body[3].text).toBe(bodyText)
   })

app/webpack.common.ts

@@ -1,6 +1,5 @@
 import * as path from 'path'
 import HtmlWebpackPlugin from 'html-webpack-plugin'
-import CleanWebpackPlugin from 'clean-webpack-plugin'
 import webpack from 'webpack'
 import merge from 'webpack-merge'
 import { getChannel } from '../script/dist-info'
@@ -18,13 +17,16 @@ export const replacements = getReplacements()
 
 const commonConfig: webpack.Configuration = {
   optimization: {
-    noEmitOnErrors: true,
+    emitOnErrors: false,
   },
   externals: externals,
   output: {
     filename: '[name].js',
     path: path.resolve(__dirname, '..', outputDir),
-    libraryTarget: 'commonjs2',
+    library: {
+      name: '[name]',
+      type: 'commonjs2',
+    },
   },
   module: {
     rules: [
@@ -48,10 +50,12 @@ const commonConfig: webpack.Configuration = {
     ],
   },
   plugins: [
-    new CleanWebpackPlugin([outputDir], { verbose: false }),
     // This saves us a bunch of bytes by pruning locales (which we don't use)
     // from moment.
-    new webpack.IgnorePlugin(/^\.\/locale$/, /moment$/),
+    new webpack.IgnorePlugin({
+      resourceRegExp: /^\.\/locale$/,
+      contextRegExp: /moment$/,
+    }),
   ],
   resolve: {
     extensions: ['.js', '.ts', '.tsx'],
@@ -134,16 +138,19 @@ export const cli = merge({}, commonConfig, {
 export const highlighter = merge({}, commonConfig, {
   entry: { highlighter: path.resolve(__dirname, 'src/highlighter/index') },
   output: {
-    libraryTarget: 'var',
+    library: {
+      name: '[name]',
+      type: 'var',
+    },
     chunkFilename: 'highlighter/[name].js',
   },
   optimization: {
-    namedChunks: true,
+    chunkIds: 'named',
     splitChunks: {
       cacheGroups: {
         modes: {
           enforce: true,
-          name: (mod, chunks) => {
+          name: (mod: any) => {
             const builtInMode = /node_modules[\\\/]codemirror[\\\/]mode[\\\/](\w+)[\\\/]/i.exec(
               mod.resource
             )

app/webpack.development.ts

@@ -13,7 +13,7 @@ const cliConfig = merge({}, common.cli, config)
 const highlighterConfig = merge({}, common.highlighter, config)
 
 const getRendererEntryPoint = () => {
-  const entry = common.renderer.entry as webpack.Entry
+  const entry = common.renderer.entry as webpack.EntryObject
   if (entry == null) {
     throw new Error(
       `Unable to resolve entry point. Check webpack.common.ts and try again`
@@ -58,6 +58,9 @@ const rendererConfig = merge({}, common.renderer, config, {
       },
     ],
   },
+  infrastructureLogging: {
+    level: 'error',
+  },
   plugins: [new webpack.HotModuleReplacementPlugin()],
 })
 

app/yarn.lock

@@ -585,11 +585,6 @@ file-stream-rotator@^0.6.1:
   dependencies:
     moment "^2.29.1"
 
-file-url@^2.0.2:
-  version "2.0.2"
-  resolved "https://registry.yarnpkg.com/file-url/-/file-url-2.0.2.tgz#e951784d79095127d3713029ab063f40818ca2ae"
-  integrity sha1-6VF4TXkJUSfTcTApqwY/QIGMoq4=
-
 fn.name@1.x.x:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/fn.name/-/fn.name-1.1.0.tgz#26cad8017967aea8731bc42961d04a3d5988accc"

changelog.json

@@ -1,5 +1,15 @@
 {
   "releases": {
+    "2.9.11": [
+      "[Added] Add tooltip to show types of file changes in a commit - #13957. Thanks @uttiya10!",
+      "[Fixed] Discarding submodules with spaces in their relative path now correctly updates the submodule instead of moving it to Trash - #14024",
+      "[Fixed] Prevent crash report dialog from appearing when launching on macOS Catalina or earlier - #13974",
+      "[Fixed] Pre-fill clone path with repository name - #13971",
+      "[Fixed] Allow discarding changes in scenarios where they cannot be moved to Trash - #13888",
+      "[Fixed] \"Create New Repository\" dialog preserves the path set from \"Add Local Repository\" dialog - #13909",
+      "[Fixed] Treat the old and new format of private email addresses equally when showing commit attribution warning - #13879",
+      "[Fixed] Repositories containing untracked submodules no longer display a duplicated first character on Windows - #12314"
+    ],
     "2.9.11-beta1": [
       "[Added] Add tooltip to show types of file changes in a commit - #13957. Thanks @uttiya10!",
       "[Fixed] Discarding submodules with spaces in their relative path now correctly updates the submodule instead of moving it to Trash - #14024",

package.json

@@ -67,7 +67,6 @@
     "awesome-node-loader": "^1.1.0",
     "azure-storage": "^2.10.4",
     "chalk": "^2.2.0",
-    "clean-webpack-plugin": "^0.1.19",
     "codecov": "^3.7.1",
     "cross-env": "^5.0.5",
     "css-loader": "^2.1.0",
@@ -83,7 +82,7 @@
     "front-matter": "^2.3.0",
     "fs-extra": "^9.0.1",
     "glob": "^7.1.2",
-    "html-webpack-plugin": "^3.2.0",
+    "html-webpack-plugin": "^5.5.0",
     "jest": "^26.6.3",
     "jest-diff": "^25.0.0",
     "jest-extended": "^0.11.2",
@@ -91,7 +90,7 @@
     "jszip": "^3.7.1",
     "klaw-sync": "^3.0.0",
     "legal-eagle": "0.16.0",
-    "mini-css-extract-plugin": "^0.4.0",
+    "mini-css-extract-plugin": "^2.5.3",
     "parallel-webpack": "^2.3.0",
     "prettier": "2.0.5",
     "request": "^2.72.0",
@@ -104,40 +103,35 @@
     "style-loader": "^0.21.0",
     "to-camel-case": "^1.0.0",
     "ts-jest": "^26.4.4",
-    "ts-loader": "^8",
+    "ts-loader": "^9",
     "ts-node": "^7.0.0",
     "typescript": "^4.5.5",
-    "webpack": "^4.8.3",
-    "webpack-bundle-analyzer": "^3.3.2",
-    "webpack-dev-middleware": "^3.1.3",
-    "webpack-hot-middleware": "^2.22.2",
-    "webpack-merge": "^4.1.2",
+    "webpack": "^5.68.0",
+    "webpack-bundle-analyzer": "^4.5.0",
+    "webpack-dev-middleware": "^5.3.1",
+    "webpack-hot-middleware": "^2.25.1",
+    "webpack-merge": "^5.8.0",
     "xml2js": "^0.4.16"
   },
   "devDependencies": {
     "@types/byline": "^4.2.31",
     "@types/classnames": "^2.2.2",
-    "@types/clean-webpack-plugin": "^0.1.2",
     "@types/codemirror": "5.60.4",
     "@types/deep-equal": "^1.0.1",
     "@types/dompurify": "^2.3.1",
     "@types/double-ended-queue": "^2.1.0",
     "@types/electron-winstaller": "^4.0.0",
-    "@types/eslint": "^7.2.13",
+    "@types/eslint": "^8.4.1",
     "@types/estree": "^0.0.49",
     "@types/event-kit": "^1.2.28",
     "@types/express": "^4.11.0",
-    "@types/extract-text-webpack-plugin": "^3.0.3",
-    "@types/file-url": "^2.0.0",
     "@types/fs-extra": "^7.0.0",
     "@types/fuzzaldrin-plus": "^0.0.1",
     "@types/glob": "^5.0.35",
-    "@types/html-webpack-plugin": "^2.30.3",
     "@types/jest": "^23.3.1",
     "@types/klaw-sync": "^6.0.0",
     "@types/legal-eagle": "^0.15.0",
     "@types/memoize-one": "^3.1.1",
-    "@types/mini-css-extract-plugin": "^0.2.0",
     "@types/moment-duration-format": "^2.2.2",
     "@types/mri": "^1.1.0",
     "@types/node": "16.11.11",
@@ -161,11 +155,10 @@
     "@types/untildify": "^3.0.0",
     "@types/username": "^3.0.0",
     "@types/uuid": "^3.4.0",
-    "@types/webpack": "^4.4.0",
-    "@types/webpack-bundle-analyzer": "^2.9.2",
-    "@types/webpack-dev-middleware": "^2.0.1",
-    "@types/webpack-hot-middleware": "^2.16.3",
-    "@types/webpack-merge": "^4.1.3",
+    "@types/webpack": "^5.28.0",
+    "@types/webpack-bundle-analyzer": "^4.4.1",
+    "@types/webpack-hot-middleware": "^2.25.6",
+    "@types/webpack-merge": "^5.0.0",
     "@types/xml2js": "^0.4.0",
     "electron": "17.0.1",
     "electron-builder": "^22.7.0",

script/changelog/git.ts

@@ -1,21 +1,13 @@
-import { spawn } from './spawn'
+import { sh } from '../sh'
 
-export async function getLogLines(
-  previousVersion: string
-): Promise<ReadonlyArray<string>> {
-  const log = await spawn('git', [
+export const getLogLines = (previousVersion: string) =>
+  sh(
+    'git',
     'log',
     `...${previousVersion}`,
     '--merges',
     '--grep="Merge pull request"',
     '--format=format:%s',
     '-z',
-    '--',
-  ])
-
-  if (log.length === 0) {
-    return []
-  }
-
-  return log.split('\0')
-}
+    '--'
+  ).then(x => (x.length === 0 ? [] : x.split('\0')))

script/changelog/spawn.ts

@@ -1,31 +0,0 @@
-import * as ChildProcess from 'child_process'
-
-export function spawn(
-  cmd: string,
-  args: ReadonlyArray<string>
-): Promise<string> {
-  return new Promise((resolve, reject) => {
-    const child = ChildProcess.spawn(cmd, args as string[], { shell: true })
-    let receivedData = ''
-
-    child.on('error', reject)
-
-    child.stdout.on('data', data => {
-      receivedData += data
-    })
-
-    child.on('close', (code, signal) => {
-      if (code === 0) {
-        resolve(receivedData)
-      } else {
-        reject(
-          new Error(
-            `'${cmd} ${args.join(
-              ' '
-            )}' exited with code ${code}, signal ${signal}`
-          )
-        )
-      }
-    })
-  })
-}

script/draft-release/run.ts

@@ -1,6 +1,5 @@
 import { sort as semverSort, SemVer } from 'semver'
 
-import { spawn } from '../changelog/spawn'
 import { getLogLines } from '../changelog/git'
 import {
   convertToChangelogFormat,
@@ -15,6 +14,7 @@ import { writeFileSync } from 'fs'
 import { join } from 'path'
 import { format } from 'prettier'
 import { assertNever } from '../../app/src/lib/fatal-error'
+import { sh } from '../sh'
 
 const changelogPath = join(__dirname, '..', '..', 'changelog.json')
 
@@ -28,8 +28,7 @@ const changelogPath = join(__dirname, '..', '..', 'changelog.json')
 async function getLatestRelease(options: {
   excludeBetaReleases: boolean
 }): Promise<string> {
-  const allTags = await spawn('git', ['tag'])
-  let releaseTags = allTags
+  let releaseTags = (await sh('git', 'tag'))
     .split('\n')
     .filter(tag => tag.startsWith('release-'))
     .filter(tag => !tag.includes('-linux'))
@@ -39,7 +38,7 @@ async function getLatestRelease(options: {
     releaseTags = releaseTags.filter(tag => !tag.includes('-beta'))
   }
 
-  const releaseVersions = releaseTags.map(tag => tag.substr(8))
+  const releaseVersions = releaseTags.map(tag => tag.substring(8))
 
   const sortedTags = semverSort(releaseVersions)
   const latestTag = sortedTags[sortedTags.length - 1]

script/draft-release/version.ts

@@ -13,7 +13,7 @@ function isTestTag(version: SemVer) {
 function tryGetBetaNumber(version: SemVer): number | null {
   if (isBetaTag(version)) {
     const tag = version.prerelease[0]
-    const text = tag.substr(4)
+    const text = tag.substring(4)
     const betaNumber = parseInt(text, 10)
     return isNaN(betaNumber) ? null : betaNumber
   }

script/publish.ts

@@ -32,7 +32,7 @@ import request from 'request'
 console.log('Packaging…')
 execSync('yarn package', { stdio: 'inherit' })
 
-const sha = platforms.getSha().substr(0, 8)
+const sha = platforms.getSha().substring(0, 8)
 
 function getSecret() {
   if (process.env.DEPLOYMENT_SECRET != null) {

script/sh.ts

@@ -0,0 +1,15 @@
+import { execFile } from 'child_process'
+import { promisify } from 'util'
+
+const execFileP = promisify(execFile)
+
+/**
+ * Helper method for running a shell command and capturing its stdout
+ *
+ * Do not pass unsanitized user input to this function! Any input containing
+ * shell metacharacters may be used to trigger arbitrary command execution.
+ */
+export const sh = (cmd: string, ...args: string[]) =>
+  execFileP(cmd, args, { maxBuffer: Infinity, shell: true }).then(
+    ({ stdout }) => stdout
+  )

script/start.ts

@@ -52,7 +52,6 @@ if (process.env.NODE_ENV === 'production') {
         message,
         u(message, u(message, rendererConfig).output).publicPath
       ),
-      logLevel: 'error',
     })
   )
 

script/validate-changelog.ts

@@ -24,7 +24,7 @@ function formatErrors(errors: ErrorObject[]): string {
       }
 
       // dataPath starts with a leading "."," which is a bit confusing
-      const element = dataPath.substr(1)
+      const element = dataPath.substring(1)
 
       return ` - ${element} - ${message}${additionalPropertyText}`
     })