Simplify pull single branch logic

Reuse the existing dispatcher fetch() and pull() instead of duplicating the fetch logic.
While the original implemnetation worked well, this greatly reduces the diff with respect to upstream, so that we can maintain it long-term.
This has the side effect of also fast-forwarding the other branches (not only pulling the selected), which is fine because that is usually the user's intention.

Author: pol-rivero

app/src/lib/progress/fetch.ts

@@ -20,26 +20,3 @@ export class FetchProgressParser extends GitProgressParser {
     super(steps)
   }
 }
-
-/**
- * Progress steps for a single-branch fetch operation. Unlike a full fetch,
- * Highly approximate (some would say outright inaccurate) division
- * of the individual progress reporting steps in a fetch operation
- */
-const singleBranchFetchSteps = [
-  { title: 'remote: Enumerating objects', weight: 0.1 },
-  { title: 'remote: Counting objects', weight: 0.2 },
-  { title: 'remote: Compressing objects', weight: 0.3 },
-  { title: 'remote', weight: 0.4 },
-]
-
-/**
- * A utility class for interpreting the output from
- * `git fetch --progress <remote> <branch>` and turning that into a percentage
- * value estimating the overall progress of a single branch fetch.
- */
-export class SingleBranchFetchProgressParser extends GitProgressParser {
-  public constructor() {
-    super(singleBranchFetchSteps)
-  }
-}

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

@@ -266,8 +266,6 @@ import {
   listWorktrees,
   unstageAll,
   git,
-  IGitStringExecutionOptions,
-  IGitStringResult,
 } from '../git'
 import {
   installGlobalLFSFilters,
@@ -453,11 +451,6 @@ import {
   gatherCommitContext,
 } from '../copilot-conflict-context'
 import { resolveWithin } from '../path'
-import {
-  executionOptionsWithProgress,
-  SingleBranchFetchProgressParser,
-} from '../progress'
-import { envForRemoteOperation } from '../git/environment'
 
 const LastSelectedRepositoryIDKey = 'last-selected-repository-id'
 
@@ -6082,171 +6075,6 @@ export class AppStore extends TypedBaseStore<IAppState> {
     })
   }
 
