Merge pull request desktop#19817 from sfadschm/fix-rebase-commit-checks

fix: inconsistent rebase behaviour

Author: sergiou87

app/src/models/rebase.ts

@@ -32,7 +32,10 @@ export type RebaseProgressOptions = {
 
 export type CleanRebase = {
   readonly kind: ComputedAction.Clean
-  readonly commits: ReadonlyArray<CommitOneLine>
+  /** The commits present in the base branch but not in the target branch */
+  readonly commitsAhead: ReadonlyArray<CommitOneLine>
+  /** The commits present in the target branch but not in the base branch */
+  readonly commitsBehind: ReadonlyArray<CommitOneLine>
 }
 
 export type RebaseWithConflicts = {

app/src/ui/history/merge-call-to-action-with-conflicts.tsx

@@ -58,7 +58,7 @@ export class MergeCallToActionWithConflicts extends React.Component<
     if (selectedOperation === MultiCommitOperationKind.Rebase) {
       return rebasePreview !== null &&
         rebasePreview.kind === ComputedAction.Clean
-        ? rebasePreview.commits.length
+        ? rebasePreview.commitsAhead.length
         : 0
     }
 
@@ -148,7 +148,7 @@ export class MergeCallToActionWithConflicts extends React.Component<
       const commits =
         this.state.rebasePreview !== null &&
         this.state.rebasePreview.kind === ComputedAction.Clean
-          ? this.state.rebasePreview.commits
+          ? this.state.rebasePreview.commitsAhead
           : []
       return dispatcher.startRebase(
         repository,

app/src/ui/lib/update-branch.ts

@@ -42,7 +42,17 @@ export async function updateRebasePreview(
     kind: ComputedAction.Loading,
   })
 
-  const commits = await promiseWithMinimumTimeout(
+  const commitsBehind = await promiseWithMinimumTimeout(
+    () =>
+      getCommitsBetweenCommits(
+        repository,
+        targetBranch.tip.sha,
+        baseBranch.tip.sha
+      ),
+    500
+  )
+
+  const commitsAhead = await promiseWithMinimumTimeout(
     () =>
       getCommitsBetweenCommits(
         repository,
@@ -62,7 +72,7 @@ export async function updateRebasePreview(
 
   // if we are unable to find any commits to rebase, indicate that we're
   // unable to proceed with the rebase
-  if (commits === null) {
+  if (commitsBehind === null) {
     onUpdate({
       kind: ComputedAction.Invalid,
     })
@@ -71,6 +81,7 @@ export async function updateRebasePreview(
 
   onUpdate({
     kind: ComputedAction.Clean,
-    commits,
+    commitsAhead: commitsAhead ?? [],
+    commitsBehind: commitsBehind,
   })
 }

app/src/ui/multi-commit-operation/choose-branch/merge-choose-branch-dialog.tsx

@@ -182,8 +182,9 @@ export class MergeChooseBranchDialog extends React.Component<
     if (commitCount === 0) {
       return (
         <React.Fragment>
-          {`This branch is up to date with `}
-          <strong>{branch.name}</strong>
+          <strong>{currentBranch.name}</strong>
+          {` `}
+          is already up to date with <strong>{branch.name}</strong>
         </React.Fragment>
       )
     }

app/src/ui/multi-commit-operation/choose-branch/rebase-choose-branch-dialog.tsx

@@ -50,7 +50,7 @@ export class RebaseChooseBranchDialog extends React.Component<
       repository,
       selectedBranch,
       currentBranch,
-      rebasePreview.commits
+      rebasePreview.commitsAhead
     )
   }
 
@@ -59,7 +59,7 @@ export class RebaseChooseBranchDialog extends React.Component<
     const { selectedBranch, rebasePreview } = this.state
     const commitCount =
       rebasePreview?.kind === ComputedAction.Clean
-        ? rebasePreview.commits.length
+        ? rebasePreview.commitsBehind.length
         : undefined
     return canStartOperation(
       selectedBranch,
@@ -89,15 +89,15 @@ export class RebaseChooseBranchDialog extends React.Component<
       currentBranch !== null &&
       selectedBranch.name === currentBranch.name
 
-    const areCommitsToRebase =
+    const currentBranchIsBehindSelectedBranch =
       rebasePreview?.kind === ComputedAction.Clean
-        ? rebasePreview.commits.length > 0
+        ? rebasePreview.commitsBehind.length > 0
         : false
 
     return selectedBranchIsCurrentBranch
       ? 'You are not able to rebase this branch onto itself.'
-      : !areCommitsToRebase
-      ? 'There are no commits on the current branch to rebase.'
+      : !currentBranchIsBehindSelectedBranch
+      ? 'The current branch is already up to date with the selected branch.'
       : undefined
   }
 
@@ -135,7 +135,8 @@ export class RebaseChooseBranchDialog extends React.Component<
       return this.renderCleanRebaseMessage(
         currentBranch,
         baseBranch,
-        rebasePreview.commits.length
+        rebasePreview.commitsAhead.length,
+        rebasePreview.commitsBehind.length
       )
     }
 
@@ -157,25 +158,43 @@ export class RebaseChooseBranchDialog extends React.Component<
   private renderCleanRebaseMessage(
     currentBranch: Branch,
     baseBranch: Branch,
-    commitsToRebase: number
+    commitsAheadCount: number,
+    commitsBehindCount: number
   ) {
-    if (commitsToRebase <= 0) {
+    // The current branch is behind the base branch
+    if (commitsBehindCount > 0 && commitsAheadCount <= 0) {
+      const pluralized = commitsBehindCount === 1 ? 'commit' : 'commits'
       return (
         <>
-          This branch is up to date with{` `}
-          <strong>{currentBranch.name}</strong>
+          This will fast-forward <strong>{currentBranch.name}</strong> by
+          <strong>{` ${commitsBehindCount} ${pluralized}`}</strong>
+          {` to match `}
+          <strong>{baseBranch.name}</strong>
         </>
       )
     }
 
-    const pluralized = commitsToRebase === 1 ? 'commit' : 'commits'
+    // The current branch is behind and ahead of the base branch
+    if (commitsBehindCount > 0 && commitsAheadCount > 0) {
+      const pluralized = commitsAheadCount === 1 ? 'commit' : 'commits'
+      return (
+        <>
+          This will update <strong>{currentBranch.name}</strong>
+          {` by applying its `}
+          <strong>{` ${commitsAheadCount} ${pluralized}`}</strong>
+          {` on top of `}
+          <strong>{baseBranch.name}</strong>
+        </>
+      )
+    }
+
+    // The current branch is a direct child of the base branch
+    // Condition: commitsBehindCount <= 0 && commitsAheadCount >= 0
     return (
       <>
-        This will update <strong>{currentBranch.name}</strong>
-        {` by applying its `}
-        <strong>{` ${commitsToRebase} ${pluralized}`}</strong>
-        {` on top of `}
-        <strong>{baseBranch.name}</strong>
+        <strong>{currentBranch.name}</strong>
+        {` `}
+        is already up to date with <strong>{baseBranch.name}</strong>
       </>
     )
   }