-  public async _fetchSingleBranch(
-    repository: Repository,
-    branch: Branch
-  ): Promise<void> {
-    const state = this.repositoryStateCache.get(repository)
-    // Don't allow concurrent network operations.
-    if (state.isPushPullFetchInProgress) {
-      this._showPopup({
-        type: PopupType.Error,
-        error: new Error(
-          'Another push/pull/fetch request is in progress.\nTry again after the ongoing request is finished'
-        ),
-      })
-      return
-    }
-
-    return this.withRefreshedGitHubRepository(repository, repo => {
-      return this.performFetchSingleBranch(repo, branch)
-    })
-  }
-
-  /** This shouldn't be called directly. See `Dispatcher`. */
-  private async performFetchSingleBranch(
-    repository: Repository,
-    branch: Branch
-  ) {
-    const isRemote = branch.type === BranchType.Remote
-
-    const remoteName = isRemote ? branch.remoteName : branch.upstreamRemoteName
-    const remoteBranchName = isRemote
-      ? branch.nameWithoutRemote
-      : branch.upstreamWithoutRemote
-
-    if (!remoteName) {
-      throw new Error('Remote name not found')
-    }
-    if (!remoteBranchName) {
-      throw new Error('Remote branch not found')
-    }
-
-    const isBackgroundTask = false
-    const gitStore = this.gitStoreCache.get(repository)
-
-    // repository.url
-    const remote = { name: remoteName, url: 'file://' }
-
-    const progressCb = (progress: IFetchProgress) => {
-      this.updatePushPullFetchProgress(repository, progress)
-    }
-    const progressTitle = isRemote
-      ? `Fetching ${branch.name}`
-      : `Fetching ${remoteBranchName}`
-    const kind = 'fetch'
-
-    const fetchFn = async (isRemote: boolean): Promise<IGitStringResult> => {
-      let opts: IGitStringExecutionOptions = {
-        successExitCodes: new Set([0]),
-      }
-      if (remote.url) {
-        opts = {
-          ...opts,
-          env: await envForRemoteOperation(remote.url),
-        }
-      }
-      opts = await executionOptionsWithProgress(
-        { ...opts, trackLFSProgress: true, isBackgroundTask },
-        new SingleBranchFetchProgressParser(),
-        progress => {
-          if (progress.kind === 'context') {
-            const text = progress.text
-            if (
-              !text.startsWith('remote: Counting objects') &&
-              !text.startsWith('remote: Compressing objects')
-            ) {
-              return
-            }
-          }
-
-          const description =
-            progress.kind === 'progress' ? progress.details.text : progress.text
-          const value = progress.percent
-
-          progressCb({
-            kind,
-            title: progressTitle,
-            description,
-            value,
-            remote: remote.name,
-          })
-        }
-      )
-      const flags = isRemote
-        ? ['fetch', '--progress', '--recurse-submodules=on-demand', remoteName]
-        : [
-            'fetch',
-            '--progress',
-            '--show-forced-updates',
-            // '--no-write-fetch-head',
-            '--recurse-submodules=on-demand',
-            remoteName,
-          ]
-
-      const branchTarget = isRemote
-        ? remoteBranchName
-        : `${remoteBranchName}:${remoteBranchName}`
-      const actionName = isRemote ? 'fetchRemoteBranch' : 'fetchLocalBranch'
-
-      const executionOpts = isRemote
-        ? opts
-        : {
-            ...opts,
-            successExitCodes: new Set([0, 1]),
-          }
-
-      return await git(
-        [...flags, branchTarget],
-        repository.path,
-        actionName,
-        executionOpts
-      )
-    }
-
-    const execFetchFn = async () => {
-      // Initial progress
-      progressCb({
-        kind,
-        title: progressTitle,
-        value: 0,
-        remote: remote.name,
-      })
-
-      await gitStore.performFailableOperation(
-        async () => {
-          const result = await fetchFn(isRemote)
-          if (
-            !isRemote &&
-            result &&
-            (result.stderr?.includes('rejected') ||
-              result.stderr?.includes('non-fast-forward'))
-          ) {
-            this.emitError(
-              new ErrorWithMetadata(new Error(result.stderr), { repository })
-            )
-          }
-
-          await this._refreshRepository(repository)
-        },
-        {
-          backgroundTask: isBackgroundTask,
-        }
-      )
-    }
-
-    try {
-      await this.withPushPullFetch(repository, execFetchFn)
-    } catch (error) {
-      const errorWithMetadata = new ErrorWithMetadata(error, {
-        repository,
-      })
-      this.emitError(errorWithMetadata)
-    } finally {
-      this.updatePushPullFetchProgress(repository, null)
-    }
-  }
-
   public async _resetHardToUpstream(repository: Repository): Promise<void> {
     const { branchesState } = this.repositoryStateCache.get(repository)
     const { tip } = branchesState
@@ -6778,6 +6606,30 @@ export class AppStore extends TypedBaseStore<IAppState> {
     })
   }
 
+  /**
+   * Attempts to fast-forward a branch to its upstream.
+   *
+   * @returns true if it was successful
+   */
+  public async _fastForwardBranch(
+    repository: Repository,
+    branch: Branch
+  ): Promise<void> {
+    if (branch.upstream === null || branch.isGone) {
+      throw new Error(`The branch '${branch.name}' has not been published yet.`)
+    }
+    await this._fetch(repository, FetchType.UserInitiatedTask)
+    const stillDifferingBranches = await getBranchesDifferingFromUpstream(
+      repository
+    )
+    const stillDiffers = stillDifferingBranches.some(b => b.ref === branch.ref)
+    if (stillDiffers) {
+      throw new Error(
+        `The branch '${branch.name}' cannot be fast-forwarded. Switch to it and pull it manually.`
+      )
+    }
+  }
+
   /**
    * Fetch a particular remote in a repository.
    *

app/src/ui/branches/branch-list-item-context-menu.tsx

@@ -7,15 +7,14 @@ interface IBranchContextMenuConfig {
   name: string
   nameWithoutRemote: string
   isLocal: boolean
-  isCurrentBranch: boolean
   repoType: RepoType | undefined
   isInUseByOtherWorktree: boolean
   onRenameBranch?: (branchName: string) => void
   onViewBranchOnGitHub?: () => void
   onViewPullRequestOnGitHub?: () => void
   onSetAsDefaultBranch?: (branchName: string) => void
   onDeleteBranch?: (branchName: string) => void
-  onFetchSingleBranch?: (branchName: string) => void
+  onPullSingleBranch?: (branchName: string) => void
 }
 
 export function generateBranchContextMenuItems(
@@ -25,17 +24,17 @@ export function generateBranchContextMenuItems(
     name,
     nameWithoutRemote,
     isLocal,
-    isCurrentBranch,
     repoType,
     isInUseByOtherWorktree,
     onRenameBranch,
     onViewBranchOnGitHub,
     onViewPullRequestOnGitHub,
     onSetAsDefaultBranch,
     onDeleteBranch,
-    onFetchSingleBranch,
+    onPullSingleBranch,
   } = config
   const items = new Array<IMenuItem>()
+
   if (onRenameBranch !== undefined) {
     items.push({
       label: 'Rename…',
@@ -70,12 +69,11 @@ export function generateBranchContextMenuItems(
     })
   }
 
-  // This should be the selected branch.
-  if (!isCurrentBranch && onFetchSingleBranch !== undefined) {
+  if (onPullSingleBranch) {
     items.push({ type: 'separator' })
     items.push({
-      label: getSingleFetchBranchLabel(),
-      action: () => onFetchSingleBranch(name),
+      label: __DARWIN__ ? 'Pull branch' : 'Pull branch',
+      action: () => onPullSingleBranch(name),
       enabled: true,
     })
   }
@@ -117,7 +115,3 @@ function getViewPullRequestLabel(repoType: RepoType): string {
       return assertNever(repoType, `Unknown repo type: ${repoType}`)
   }
 }
-
-function getSingleFetchBranchLabel(): string {
-  return __DARWIN__ ? 'Fetch Branch' : 'Fetch branch'
-}

app/src/ui/branches/branch-list.tsx

@@ -145,8 +145,8 @@ interface IBranchListProps {
   /** Optional: Callback for if delete context menu should exist */
   readonly onDeleteBranch?: (branchName: string) => void
 
-  /** Optional: Callback if pull option for remote branch context menu should exist */
-  readonly onFetchSingleBranch?: (branchName: string) => void
+  /** Optional: Callback if option to fetch branches should exist */
+  readonly onPullSingleBranch?: (branchName: string) => void
 }
 
 /** The Branches list component. */
@@ -241,7 +241,7 @@ export class BranchList extends React.Component<IBranchListProps> {
       onRenameBranch,
       onDeleteBranch,
       onSetAsDefaultBranch,
-      onFetchSingleBranch,
+      onPullSingleBranch,
     } = this.props
 
     if (
@@ -255,11 +255,11 @@ export class BranchList extends React.Component<IBranchListProps> {
     const { type, name, nameWithoutRemote } = item.branch
     const isLocal = type === BranchType.Local
     const isInUseByOtherWorktree = !!this.inUseByOtherWorktreeName(item)
+
     const items = generateBranchContextMenuItems({
       name,
       nameWithoutRemote,
       isLocal,
-      isCurrentBranch: item.branch.name === this.props.currentBranch?.name,
       repoType: this.props.repository.gitHubRepository?.type,
       isInUseByOtherWorktree,
       onRenameBranch,
@@ -268,7 +268,7 @@ export class BranchList extends React.Component<IBranchListProps> {
           ? undefined
           : onSetAsDefaultBranch,
       onDeleteBranch,
-      onFetchSingleBranch,
+      onPullSingleBranch,
     })
 
     showContextualMenu(items)

app/src/ui/branches/branches-container.tsx

@@ -53,7 +53,7 @@ interface IBranchesContainerProps {
   readonly onRenameBranch: (branchName: string) => void
   readonly onSetAsDefaultBranch: (branchName: string) => void
   readonly onDeleteBranch: (branchName: string) => void
-  readonly onFetchSingleBranch: (branchName: string) => void
+  readonly onPullSingleBranch: (branchName: string) => void
 
   readonly branchSortOrder: BranchSortOrder
 
@@ -294,7 +294,7 @@ export class BranchesContainer extends React.Component<
             onRenameBranch={this.props.onRenameBranch}
             onSetAsDefaultBranch={this.props.onSetAsDefaultBranch}
             onDeleteBranch={this.props.onDeleteBranch}
-            onFetchSingleBranch={this.props.onFetchSingleBranch}
+            onPullSingleBranch={this.props.onPullSingleBranch}
           />
         )
       case BranchesTab.PullRequests: {

app/src/ui/dispatcher/dispatcher.ts

@@ -821,12 +821,11 @@ export class Dispatcher {
     return this.appStore._pull(repository)
   }
 
-  /** Pull remote branch by name */
-  public fetchSingleBranch(
+  public fastForwardBranch(
     repository: Repository,
     branch: Branch
   ): Promise<void> {
-    return this.appStore._fetchSingleBranch(repository, branch)
+    return this.appStore._fastForwardBranch(repository, branch)
   }
 
   public async pullAllRepositories(): Promise<void> {