From e25541134aba33a35fbec942a2240e7f5056bbf3 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 12 Aug 2021 15:07:43 -0400 Subject: [PATCH 01/22] Move dialogs --- .../base-multi-commit-operation.tsx | 8 ++++---- .../{ => dialog}/confirm-abort-dialog.tsx | 4 ++-- .../{ => dialog}/conflicts-dialog.tsx | 16 ++++++++-------- .../{ => dialog}/progress-dialog.tsx | 12 ++++++------ .../{ => dialog}/warn-force-push-dialog.tsx | 8 ++++---- app/src/ui/rebase/rebase-flow.tsx | 2 +- 6 files changed, 25 insertions(+), 25 deletions(-) rename app/src/ui/multi-commit-operation/{ => dialog}/confirm-abort-dialog.tsx (93%) rename app/src/ui/multi-commit-operation/{ => dialog}/conflicts-dialog.tsx (92%) rename app/src/ui/multi-commit-operation/{ => dialog}/progress-dialog.tsx (80%) rename app/src/ui/multi-commit-operation/{ => dialog}/warn-force-push-dialog.tsx (91%) diff --git a/app/src/ui/multi-commit-operation/base-multi-commit-operation.tsx b/app/src/ui/multi-commit-operation/base-multi-commit-operation.tsx index 6dd43886bc..1722677cc4 100644 --- a/app/src/ui/multi-commit-operation/base-multi-commit-operation.tsx +++ b/app/src/ui/multi-commit-operation/base-multi-commit-operation.tsx @@ -7,10 +7,10 @@ import { getResolvedFiles } from '../../lib/status' import { ConflictState, IMultiCommitOperationState } from '../../lib/app-state' import { Branch } from '../../models/branch' import { MultiCommitOperationStepKind } from '../../models/multi-commit-operation' -import { ConflictsDialog } from './conflicts-dialog' -import { ConfirmAbortDialog } from './confirm-abort-dialog' -import { ProgressDialog } from './progress-dialog' -import { WarnForcePushDialog } from './warn-force-push-dialog' +import { ConflictsDialog } from './dialog/conflicts-dialog' +import { ConfirmAbortDialog } from './dialog/confirm-abort-dialog' +import { ProgressDialog } from './dialog/progress-dialog' +import { WarnForcePushDialog } from './dialog/warn-force-push-dialog' import { PopupType } from '../../models/popup' export interface IMultiCommitOperationProps { diff --git a/app/src/ui/multi-commit-operation/confirm-abort-dialog.tsx b/app/src/ui/multi-commit-operation/dialog/confirm-abort-dialog.tsx similarity index 93% rename from app/src/ui/multi-commit-operation/confirm-abort-dialog.tsx rename to app/src/ui/multi-commit-operation/dialog/confirm-abort-dialog.tsx index 69d584e68b..6bcaee3c10 100644 --- a/app/src/ui/multi-commit-operation/confirm-abort-dialog.tsx +++ b/app/src/ui/multi-commit-operation/dialog/confirm-abort-dialog.tsx @@ -1,6 +1,6 @@ import * as React from 'react' -import { Dialog, DialogContent, DialogFooter } from '../dialog' -import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group' +import { Dialog, DialogContent, DialogFooter } from '../../dialog' +import { OkCancelButtonGroup } from '../../dialog/ok-cancel-button-group' interface IConfirmAbortDialogProps { /** diff --git a/app/src/ui/multi-commit-operation/conflicts-dialog.tsx b/app/src/ui/multi-commit-operation/dialog/conflicts-dialog.tsx similarity index 92% rename from app/src/ui/multi-commit-operation/conflicts-dialog.tsx rename to app/src/ui/multi-commit-operation/dialog/conflicts-dialog.tsx index 8c534acb4c..e07d996601 100644 --- a/app/src/ui/multi-commit-operation/conflicts-dialog.tsx +++ b/app/src/ui/multi-commit-operation/dialog/conflicts-dialog.tsx @@ -1,25 +1,25 @@ import * as React from 'react' -import { Dialog, DialogContent, DialogFooter } from '../dialog' -import { Dispatcher } from '../dispatcher' -import { Repository } from '../../models/repository' +import { Dialog, DialogContent, DialogFooter } from '../../dialog' +import { Dispatcher } from '../../dispatcher' +import { Repository } from '../../../models/repository' import { WorkingDirectoryStatus, WorkingDirectoryFileChange, -} from '../../models/status' +} from '../../../models/status' import { isConflictedFile, getResolvedFiles, getConflictedFiles, getUnmergedFiles, -} from '../../lib/status' +} from '../../../lib/status' import { renderUnmergedFile, renderUnmergedFilesSummary, renderShellLink, renderAllResolved, -} from '../lib/conflicts' -import { ManualConflictResolution } from '../../models/manual-conflict-resolution' -import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group' +} from '../../lib/conflicts' +import { ManualConflictResolution } from '../../../models/manual-conflict-resolution' +import { OkCancelButtonGroup } from '../../dialog/ok-cancel-button-group' interface IConflictsDialogProps { readonly dispatcher: Dispatcher diff --git a/app/src/ui/multi-commit-operation/progress-dialog.tsx b/app/src/ui/multi-commit-operation/dialog/progress-dialog.tsx similarity index 80% rename from app/src/ui/multi-commit-operation/progress-dialog.tsx rename to app/src/ui/multi-commit-operation/dialog/progress-dialog.tsx index e3a89c4e75..3ee25432dd 100644 --- a/app/src/ui/multi-commit-operation/progress-dialog.tsx +++ b/app/src/ui/multi-commit-operation/dialog/progress-dialog.tsx @@ -1,10 +1,10 @@ import * as React from 'react' -import { formatRebaseValue } from '../../lib/rebase' -import { RichText } from '../lib/rich-text' -import { Dialog, DialogContent } from '../dialog' -import { Octicon } from '../octicons' -import * as OcticonSymbol from '../octicons/octicons.generated' -import { IMultiCommitOperationProgress } from '../../models/progress' +import { formatRebaseValue } from '../../../lib/rebase' +import { RichText } from '../../lib/rich-text' +import { Dialog, DialogContent } from '../../dialog' +import { Octicon } from '../../octicons' +import * as OcticonSymbol from '../../octicons/octicons.generated' +import { IMultiCommitOperationProgress } from '../../../models/progress' interface IProgressDialogProps { /** diff --git a/app/src/ui/multi-commit-operation/warn-force-push-dialog.tsx b/app/src/ui/multi-commit-operation/dialog/warn-force-push-dialog.tsx similarity index 91% rename from app/src/ui/multi-commit-operation/warn-force-push-dialog.tsx rename to app/src/ui/multi-commit-operation/dialog/warn-force-push-dialog.tsx index ed74ae8385..c8e5d3a1bf 100644 --- a/app/src/ui/multi-commit-operation/warn-force-push-dialog.tsx +++ b/app/src/ui/multi-commit-operation/dialog/warn-force-push-dialog.tsx @@ -1,8 +1,8 @@ import * as React from 'react' -import { Checkbox, CheckboxValue } from '../lib/checkbox' -import { Dispatcher } from '../dispatcher' -import { DialogFooter, DialogContent, Dialog } from '../dialog' -import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group' +import { Checkbox, CheckboxValue } from '../../lib/checkbox' +import { Dispatcher } from '../../dispatcher' +import { DialogFooter, DialogContent, Dialog } from '../../dialog' +import { OkCancelButtonGroup } from '../../dialog/ok-cancel-button-group' interface IWarnForcePushProps { /** diff --git a/app/src/ui/rebase/rebase-flow.tsx b/app/src/ui/rebase/rebase-flow.tsx index 207cab6d36..11ba95bc7d 100644 --- a/app/src/ui/rebase/rebase-flow.tsx +++ b/app/src/ui/rebase/rebase-flow.tsx @@ -16,7 +16,7 @@ import { RebaseProgressDialog } from './progress-dialog' import { ConfirmAbortDialog } from './confirm-abort-dialog' import { getResolvedFiles } from '../../lib/status' import { WarnForcePushDialog } from './warn-force-push-dialog' -import { ConflictsDialog } from '../multi-commit-operation/conflicts-dialog' +import { ConflictsDialog } from '../multi-commit-operation/dialog/conflicts-dialog' import { IMultiCommitOperationProgress } from '../../models/progress' import { RebaseChooseBranchDialog } from '../multi-commit-operation/choose-branch/rebase-choose-branch-dialog' import { MultiCommitOperationKind } from '../../models/multi-commit-operation' From 502cf3a0f16d0111c5ec0cd968e63d40d115a4e0 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 12 Aug 2021 15:39:43 -0400 Subject: [PATCH 02/22] Create base rebase multi commit operation --- app/src/models/multi-commit-operation.ts | 24 ++++--- .../ui/multi-commit-operation/base-rebase.tsx | 71 +++++++++++++++++++ app/src/ui/multi-commit-operation/reorder.tsx | 67 ++--------------- app/src/ui/multi-commit-operation/squash.tsx | 67 ++--------------- 4 files changed, 95 insertions(+), 134 deletions(-) create mode 100644 app/src/ui/multi-commit-operation/base-rebase.tsx diff --git a/app/src/models/multi-commit-operation.ts b/app/src/models/multi-commit-operation.ts index cf9f874696..7b9588c48f 100644 --- a/app/src/models/multi-commit-operation.ts +++ b/app/src/models/multi-commit-operation.ts @@ -134,13 +134,7 @@ export type CreateBranchStep = { targetBranchName: string } -interface IInteractiveRebaseDetails { - /** - * The reference to the last retained commit on the branch during an - * interactive rebase or null if rebasing to the root. - */ - readonly lastRetainedCommitRef: string | null - +interface IBaseRebaseDetails { /** * Array of commits used during the operation. */ @@ -153,6 +147,14 @@ interface IInteractiveRebaseDetails { readonly currentTip: string } +interface IInteractiveRebaseDetails extends IBaseRebaseDetails { + /** + * The reference to the last retained commit on the branch during an + * interactive rebase or null if rebasing to the root. + */ + readonly lastRetainedCommitRef: string | null +} + interface ISourceBranchDetails { /** * The branch that are the source of the commits for the operation. @@ -201,7 +203,7 @@ interface ICherryPickDetails extends ISourceBranchDetails { readonly commits: ReadonlyArray } -interface IRebaseDetails extends ISourceBranchDetails { +interface IRebaseDetails extends ISourceBranchDetails, IBaseRebaseDetails { readonly kind: MultiCommitOperationKind.Rebase } @@ -216,3 +218,9 @@ export type MultiCommitOperationDetail = | ICherryPickDetails | IRebaseDetails | IMergeDetails + +export function instanceOfIBaseRebaseDetails( + object: any +): object is IBaseRebaseDetails { + return 'commits' in object && 'currentTip' in object +} diff --git a/app/src/ui/multi-commit-operation/base-rebase.tsx b/app/src/ui/multi-commit-operation/base-rebase.tsx new file mode 100644 index 0000000000..e6e1fb5f9d --- /dev/null +++ b/app/src/ui/multi-commit-operation/base-rebase.tsx @@ -0,0 +1,71 @@ +import { RebaseConflictState } from '../../lib/app-state' +import { + instanceOfIBaseRebaseDetails, + MultiCommitOperationKind, +} from '../../models/multi-commit-operation' +import { BaseMultiCommitOperation } from './base-multi-commit-operation' + +export abstract class BaseRebase extends BaseMultiCommitOperation { + protected abstract conflictDialogOperationPrefix: string + protected abstract rebaseKind: MultiCommitOperationKind + + protected onContinueAfterConflicts = async (): Promise => { + const { + repository, + dispatcher, + workingDirectory, + state, + conflictState, + } = this.props + const { operationDetail, targetBranch, originalBranchTip } = state + + if ( + conflictState === null || + targetBranch === null || + originalBranchTip === null || + !instanceOfIBaseRebaseDetails(operationDetail) + ) { + this.endFlowInvalidState() + return + } + + const { commits, currentTip } = operationDetail + + await dispatcher.switchMultiCommitOperationToShowProgress(repository) + + const rebaseConflictState: RebaseConflictState = { + kind: 'rebase', + currentTip, + targetBranch: targetBranch.name, + baseBranch: undefined, + originalBranchTip, + baseBranchTip: currentTip, + manualResolutions: conflictState.manualResolutions, + } + + const rebaseResult = await dispatcher.continueRebase( + this.rebaseKind, + repository, + workingDirectory, + rebaseConflictState + ) + + return dispatcher.processMultiCommitOperationRebaseResult( + this.rebaseKind, + repository, + rebaseResult, + commits.length + 1, + targetBranch.name + ) + } + + protected onAbort = async (): Promise => { + const { repository, dispatcher } = this.props + this.onFlowEnded() + return dispatcher.abortRebase(repository) + } + + protected onConflictsDialogDismissed = () => { + this.onInvokeConflictsDialogDismissed(this.conflictDialogOperationPrefix) + } +} diff --git a/app/src/ui/multi-commit-operation/reorder.tsx b/app/src/ui/multi-commit-operation/reorder.tsx index 448ebbed36..46a1a860ba 100644 --- a/app/src/ui/multi-commit-operation/reorder.tsx +++ b/app/src/ui/multi-commit-operation/reorder.tsx @@ -1,8 +1,9 @@ -import { RebaseConflictState } from '../../lib/app-state' import { MultiCommitOperationKind } from '../../models/multi-commit-operation' -import { BaseMultiCommitOperation } from './base-multi-commit-operation' +import { BaseRebase } from './base-rebase' + +export abstract class Reorder extends BaseRebase { + protected conflictDialogOperationPrefix = 'reordering commits on' -export abstract class Reorder extends BaseMultiCommitOperation { protected onBeginOperation = () => { const { repository, dispatcher, state } = this.props const { operationDetail } = state @@ -22,64 +23,4 @@ export abstract class Reorder extends BaseMultiCommitOperation { true ) } - - protected onContinueAfterConflicts = async (): Promise => { - const { - repository, - dispatcher, - workingDirectory, - state, - conflictState, - } = this.props - const { operationDetail, targetBranch, originalBranchTip } = state - - if ( - conflictState === null || - targetBranch === null || - originalBranchTip === null || - operationDetail.kind !== MultiCommitOperationKind.Reorder - ) { - this.endFlowInvalidState() - return - } - - const { commits, currentTip } = operationDetail - - await dispatcher.switchMultiCommitOperationToShowProgress(repository) - - const rebaseConflictState: RebaseConflictState = { - kind: 'rebase', - currentTip, - targetBranch: targetBranch.name, - baseBranch: undefined, - originalBranchTip, - baseBranchTip: currentTip, - manualResolutions: conflictState.manualResolutions, - } - - const rebaseResult = await dispatcher.continueRebase( - MultiCommitOperationKind.Reorder, - repository, - workingDirectory, - rebaseConflictState - ) - - return dispatcher.processMultiCommitOperationRebaseResult( - MultiCommitOperationKind.Reorder, - repository, - rebaseResult, - commits.length, - targetBranch.name - ) - } - - protected onAbort = async (): Promise => { - const { repository, dispatcher } = this.props - this.onFlowEnded() - return dispatcher.abortRebase(repository) - } - - protected onConflictsDialogDismissed = () => { - this.onInvokeConflictsDialogDismissed('reordering commits on') - } } diff --git a/app/src/ui/multi-commit-operation/squash.tsx b/app/src/ui/multi-commit-operation/squash.tsx index 518dd6bade..2db58f2ae3 100644 --- a/app/src/ui/multi-commit-operation/squash.tsx +++ b/app/src/ui/multi-commit-operation/squash.tsx @@ -1,8 +1,9 @@ -import { RebaseConflictState } from '../../lib/app-state' import { MultiCommitOperationKind } from '../../models/multi-commit-operation' -import { BaseMultiCommitOperation } from './base-multi-commit-operation' +import { BaseRebase } from './base-rebase' + +export abstract class Squash extends BaseRebase { + protected conflictDialogOperationPrefix = 'squashing commits on' -export abstract class Squash extends BaseMultiCommitOperation { protected onBeginOperation = () => { const { repository, dispatcher, state } = this.props const { operationDetail } = state @@ -28,64 +29,4 @@ export abstract class Squash extends BaseMultiCommitOperation { true ) } - - protected onContinueAfterConflicts = async (): Promise => { - const { - repository, - dispatcher, - workingDirectory, - state, - conflictState, - } = this.props - const { operationDetail, targetBranch, originalBranchTip } = state - - if ( - conflictState === null || - targetBranch === null || - originalBranchTip === null || - operationDetail.kind !== MultiCommitOperationKind.Squash - ) { - this.endFlowInvalidState() - return - } - - const { commits, currentTip } = operationDetail - - await dispatcher.switchMultiCommitOperationToShowProgress(repository) - - const rebaseConflictState: RebaseConflictState = { - kind: 'rebase', - currentTip, - targetBranch: targetBranch.name, - baseBranch: undefined, - originalBranchTip, - baseBranchTip: currentTip, - manualResolutions: conflictState.manualResolutions, - } - - const rebaseResult = await dispatcher.continueRebase( - MultiCommitOperationKind.Squash, - repository, - workingDirectory, - rebaseConflictState - ) - - return dispatcher.processMultiCommitOperationRebaseResult( - MultiCommitOperationKind.Squash, - repository, - rebaseResult, - commits.length + 1, - targetBranch.name - ) - } - - protected onAbort = async (): Promise => { - const { repository, dispatcher } = this.props - this.onFlowEnded() - return dispatcher.abortRebase(repository) - } - - protected onConflictsDialogDismissed = () => { - this.onInvokeConflictsDialogDismissed('squashing commits on') - } } From 3c4794fc3bfac16269d36c3c739a52749ecce981 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Sat, 14 Aug 2021 14:14:06 -0400 Subject: [PATCH 03/22] Create a rebase multi commit op --- app/src/ui/multi-commit-operation/rebase.tsx | 31 ++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 app/src/ui/multi-commit-operation/rebase.tsx diff --git a/app/src/ui/multi-commit-operation/rebase.tsx b/app/src/ui/multi-commit-operation/rebase.tsx new file mode 100644 index 0000000000..acf8a82c97 --- /dev/null +++ b/app/src/ui/multi-commit-operation/rebase.tsx @@ -0,0 +1,31 @@ +import { MultiCommitOperationKind } from '../../models/multi-commit-operation' +import { BaseRebase } from './base-rebase' + +export abstract class Rebase extends BaseRebase { + protected conflictDialogOperationPrefix = 'rebasing' + + protected onBeginOperation = () => { + const { repository, dispatcher, state } = this.props + const { operationDetail, targetBranch } = state + + if (operationDetail.kind !== MultiCommitOperationKind.Rebase) { + this.endFlowInvalidState() + return + } + + const { commits, sourceBranch } = operationDetail + + if (sourceBranch === null || targetBranch === null) { + this.endFlowInvalidState() + return + } + + return dispatcher.startRebase( + repository, + sourceBranch, + targetBranch, + commits, + { continueWithForcePush: true } + ) + } +} From 2839c3223c3f481faaac8050e9734c7b8c4c29bc Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Mon, 16 Aug 2021 15:34:10 -0400 Subject: [PATCH 04/22] Swap to multi commit operation rebase --- app/src/lib/app-state.ts | 38 +- app/src/lib/multi-commit-operation.ts | 57 ++ app/src/lib/rebase.ts | 67 +- app/src/lib/stats/stats-store.ts | 9 +- app/src/lib/stores/app-store.ts | 598 ++++++------------ app/src/lib/stores/repository-state-cache.ts | 18 - app/src/models/multi-commit-operation.ts | 19 +- app/src/models/popup.ts | 5 - app/src/ui/app.tsx | 154 +---- app/src/ui/dispatcher/dispatcher.ts | 289 ++++----- app/src/ui/dispatcher/error-handlers.ts | 2 +- .../ui/multi-commit-operation/base-rebase.tsx | 24 +- .../base-choose-branch-dialog.tsx | 3 - .../multi-commit-operation.tsx | 3 +- app/src/ui/multi-commit-operation/rebase.tsx | 41 +- 15 files changed, 467 insertions(+), 860 deletions(-) create mode 100644 app/src/lib/multi-commit-operation.ts diff --git a/app/src/lib/app-state.ts b/app/src/lib/app-state.ts index ff7d200573..210cf6cd00 100644 --- a/app/src/lib/app-state.ts +++ b/app/src/lib/app-state.ts @@ -4,7 +4,7 @@ import { IDiff, ImageDiffType } from '../models/diff' import { Repository, ILocalRepositoryState } from '../models/repository' import { Branch, IAheadBehind } from '../models/branch' import { Tip } from '../models/tip' -import { Commit, CommitOneLine } from '../models/commit' +import { Commit } from '../models/commit' import { CommittedFileChange, WorkingDirectoryStatus } from '../models/status' import { CloningRepository } from '../models/cloning-repository' import { IMenu } from '../models/app-menu' @@ -33,7 +33,6 @@ import { ApplicableTheme, ApplicationTheme } from '../ui/lib/application-theme' import { IAccountRepositories } from './stores/api-repositories-store' import { ManualConflictResolution } from '../models/manual-conflict-resolution' import { Banner } from '../models/banner' -import { RebaseFlowStep } from '../models/rebase-flow-step' import { IStashEntry } from '../models/stash-entry' import { TutorialStep } from '../models/tutorial-step' import { UncommittedChangesStrategy } from '../models/uncommitted-changes-strategy' @@ -412,8 +411,6 @@ export interface IRepositoryState { readonly branchesState: IBranchesState - readonly rebaseState: IRebaseState - /** The commits loaded, keyed by their full SHA. */ readonly commitLookup: Map @@ -535,39 +532,6 @@ export interface IBranchesState { readonly rebasedBranches: ReadonlyMap } -/** State associated with a rebase being performed on a repository */ -export interface IRebaseState { - /** - * The current step of the flow the user should see. - * - * `null` indicates that there is no rebase underway. - */ - readonly step: RebaseFlowStep | null - - /** - * The underlying Git information associated with the current rebase - * - * This will be set to `null` when no base branch has been selected to - * initiate the rebase. - */ - readonly progress: IMultiCommitOperationProgress | null - - /** - * The known range of commits that will be applied to the repository - * - * This will be set to `null` when no base branch has been selected to - * initiate the rebase. - */ - readonly commits: ReadonlyArray | null - - /** - * Whether the user has done work to resolve any conflicts as part of this - * rebase, as the rebase flow should confirm the user wishes to abort the - * rebase and lose that work. - */ - readonly userHasResolvedConflicts: boolean -} - export interface ICommitSelection { /** The commits currently selected in the app */ readonly shas: ReadonlyArray diff --git a/app/src/lib/multi-commit-operation.ts b/app/src/lib/multi-commit-operation.ts new file mode 100644 index 0000000000..406d3f0124 --- /dev/null +++ b/app/src/lib/multi-commit-operation.ts @@ -0,0 +1,57 @@ +import { Branch } from '../models/branch' +import { + ChooseBranchStep, + conflictSteps, + MultiCommitOperationStepKind, +} from '../models/multi-commit-operation' +import { Popup, PopupType } from '../models/popup' +import { TipState } from '../models/tip' +import { IMultiCommitOperationState, IRepositoryState } from './app-state' + +/** + * Setup the multi commit operation state when the user needs to select a branch as the + * base for the operation. + */ +export function getMultiCommitOperationChooseBranchStep( + state: IRepositoryState, + initialBranch?: Branch | null +) { + const { + defaultBranch, + allBranches, + recentBranches, + tip, + } = state.branchesState + let currentBranch: Branch | null = null + + if (tip.kind === TipState.Valid) { + currentBranch = tip.branch + } else { + throw new Error( + 'Tip is not in a valid state, which is required to start the rebase flow' + ) + } + + const initialState: ChooseBranchStep = { + kind: MultiCommitOperationStepKind.ChooseBranch, + defaultBranch, + currentBranch, + allBranches, + recentBranches, + initialBranch: initialBranch !== null ? initialBranch : undefined, + } + + return initialState +} + +export function isConflictsFlow( + currentPopup: Popup | null, + multiCommitOperationState: IMultiCommitOperationState | null +): boolean { + return ( + currentPopup !== null && + currentPopup.type === PopupType.MultiCommitOperation && + multiCommitOperationState !== null && + conflictSteps.includes(multiCommitOperationState.step.kind) + ) +} diff --git a/app/src/lib/rebase.ts b/app/src/lib/rebase.ts index b16a55951a..a2ee339e83 100644 --- a/app/src/lib/rebase.ts +++ b/app/src/lib/rebase.ts @@ -1,71 +1,8 @@ -import { - IRepositoryState, - RebaseConflictState, - IBranchesState, -} from '../lib/app-state' -import { - ChooseBranchesStep, - RebaseStep, - ShowConflictsStep, -} from '../models/rebase-flow-step' -import { Branch, IAheadBehind } from '../models/branch' +import { IBranchesState } from '../lib/app-state' +import { IAheadBehind } from '../models/branch' import { TipState } from '../models/tip' import { clamp } from './clamp' -/** - * Setup the rebase flow state when the user needs to select a branch as the - * base for the operation. - */ -export function initializeNewRebaseFlow( - state: IRepositoryState, - initialBranch?: Branch | null -) { - const { - defaultBranch, - allBranches, - recentBranches, - tip, - } = state.branchesState - let currentBranch: Branch | null = null - - if (tip.kind === TipState.Valid) { - currentBranch = tip.branch - } else { - throw new Error( - 'Tip is not in a valid state, which is required to start the rebase flow' - ) - } - - const initialState: ChooseBranchesStep = { - kind: RebaseStep.ChooseBranch, - defaultBranch, - currentBranch, - allBranches, - recentBranches, - initialBranch: initialBranch !== null ? initialBranch : undefined, - } - - return initialState -} - -/** - * Setup the rebase flow when rebase conflicts are detected in the repository. - * - * This indicates a rebase is in progress, and the application needs to guide - * the user to resolve conflicts and complete the rebase. - * - * @param conflictState current set of conflicts - */ -export function initializeRebaseFlowForConflictedRepository( - conflictState: RebaseConflictState -): ShowConflictsStep { - const initialState: ShowConflictsStep = { - kind: RebaseStep.ShowConflicts, - conflictState, - } - return initialState -} - /** * Format rebase percentage to ensure it's a value between 0 and 1, but to also * constrain it to two significant figures, avoiding the remainder that comes diff --git a/app/src/lib/stats/stats-store.ts b/app/src/lib/stats/stats-store.ts index 70c06b8d0e..4102af24fc 100644 --- a/app/src/lib/stats/stats-store.ts +++ b/app/src/lib/stats/stats-store.ts @@ -1609,8 +1609,10 @@ export class StatsStore implements IStatsStore { return this.recordSquashConflictsEncountered() case MultiCommitOperationKind.Reorder: return this.recordReorderConflictsEncountered() - case MultiCommitOperationKind.CherryPick: case MultiCommitOperationKind.Rebase: + // ignored because rebase records different stats + return + case MultiCommitOperationKind.CherryPick: case MultiCommitOperationKind.Merge: log.error( `[recordOperationConflictsEncounteredCount] - Operation not supported: ${kind}` @@ -1632,6 +1634,8 @@ export class StatsStore implements IStatsStore { case MultiCommitOperationKind.CherryPick: return this.recordCherryPickSuccessful() case MultiCommitOperationKind.Rebase: + // ignored because rebase records different stats + return case MultiCommitOperationKind.Merge: log.error( `[recordOperationSuccessful] - Operation not supported: ${kind}` @@ -1650,8 +1654,9 @@ export class StatsStore implements IStatsStore { return this.recordSquashSuccessfulWithConflicts() case MultiCommitOperationKind.Reorder: return this.recordReorderSuccessfulWithConflicts() - case MultiCommitOperationKind.CherryPick: case MultiCommitOperationKind.Rebase: + return this.recordRebaseSuccessAfterConflicts() + case MultiCommitOperationKind.CherryPick: case MultiCommitOperationKind.Merge: log.error( `[recordOperationSuccessfulWithConflicts] - Operation not supported: ${kind}` diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index d9c99a5a03..0b3d3c4c68 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -52,7 +52,7 @@ import { WorkingDirectoryStatus, AppFileStatusKind, } from '../../models/status' -import { TipState, tipEquals, IValidBranch, Tip } from '../../models/tip' +import { TipState, tipEquals, IValidBranch } from '../../models/tip' import { ICommitMessage } from '../../models/commit-message' import { Progress, @@ -98,16 +98,12 @@ import { PossibleSelections, RepositorySectionTab, SelectionType, - MergeConflictState, - RebaseConflictState, - IRebaseState, IRepositoryState, ChangesSelectionKind, ChangesWorkingDirectorySelection, isRebaseConflictState, isCherryPickConflictState, isMergeConflictState, - CherryPickConflictState, IMultiCommitOperationState, } from '../app-state' import { @@ -157,6 +153,7 @@ import { GitResetMode, reset, getBranchAheadBehind, + getRebaseInternalState, } from '../git' import { installGlobalLFSFilters, @@ -172,11 +169,7 @@ import { matchExistingRepository, urlMatchesRemote, } from '../repository-matching' -import { - initializeRebaseFlowForConflictedRepository, - formatRebaseValue, - isCurrentBranchForcePush, -} from '../rebase' +import { isCurrentBranchForcePush } from '../rebase' import { RetryAction, RetryActionType } from '../../models/retry-actions' import { Default as DefaultShell, @@ -194,7 +187,7 @@ import { } from '../window-state' import { TypedBaseStore } from './base-store' import { MergeTreeResult } from '../../models/merge' -import { promiseWithMinimumTimeout, sleep } from '../promise' +import { promiseWithMinimumTimeout } from '../promise' import { BackgroundFetcher } from './helpers/background-fetcher' import { validatedRepositoryPath } from './helpers/validated-repository-path' import { RepositoryStateCache } from './repository-state-cache' @@ -236,7 +229,6 @@ import { defaultUncommittedChangesStrategy, } from '../../models/uncommitted-changes-strategy' import { IStashEntry, StashedChangesLoadStates } from '../../models/stash-entry' -import { RebaseFlowStep, RebaseStep } from '../../models/rebase-flow-step' import { arrayEquals } from '../equality' import { MenuLabelsEvent } from '../../models/menu-labels' import { findRemoteBranchName } from './helpers/find-branch-name' @@ -289,6 +281,7 @@ import { import { reorder } from '../git/reorder' import { DragAndDropIntroType } from '../../ui/history/drag-and-drop-intro' import { UseWindowsOpenSSHKey } from '../ssh/ssh' +import { isConflictsFlow } from '../multi-commit-operation' const LastSelectedRepositoryIDKey = 'last-selected-repository-id' @@ -2009,8 +2002,8 @@ export class AppStore extends TypedBaseStore { conflictState: updateConflictState(state, status, this.statsStore), })) - this.updateRebaseFlowConflictsIfFound(repository) this.updateMultiCommitOperationConflictsIfFound(repository) + this.initializeMultiCommitOperationIfConflictsFound(repository, status) if (this.selectedRepository === repository) { this._triggerConflictsFlow(repository, status) @@ -2024,46 +2017,116 @@ export class AppStore extends TypedBaseStore { } /** - * Push changes from latest conflicts into current rebase flow step, if needed + * This method is to initialize a multi commit operation state on app load + * if conflicts are found but not multi commmit operation exists. */ - private updateRebaseFlowConflictsIfFound(repository: Repository) { - const { changesState, rebaseState } = this.repositoryStateCache.get( - repository - ) - const { conflictState } = changesState + private async initializeMultiCommitOperationIfConflictsFound( + repository: Repository, + status: IStatusResult + ) { + const state = this.repositoryStateCache.get(repository) + const { + changesState: { conflictState }, + multiCommitOperationState, + branchesState, + } = state - if (conflictState === null || !isRebaseConflictState(conflictState)) { + if (conflictState === null) { + this.clearConflictsFlowVisuals(state) return } - const { step } = rebaseState - if (step === null) { + if (multiCommitOperationState !== null) { return } - if ( - step.kind === RebaseStep.ShowConflicts || - step.kind === RebaseStep.ConfirmAbort - ) { - // merge in new conflicts with known branches so they are not forgotten - const { baseBranch, targetBranch } = step.conflictState - const newConflictsState = { - ...conflictState, - baseBranch, - targetBranch, + let operationDetail: MultiCommitOperationDetail + let targetBranch: Branch | null = null + let commits: ReadonlyArray = [] + let originalBranchTip: string | null = '' + let progress: IMultiCommitOperationProgress | undefined = undefined + + if (branchesState.tip.kind === TipState.Valid) { + targetBranch = branchesState.tip.branch + originalBranchTip = targetBranch.tip.sha + } + + if (isMergeConflictState(conflictState)) { + operationDetail = { + kind: MultiCommitOperationKind.Merge, + isSquash: status.squashMsgFound, + sourceBranch: null, + } + originalBranchTip = targetBranch !== null ? targetBranch.tip.sha : null + } else if (isRebaseConflictState(conflictState)) { + const snapshot = await getRebaseSnapshot(repository) + const rebaseState = await getRebaseInternalState(repository) + if (snapshot === null || rebaseState === null) { + return } - this.repositoryStateCache.updateRebaseState(repository, () => ({ - step: { ...step, conflictState: newConflictsState }, - })) - } - } + originalBranchTip = rebaseState.originalBranchTip + commits = snapshot.commits + progress = snapshot.progress + operationDetail = { + kind: MultiCommitOperationKind.Rebase, + sourceBranch: null, + commits, + currentTip: rebaseState.baseBranchTip, + } + } else if (isCherryPickConflictState(conflictState)) { + const snapshot = await getCherryPickSnapshot(repository) + if (snapshot === null) { + return + } + originalBranchTip = null + commits = snapshot.commits + progress = snapshot.progress + operationDetail = { + kind: MultiCommitOperationKind.CherryPick, + sourceBranch: null, + branchCreated: false, + commits, + } + + this.repositoryStateCache.updateMultiCommitOperationUndoState( + repository, + () => ({ + undoSha: snapshot.targetBranchUndoSha, + branchName: '', + }) + ) + } else { + assertNever(conflictState, `Unsupported conflict kind`) + } + + this._initializeMultiCommitOperation( + repository, + operationDetail, + targetBranch, + commits, + originalBranchTip, + false + ) + + if (progress === undefined) { + return + } + + this.repositoryStateCache.updateMultiCommitOperationState( + repository, + () => ({ + progress: progress as IMultiCommitOperationProgress, + }) + ) + } /** * Push changes from latest conflicts into current multi step operation step, if needed * - i.e. - multiple instance of running in to conflicts */ private updateMultiCommitOperationConflictsIfFound(repository: Repository) { + const state = this.repositoryStateCache.get(repository) const { changesState, multiCommitOperationState, @@ -2071,6 +2134,7 @@ export class AppStore extends TypedBaseStore { const { conflictState } = changesState if (conflictState === null || multiCommitOperationState === null) { + this.clearConflictsFlowVisuals(state) return } @@ -2105,7 +2169,6 @@ export class AppStore extends TypedBaseStore { const { changesState: { conflictState }, multiCommitOperationState, - branchesState, } = state if (conflictState === null) { @@ -2113,138 +2176,70 @@ export class AppStore extends TypedBaseStore { return } - if (isMergeConflictState(conflictState)) { - await this.showMergeConflictsDialog( - repository, - conflictState, - multiCommitOperationState, - branchesState.tip, - status.squashMsgFound - ) - } else if (isRebaseConflictState(conflictState)) { - // TODO: This will likely get refactored to a showConflictsDialog method - const invalidOperationKinds = new Set([ - MultiCommitOperationKind.Squash, - MultiCommitOperationKind.Reorder, - ]) - if ( - multiCommitOperationState === null || - !invalidOperationKinds.has( - multiCommitOperationState.operationDetail.kind - ) - ) { - await this.showRebaseConflictsDialog(repository, conflictState) - } - } else if (isCherryPickConflictState(conflictState)) { - await this.showCherryPickConflictsDialog( - repository, - conflictState, - multiCommitOperationState, - branchesState.tip - ) - } else { - assertNever(conflictState, `Unsupported conflict kind`) - } - } - - /** - * Cleanup any related UI related to conflicts if still in use. - */ - private clearConflictsFlowVisuals(state: IRepositoryState) { - const { multiCommitOperationState, rebaseState } = state - if ( - userIsStartingRebaseFlow(this.currentPopup, rebaseState) || - userIsStartingMultiCommitOperation( - this.currentPopup, - multiCommitOperationState - ) - ) { - return - } - - this._closePopup(PopupType.MultiCommitOperation) - this._clearBanner(BannerType.MergeConflictsFound) - - this._closePopup(PopupType.RebaseFlow) - this._clearBanner(BannerType.RebaseConflictsFound) - } - - /** display the rebase flow, if not already in this flow */ - private async showRebaseConflictsDialog( - repository: Repository, - conflictState: RebaseConflictState - ) { - const alreadyInFlow = - this.currentPopup !== null && - this.currentPopup.type === PopupType.RebaseFlow - - if (alreadyInFlow) { + if (multiCommitOperationState === null) { return } const displayingBanner = this.currentBanner !== null && - this.currentBanner.type === BannerType.RebaseConflictsFound + this.currentBanner.type === BannerType.ConflictsFound - if (displayingBanner) { + if ( + displayingBanner || + isConflictsFlow(this.currentPopup, multiCommitOperationState) + ) { return } - await this._setRebaseProgressFromState(repository) + const { manualResolutions } = conflictState + let ourBranch, theirBranch - const step = initializeRebaseFlowForConflictedRepository(conflictState) + if (isMergeConflictState(conflictState)) { + theirBranch = await this.getMergeConflictsTheirBranch( + repository, + status.squashMsgFound, + multiCommitOperationState + ) + ourBranch = conflictState.currentBranch + } else if (isRebaseConflictState(conflictState)) { + theirBranch = conflictState.targetBranch + ourBranch = conflictState.baseBranch + } else if (isCherryPickConflictState(conflictState)) { + if ( + multiCommitOperationState !== null && + multiCommitOperationState.operationDetail.kind === + MultiCommitOperationKind.CherryPick && + multiCommitOperationState.operationDetail.sourceBranch !== null + ) { + theirBranch = + multiCommitOperationState.operationDetail.sourceBranch.name + } + ourBranch = conflictState.targetBranchName + } else { + assertNever(conflictState, `Unsupported conflict kind`) + } - this.repositoryStateCache.updateRebaseState(repository, () => ({ - step, - })) + this._setMultiCommitOperationStep(repository, { + kind: MultiCommitOperationStepKind.ShowConflicts, + conflictState: { + kind: 'multiCommitOperation', + manualResolutions, + ourBranch, + theirBranch, + }, + }) this._showPopup({ - type: PopupType.RebaseFlow, + type: PopupType.MultiCommitOperation, repository, }) } - /** starts the conflict resolution flow, if appropriate */ - private async showMergeConflictsDialog( + private async getMergeConflictsTheirBranch( repository: Repository, - conflictState: MergeConflictState, - multiCommitOperationState: IMultiCommitOperationState | null, - tip: Tip, - isSquash: boolean - ) { - if (multiCommitOperationState === null && tip.kind === TipState.Valid) { - this._initializeMultiCommitOperation( - repository, - { - kind: MultiCommitOperationKind.Merge, - isSquash, - sourceBranch: null, - }, - tip.branch, - [], - tip.branch.tip.sha - ) - } - - // are we already in the merge conflicts flow? - const alreadyInFlow = - this.currentPopup !== null && - this.currentPopup.type === PopupType.MultiCommitOperation && - multiCommitOperationState !== null && - (multiCommitOperationState.step.kind === - MultiCommitOperationStepKind.ShowConflicts || - multiCommitOperationState.step.kind === - MultiCommitOperationStepKind.ConfirmAbort) - - // have we already been shown the merge conflicts flow *and closed it*? - const alreadyExitedFlow = - this.currentBanner !== null && - this.currentBanner.type === BannerType.ConflictsFound - - if (alreadyInFlow || alreadyExitedFlow) { - return - } - + isSquash: boolean, + multiCommitOperationState: IMultiCommitOperationState | null + ): Promise { let theirBranch: string | undefined if ( multiCommitOperationState !== null && @@ -2271,22 +2266,26 @@ export class AppStore extends TypedBaseStore { ? possibleTheirsBranches[0] : undefined } + return theirBranch + } - const { manualResolutions, currentBranch: ourBranch } = conflictState - this._setMultiCommitOperationStep(repository, { - kind: MultiCommitOperationStepKind.ShowConflicts, - conflictState: { - kind: 'multiCommitOperation', - manualResolutions, - ourBranch, - theirBranch, - }, - }) + /** + * Cleanup any related UI related to conflicts if still in use. + */ + private clearConflictsFlowVisuals(state: IRepositoryState) { + const { multiCommitOperationState } = state + if ( + userIsStartingMultiCommitOperation( + this.currentPopup, + multiCommitOperationState + ) + ) { + return + } - this._showPopup({ - type: PopupType.MultiCommitOperation, - repository, - }) + this._closePopup(PopupType.MultiCommitOperation) + this._clearBanner(BannerType.ConflictsFound) + this._clearBanner(BannerType.MergeConflictsFound) } /** This shouldn't be called directly. See `Dispatcher`. */ @@ -4627,89 +4626,17 @@ export class AppStore extends TypedBaseStore { return this._refreshRepository(repository) } - /** This shouldn't be called directly. See `Dispatcher`. */ - public async _setRebaseProgressFromState(repository: Repository) { - const snapshot = await getRebaseSnapshot(repository) - if (snapshot === null) { - return - } - - const { progress, commits } = snapshot - - this.repositoryStateCache.updateRebaseState(repository, () => { - return { - progress, - commits, - } - }) - } - - /** This shouldn't be called directly. See `Dispatcher`. */ - public _initializeRebaseProgress( - repository: Repository, - commits: ReadonlyArray - ) { - this.repositoryStateCache.updateRebaseState(repository, () => { - const hasCommits = commits.length > 0 - const firstCommitSummary = hasCommits ? commits[0].summary : null - - return { - progress: { - kind: 'multiCommitOperation', - value: formatRebaseValue(0), - position: 0, - currentCommitSummary: - firstCommitSummary !== null ? firstCommitSummary : '', - totalCommitCount: commits.length, - }, - commits, - } - }) - - this.emitUpdate() - } - /** This shouldn't be called directly. See `Dispatcher`. */ public _setConflictsResolved(repository: Repository) { // an update is not emitted here because there is no need // to trigger a re-render at this point - this.repositoryStateCache.updateRebaseState(repository, () => ({ - userHasResolvedConflicts: true, - })) - } - - /** This shouldn't be called directly. See `Dispatcher`. */ - public async _setRebaseFlowStep( - repository: Repository, - step: RebaseFlowStep - ): Promise { - this.repositoryStateCache.updateRebaseState(repository, () => ({ - step, - })) - - this.emitUpdate() - - if (step.kind === RebaseStep.ShowProgress && step.rebaseAction !== null) { - // this timeout is intended to defer the action from running immediately - // after the progress UI is shown, to better show that rebase is - // progressing rather than suddenly appearing and disappearing again - await sleep(500) - await step.rebaseAction() - } - } - - /** This shouldn't be called directly. See `Dispatcher`. */ - public _endRebaseFlow(repository: Repository) { - this.repositoryStateCache.updateRebaseState(repository, () => ({ - step: null, - progress: null, - commits: null, - preview: null, - userHasResolvedConflicts: false, - })) - - this.emitUpdate() + this.repositoryStateCache.updateMultiCommitOperationState( + repository, + () => ({ + userHasResolvedConflicts: true, + }) + ) } /** This shouldn't be called directly. See `Dispatcher`. */ @@ -4718,14 +4645,9 @@ export class AppStore extends TypedBaseStore { baseBranch: Branch, targetBranch: Branch ): Promise { - const progressCallback = (progress: IMultiCommitOperationProgress) => { - this.repositoryStateCache.updateRebaseState(repository, () => ({ - progress, - })) - - this.emitUpdate() - } - + const progressCallback = this.getMultiCommitOperationProgressCallBack( + repository + ) const gitStore = this.gitStoreCache.get(repository) const result = await gitStore.performFailableOperation( () => rebase(repository, baseBranch, targetBranch, progressCallback), @@ -4756,13 +4678,9 @@ export class AppStore extends TypedBaseStore { workingDirectory: WorkingDirectoryStatus, manualResolutions: ReadonlyMap ): Promise { - const progressCallback = (progress: IMultiCommitOperationProgress) => { - this.repositoryStateCache.updateRebaseState(repository, () => ({ - progress, - })) - - this.emitUpdate() - } + const progressCallback = this.getMultiCommitOperationProgressCallBack( + repository + ) const gitStore = this.gitStoreCache.get(repository) const result = await gitStore.performFailableOperation(() => @@ -5921,35 +5839,11 @@ export class AppStore extends TypedBaseStore { } }) - this.updateRebaseStateAfterManualResolution(repository) this.updateMultiCommitOperationStateAfterManualResolution(repository) this.emitUpdate() } - /** - * Updates the rebase flow conflict step state as the manual resolutions - * have been changed. - */ - private updateRebaseStateAfterManualResolution(repository: Repository) { - const currentState = this.repositoryStateCache.get(repository) - - const { changesState, rebaseState } = currentState - const { conflictState } = changesState - const { step } = rebaseState - - if ( - conflictState !== null && - conflictState.kind === 'rebase' && - step !== null && - step.kind === RebaseStep.ShowConflicts - ) { - this.repositoryStateCache.updateRebaseState(repository, () => ({ - step: { ...step, conflictState }, - })) - } - } - /** * Updates the multi commit operation conflict step state as the manual * resolutions have been changed. @@ -6362,105 +6256,6 @@ export class AppStore extends TypedBaseStore { ) } - /** display the cherry pick flow, if not already in this flow */ - private async showCherryPickConflictsDialog( - repository: Repository, - conflictState: CherryPickConflictState, - multiCommitOperationState: IMultiCommitOperationState | null, - tip: Tip - ) { - const snapshot = await getCherryPickSnapshot(repository) - - if (snapshot === null) { - log.error( - `[showCherryPickConflictsDialog] unable to get cherry-pick status from git, unable to continue` - ) - return - } - - if (multiCommitOperationState === null && tip.kind === TipState.Valid) { - // This is only true is we get here when opening the app and therefore we - // don't know some of this data - this._initializeMultiCommitOperation( - repository, - { - kind: MultiCommitOperationKind.CherryPick, - sourceBranch: null, - branchCreated: false, - commits: snapshot.commits, - }, - tip.branch, - snapshot.commits, - null - ) - - this.repositoryStateCache.updateMultiCommitOperationUndoState( - repository, - () => ({ - undoSha: snapshot.targetBranchUndoSha, - branchName: '', - }) - ) - - this.repositoryStateCache.updateMultiCommitOperationState( - repository, - () => ({ - progress: snapshot.progress, - }) - ) - } - - // are we already in the conflicts flow? - const alreadyInFlow = - this.currentPopup !== null && - this.currentPopup.type === PopupType.MultiCommitOperation && - multiCommitOperationState !== null && - (multiCommitOperationState.step.kind === - MultiCommitOperationStepKind.ShowConflicts || - multiCommitOperationState.step.kind === - MultiCommitOperationStepKind.ConfirmAbort) - - // have we already been shown the merge conflicts flow *and closed it*? - const alreadyExitedFlow = - this.currentBanner !== null && - this.currentBanner.type === BannerType.ConflictsFound - - const displayingBanner = - this.currentBanner !== null && - this.currentBanner.type === BannerType.CherryPickConflictsFound - - if (alreadyInFlow || alreadyExitedFlow || displayingBanner) { - return - } - - let theirBranch = undefined - - if ( - multiCommitOperationState !== null && - multiCommitOperationState.operationDetail.kind === - MultiCommitOperationKind.CherryPick && - multiCommitOperationState.operationDetail.sourceBranch !== null - ) { - theirBranch = multiCommitOperationState.operationDetail.sourceBranch.name - } - - const { manualResolutions, targetBranchName: ourBranch } = conflictState - this._setMultiCommitOperationStep(repository, { - kind: MultiCommitOperationStepKind.ShowConflicts, - conflictState: { - kind: 'multiCommitOperation', - manualResolutions, - ourBranch, - theirBranch, - }, - }) - - this._showPopup({ - type: PopupType.MultiCommitOperation, - repository, - }) - } - /** This shouldn't be called directly. See `Dispatcher`. */ public _markDragAndDropIntroAsSeen(intro: DragAndDropIntroType) { if (this.dragAndDropIntroTypesShown.has(intro)) { @@ -6863,7 +6658,8 @@ export class AppStore extends TypedBaseStore { operationDetail: MultiCommitOperationDetail, targetBranch: Branch | null, commits: ReadonlyArray, - originalBranchTip: string | null + originalBranchTip: string | null, + emitUpdate: boolean = true ): void { this.repositoryStateCache.initializeMultiCommitOperationState(repository, { step: { @@ -6882,6 +6678,10 @@ export class AppStore extends TypedBaseStore { targetBranch, }) + if (!emitUpdate) { + return + } + this.emitUpdate() } } @@ -6909,38 +6709,6 @@ function getInitialAction( } } -/** - * Check if the user is in a rebase flow step that doesn't depend on conflicted - * state, as the app should not attempt to clean up any popups or banners while - * this is occurring. - */ -function userIsStartingRebaseFlow( - currentPopup: Popup | null, - state: IRebaseState -) { - if (currentPopup === null) { - return false - } - - if (currentPopup.type !== PopupType.RebaseFlow) { - return false - } - - if (state.step === null) { - return false - } - - if ( - state.step.kind === RebaseStep.ChooseBranch || - state.step.kind === RebaseStep.WarnForcePush || - state.step.kind === RebaseStep.ShowProgress - ) { - return true - } - - return false -} - function userIsStartingMultiCommitOperation( currentPopup: Popup | null, state: IMultiCommitOperationState | null diff --git a/app/src/lib/stores/repository-state-cache.ts b/app/src/lib/stores/repository-state-cache.ts index ecb808d7c6..a52d8757cb 100644 --- a/app/src/lib/stores/repository-state-cache.ts +++ b/app/src/lib/stores/repository-state-cache.ts @@ -15,7 +15,6 @@ import { IRepositoryState, RepositorySectionTab, ICommitSelection, - IRebaseState, ChangesSelectionKind, IMultiCommitOperationUndoState, IMultiCommitOperationState, @@ -109,17 +108,6 @@ export class RepositoryStateCache { }) } - public updateRebaseState( - repository: Repository, - fn: (branchesState: IRebaseState) => Pick - ) { - this.update(repository, state => { - const { rebaseState } = state - const newState = merge(rebaseState, fn(rebaseState)) - return { rebaseState: newState } - }) - } - public updateMultiCommitOperationUndoState< K extends keyof IMultiCommitOperationUndoState >( @@ -225,12 +213,6 @@ function getInitialRepositoryState(): IRepositoryState { recentBranches: new Array(), defaultBranch: null, }, - rebaseState: { - step: null, - progress: null, - commits: null, - userHasResolvedConflicts: false, - }, commitAuthor: null, commitLookup: new Map(), localCommitSHAs: [], diff --git a/app/src/models/multi-commit-operation.ts b/app/src/models/multi-commit-operation.ts index 7b9588c48f..4d99b530e7 100644 --- a/app/src/models/multi-commit-operation.ts +++ b/app/src/models/multi-commit-operation.ts @@ -134,7 +134,7 @@ export type CreateBranchStep = { targetBranchName: string } -interface IBaseRebaseDetails { +interface IBaseInteractiveRebaseDetails { /** * Array of commits used during the operation. */ @@ -147,7 +147,7 @@ interface IBaseRebaseDetails { readonly currentTip: string } -interface IInteractiveRebaseDetails extends IBaseRebaseDetails { +interface IInteractiveRebaseDetails extends IBaseInteractiveRebaseDetails { /** * The reference to the last retained commit on the branch during an * interactive rebase or null if rebasing to the root. @@ -203,8 +203,14 @@ interface ICherryPickDetails extends ISourceBranchDetails { readonly commits: ReadonlyArray } -interface IRebaseDetails extends ISourceBranchDetails, IBaseRebaseDetails { +interface IRebaseDetails extends ISourceBranchDetails { readonly kind: MultiCommitOperationKind.Rebase + readonly commits: ReadonlyArray + /** + * This is the commit sha of the HEAD of the in-flight operation used to compare + * the state of the after an operation to a previous state. + */ + readonly currentTip: string } interface IMergeDetails extends ISourceBranchDetails { @@ -221,6 +227,11 @@ export type MultiCommitOperationDetail = export function instanceOfIBaseRebaseDetails( object: any -): object is IBaseRebaseDetails { +): object is IBaseInteractiveRebaseDetails { return 'commits' in object && 'currentTip' in object } + +export const conflictSteps = [ + MultiCommitOperationStepKind.ShowConflicts, + MultiCommitOperationStepKind.ConfirmAbort, +] diff --git a/app/src/models/popup.ts b/app/src/models/popup.ts index 50429dace9..93d2b23028 100644 --- a/app/src/models/popup.ts +++ b/app/src/models/popup.ts @@ -51,7 +51,6 @@ export enum PopupType { OversizedFiles, CommitConflictsWarning, PushNeedsPull, - RebaseFlow, ConfirmForcePush, StashAndSwitchBranch, ConfirmOverwriteStash, @@ -195,10 +194,6 @@ export type Popup = repository: Repository upstreamBranch: string } - | { - type: PopupType.RebaseFlow - repository: Repository - } | { type: PopupType.StashAndSwitchBranch repository: Repository diff --git a/app/src/ui/app.tsx b/app/src/ui/app.tsx index 990df5baf8..d76baf6f29 100644 --- a/app/src/ui/app.tsx +++ b/app/src/ui/app.tsx @@ -8,7 +8,6 @@ import { FoldoutType, SelectionType, HistoryTabMode, - isRebaseConflictState, } from '../lib/app-state' import { Dispatcher } from './dispatcher' import { AppStore, GitHubUserStore, IssuesStore } from '../lib/stores' @@ -92,11 +91,7 @@ import { RepositoryStateCache } from '../lib/stores/repository-state-cache' import { PopupType, Popup } from '../models/popup' import { OversizedFiles } from './changes/oversized-files-warning' import { PushNeedsPullWarning } from './push-needs-pull' -import { RebaseFlow, ConfirmForcePush } from './rebase' -import { - initializeRebaseFlowForConflictedRepository, - isCurrentBranchForcePush, -} from '../lib/rebase' +import { isCurrentBranchForcePush } from '../lib/rebase' import { Banner, BannerType } from '../models/banner' import { StashAndSwitchBranch } from './stash-changes/stash-and-switch-branch-dialog' import { OverwriteStash } from './stash-changes/overwrite-stashed-changes-dialog' @@ -141,13 +136,11 @@ import { MultiCommitOperation } from './multi-commit-operation/multi-commit-oper import { WarnLocalChangesBeforeUndo } from './undo/warn-local-changes-before-undo' import { WarningBeforeReset } from './reset/warning-before-reset' import { InvalidatedToken } from './invalidated-token/invalidated-token' -import { - ChooseBranchStep, - MultiCommitOperationKind, - MultiCommitOperationStepKind, -} from '../models/multi-commit-operation' +import { MultiCommitOperationKind } from '../models/multi-commit-operation' import { AddSSHHost } from './ssh/add-ssh-host' import { SSHKeyPassphrase } from './ssh/ssh-key-passphrase' +import { getMultiCommitOperationChooseBranchStep } from '../lib/multi-commit-operation' +import { ConfirmForcePush } from './rebase/confirm-force-push' const MinuteInMilliseconds = 1000 * 60 const HourInMilliseconds = MinuteInMilliseconds * 60 @@ -1118,6 +1111,33 @@ export class App extends React.Component { if (!repository || repository instanceof CloningRepository) { return } + + const repositoryState = this.props.repositoryStateManager.get(repository) + + const { tip } = repositoryState.branchesState + let currentBranch: Branch | null = null + + if (tip.kind === TipState.Valid) { + currentBranch = tip.branch + } else { + throw new Error( + 'Tip is not in a valid state, which is required to start the rebase flow' + ) + } + + this.props.dispatcher.initializeMultiCommitOperation( + repository, + { + kind: MultiCommitOperationKind.Rebase, + sourceBranch: null, + commits: [], + currentTip: tip.branch.tip.sha, + }, + currentBranch, + [], + currentBranch.tip.sha + ) + this.props.dispatcher.showRebaseDialog(repository) } @@ -1718,63 +1738,6 @@ export class App extends React.Component { onDismissed={onPopupDismissedFn} /> ) - case PopupType.RebaseFlow: { - const { selectedState, emoji } = this.state - - if ( - selectedState === null || - selectedState.type !== SelectionType.Repository - ) { - return null - } - - const { - changesState, - rebaseState, - multiCommitOperationState, - } = selectedState.state - const { workingDirectory, conflictState } = changesState - const { progress, step, userHasResolvedConflicts } = rebaseState - - if ( - (conflictState !== null && conflictState.kind !== 'rebase') || - multiCommitOperationState !== null - ) { - log.warn( - '[App] invalid state encountered - rebase flow should not be used when merge conflicts found' - ) - return null - } - - if (step === null) { - log.warn( - '[App] invalid state encountered - rebase flow should not be active when step is null' - ) - return null - } - - return ( - - ) - } case PopupType.ConfirmForcePush: { const { askForConfirmationOnForcePush } = this.state @@ -2127,46 +2090,6 @@ export class App extends React.Component { this.props.dispatcher.createTutorialRepository(account) } - private onShowRebaseConflictsBanner = ( - repository: Repository, - targetBranch: string - ) => { - this.props.dispatcher.setBanner({ - type: BannerType.RebaseConflictsFound, - targetBranch, - onOpenDialog: async () => { - const { changesState } = this.props.repositoryStateManager.get( - repository - ) - const { conflictState } = changesState - - if (conflictState === null || !isRebaseConflictState(conflictState)) { - log.debug( - `[App.onShowRebaseConflictsBanner] no rebase conflict state found, ignoring...` - ) - return - } - - await this.props.dispatcher.setRebaseProgressFromState(repository) - - const initialStep = initializeRebaseFlowForConflictedRepository( - conflictState - ) - - this.props.dispatcher.setRebaseFlowStep(repository, initialStep) - - this.props.dispatcher.showPopup({ - type: PopupType.RebaseFlow, - repository, - }) - }, - }) - } - - private onRebaseFlowEnded = (repository: Repository) => { - this.props.dispatcher.endRebaseFlow(repository) - } - private onUpdateExistingUpstreamRemote = (repository: Repository) => { this.props.dispatcher.updateExistingUpstreamRemote(repository) } @@ -2869,12 +2792,7 @@ export class App extends React.Component { ) => { const repositoryState = this.props.repositoryStateManager.get(repository) - const { - defaultBranch, - allBranches, - recentBranches, - tip, - } = repositoryState.branchesState + const { tip } = repositoryState.branchesState let currentBranch: Branch | null = null if (tip.kind === TipState.Valid) { @@ -2898,13 +2816,7 @@ export class App extends React.Component { tip.branch.tip.sha ) - const initialStep: ChooseBranchStep = { - kind: MultiCommitOperationStepKind.ChooseBranch, - defaultBranch, - currentBranch, - allBranches, - recentBranches, - } + const initialStep = getMultiCommitOperationChooseBranchStep(repositoryState) this.props.dispatcher.setMultiCommitOperationStep(repository, initialStep) this.props.dispatcher.recordCherryPickViaContextMenu() diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index d66476cd4a..4983b70a95 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -31,6 +31,7 @@ import { PushOptions, getCommitsBetweenCommits, getBranches, + getRebaseSnapshot, } from '../../lib/git' import { isGitOnPath } from '../../lib/is-git-on-path' import { @@ -53,10 +54,6 @@ import { AppStore } from '../../lib/stores/app-store' import { validatedRepositoryPath } from '../../lib/stores/helpers/validated-repository-path' import { RepositoryStateCache } from '../../lib/stores/repository-state-cache' import { getTipSha } from '../../lib/tip' -import { - initializeNewRebaseFlow, - initializeRebaseFlowForConflictedRepository, -} from '../../lib/rebase' import { Account } from '../../models/account' import { AppMenu, ExecutableMenuItem } from '../../models/app-menu' @@ -100,7 +97,6 @@ import { } from '../../lib/stores/commit-status-store' import { MergeTreeResult } from '../../models/merge' import { UncommittedChangesStrategy } from '../../models/uncommitted-changes-strategy' -import { RebaseFlowStep, RebaseStep } from '../../models/rebase-flow-step' import { IStashEntry } from '../../models/stash-entry' import { WorkflowPreferences } from '../../models/workflow-preferences' import { resolveWithin } from '../../lib/path' @@ -118,6 +114,7 @@ import { MultiCommitOperationStepKind, } from '../../models/multi-commit-operation' import { DragAndDropIntroType } from '../history/drag-and-drop-intro' +import { getMultiCommitOperationChooseBranchStep } from '../../lib/multi-commit-operation' /** * An error handler function. @@ -440,12 +437,15 @@ export class Dispatcher { initialBranch?: Branch | null ) { const repositoryState = this.repositoryStateManager.get(repository) - const initialStep = initializeNewRebaseFlow(repositoryState, initialBranch) + const initialStep = getMultiCommitOperationChooseBranchStep( + repositoryState, + initialBranch + ) - this.setRebaseFlowStep(repository, initialStep) + this.setMultiCommitOperationStep(repository, initialStep) this.showPopup({ - type: PopupType.RebaseFlow, + type: PopupType.MultiCommitOperation, repository, }) } @@ -463,6 +463,22 @@ export class Dispatcher { const hasOverriddenForcePushCheck = options !== undefined && options.continueWithForcePush + const { branchesState } = this.repositoryStateManager.get(repository) + const originalBranchTip = getTipSha(branchesState.tip) + + this.appStore._initializeMultiCommitOperation( + repository, + { + kind: MultiCommitOperationKind.Rebase, + commits, + currentTip: baseBranch.tip.sha, + sourceBranch: baseBranch, + }, + targetBranch, + commits, + originalBranchTip + ) + if (askForConfirmationOnForcePush && !hasOverriddenForcePushCheck) { const showWarning = await this.warnAboutRemoteCommits( repository, @@ -471,32 +487,26 @@ export class Dispatcher { ) if (showWarning) { - this.setRebaseFlowStep(repository, { - kind: RebaseStep.WarnForcePush, - baseBranch, + this.setMultiCommitOperationStep(repository, { + kind: MultiCommitOperationStepKind.WarnForcePush, targetBranch, + baseBranch, commits, }) return } } - this.initializeRebaseProgress(repository, commits) - - const startRebaseAction = () => { - return this.rebase(repository, baseBranch, targetBranch) - } - - this.setRebaseFlowStep(repository, { - kind: RebaseStep.ShowProgress, - rebaseAction: startRebaseAction, - }) + await this.rebase(repository, baseBranch, targetBranch) } /** * Initialize and launch the rebase flow for a conflicted repository */ - public async launchRebaseFlow(repository: Repository, targetBranch: string) { + public async launchRebaseOperation( + repository: Repository, + targetBranch: string + ) { await this.appStore._loadStatus(repository) const repositoryState = this.repositoryStateManager.get(repository) @@ -515,16 +525,45 @@ export class Dispatcher { conflictState: updatedConflictState, })) - await this.setRebaseProgressFromState(repository) + const snapshot = await getRebaseSnapshot(repository) + if (snapshot === null) { + return + } - const initialStep = initializeRebaseFlowForConflictedRepository( - updatedConflictState + const { progress, commits } = snapshot + this.initializeMultiCommitOperation( + repository, + { + kind: MultiCommitOperationKind.Rebase, + sourceBranch: null, + commits, + currentTip: '', + }, + null, + commits, + null ) - this.setRebaseFlowStep(repository, initialStep) + this.repositoryStateManager.updateMultiCommitOperationState( + repository, + () => ({ + progress, + }) + ) + + const { manualResolutions } = conflictState + this.setMultiCommitOperationStep(repository, { + kind: MultiCommitOperationStepKind.ShowConflicts, + conflictState: { + kind: 'multiCommitOperation', + manualResolutions, + ourBranch: targetBranch, + theirBranch: undefined, + }, + }) this.showPopup({ - type: PopupType.RebaseFlow, + type: PopupType.MultiCommitOperation, repository, }) } @@ -1016,52 +1055,27 @@ export class Dispatcher { return this.appStore._setConflictsResolved(repository) } - /** - * Initialize the progress in application state based on the known commits - * that will be applied in the rebase. - * - * @param commits the list of commits that exist on the target branch which do - * not exist on the base branch - */ - public initializeRebaseProgress( - repository: Repository, - commits: ReadonlyArray - ) { - return this.appStore._initializeRebaseProgress(repository, commits) - } - - /** - * Update the rebase progress in application state by querying the Git - * repository state. - */ - public setRebaseProgressFromState(repository: Repository) { - return this.appStore._setRebaseProgressFromState(repository) - } - - /** - * Move the rebase flow to a new state. - */ - public setRebaseFlowStep( - repository: Repository, - step: RebaseFlowStep - ): Promise { - return this.appStore._setRebaseFlowStep(repository, step) - } - - /** End the rebase flow and cleanup any related app state */ - public endRebaseFlow(repository: Repository) { - return this.appStore._endRebaseFlow(repository) - } - /** Starts a rebase for the given base and target branch */ public async rebase( repository: Repository, baseBranch: Branch, targetBranch: Branch ): Promise { - const stateBefore = this.repositoryStateManager.get(repository) + const { + branchesState, + multiCommitOperationState, + } = this.repositoryStateManager.get(repository) - const beforeSha = getTipSha(stateBefore.branchesState.tip) + if ( + multiCommitOperationState == null || + multiCommitOperationState.operationDetail.kind !== + MultiCommitOperationKind.Rebase + ) { + return + } + const { commits } = multiCommitOperationState.operationDetail + + const beforeSha = getTipSha(branchesState.tip) log.info( `[rebase] starting rebase for ${targetBranch.name} at ${beforeSha}` @@ -1104,13 +1118,12 @@ export class Dispatcher { return } - const conflictsWithBranches: RebaseConflictState = { - ...conflictState, - baseBranch: baseBranch.name, - targetBranch: targetBranch.name, - } - - this.switchToConflicts(repository, conflictsWithBranches) + return this.startMultiCommitOperationConflictFlow( + MultiCommitOperationKind.Rebase, + repository, + baseBranch.name, + targetBranch.name + ) } else if (result === RebaseResult.CompletedWithoutError) { if (tip.kind !== TipState.Valid) { log.warn( @@ -1120,22 +1133,12 @@ export class Dispatcher { } this.statsStore.recordRebaseSuccessWithoutConflicts() - - await this.completeRebase( - repository, - { - type: BannerType.SuccessfulRebase, - targetBranch: targetBranch.name, - baseBranch: baseBranch.name, - }, - tip, - beforeSha - ) + await this.completeMultiCommitOperation(repository, commits.length) } else if (result === RebaseResult.Error) { // we were unable to successfully start the rebase, and an error should // be shown through the default error handling infrastructure, so we can // just abandon the rebase for now - this.endRebaseFlow(repository) + this.endMultiCommitOperation(repository) } } @@ -1186,93 +1189,6 @@ export class Dispatcher { return result } - public processContinueRebaseResult( - result: RebaseResult, - conflictsState: RebaseConflictState, - repository: Repository - ) { - const stateAfter = this.repositoryStateManager.get(repository) - const { tip } = stateAfter.branchesState - const { targetBranch, baseBranch, originalBranchTip } = conflictsState - - if (result === RebaseResult.ConflictsEncountered) { - const { conflictState } = stateAfter.changesState - if (conflictState === null) { - log.warn( - `[continueRebase] conflict state after rebase is null - unable to continue` - ) - return - } - - if (!isRebaseConflictState(conflictState)) { - log.warn( - `[continueRebase] conflict state after rebase is not rebase conflicts - unable to continue` - ) - return - } - - // ensure branches are persisted when transitioning back to conflicts - const conflictsWithBranches: RebaseConflictState = { - ...conflictState, - baseBranch, - targetBranch, - } - - return this.switchToConflicts(repository, conflictsWithBranches) - } else if (result === RebaseResult.CompletedWithoutError) { - if (tip.kind !== TipState.Valid) { - log.warn( - `[continueRebase] tip after completing rebase is ${tip.kind} but this should be a valid tip if the rebase completed without error` - ) - return - } - - this.statsStore.recordRebaseSuccessAfterConflicts() - - return this.completeRebase( - repository, - { - type: BannerType.SuccessfulRebase, - targetBranch: targetBranch, - baseBranch: baseBranch, - }, - tip, - originalBranchTip - ) - } - } - - /** Switch the rebase flow to show the latest conflicts */ - private switchToConflicts = ( - repository: Repository, - conflictState: RebaseConflictState - ) => { - this.setRebaseFlowStep(repository, { - kind: RebaseStep.ShowConflicts, - conflictState, - }) - } - - /** Tidy up the rebase flow after reaching the end */ - private async completeRebase( - repository: Repository, - banner: Banner, - tip: IValidBranch, - originalBranchTip: string - ): Promise { - this.closePopup() - - this.setBanner(banner) - - if (tip.kind === TipState.Valid) { - this.addRebasedBranchToForcePushList(repository, tip, originalBranchTip) - } - - this.endRebaseFlow(repository) - - await this.refreshRepository(repository) - } - /** aborts an in-flight merge and refreshes the repository's status */ public async abortMerge(repository: Repository) { await this.appStore._abortMerge(repository) @@ -3217,7 +3133,8 @@ export class Dispatcher { repository, result, commitsToReorder.length, - tip.branch.name + tip.branch.name, + `${MultiCommitOperationKind.Reorder.toLowerCase()} commit` ) } @@ -3326,7 +3243,8 @@ export class Dispatcher { repository, result, toSquash.length + 1, - tip.branch.name + tip.branch.name, + `${MultiCommitOperationKind.Squash.toLowerCase()} commit` ) } @@ -3378,7 +3296,8 @@ export class Dispatcher { repository: Repository, result: RebaseResult, totalNumberOfCommits: number, - targetBranchName: string + ourBranch: string, + theirBranch: string ): Promise { // This will update the conflict state of the app. This is needed to start // conflict flow if squash results in conflict. @@ -3403,8 +3322,8 @@ export class Dispatcher { this.startMultiCommitOperationConflictFlow( kind, repository, - targetBranchName, - `${kind.toLowerCase()} commit` + ourBranch, + theirBranch ) break default: @@ -3508,10 +3427,8 @@ export class Dispatcher { count: number, mcos: IMultiCommitOperationState ): Banner { - const { - operationDetail: { kind }, - targetBranch, - } = mcos + const { operationDetail, targetBranch } = mcos + const { kind } = operationDetail const bannerBase: any = { count, @@ -3536,6 +3453,16 @@ export class Dispatcher { } break case MultiCommitOperationKind.Rebase: + const sourceBranch = + operationDetail.kind === MultiCommitOperationKind.Rebase + ? operationDetail.sourceBranch + : null + banner = { + type: BannerType.SuccessfulRebase, + targetBranch: targetBranch !== null ? targetBranch.name : '', + baseBranch: sourceBranch !== null ? sourceBranch.name : '', + } + break case MultiCommitOperationKind.Merge: throw new Error(`Unexpected multi commit operation kind ${kind}`) default: diff --git a/app/src/ui/dispatcher/error-handlers.ts b/app/src/ui/dispatcher/error-handlers.ts index 3cbcb8b3a1..bef4ae7265 100644 --- a/app/src/ui/dispatcher/error-handlers.ts +++ b/app/src/ui/dispatcher/error-handlers.ts @@ -417,7 +417,7 @@ export async function rebaseConflictsHandler( const { currentBranch } = gitContext - dispatcher.launchRebaseFlow(repository, currentBranch) + dispatcher.launchRebaseOperation(repository, currentBranch) return null } diff --git a/app/src/ui/multi-commit-operation/base-rebase.tsx b/app/src/ui/multi-commit-operation/base-rebase.tsx index e6e1fb5f9d..af8d39acfa 100644 --- a/app/src/ui/multi-commit-operation/base-rebase.tsx +++ b/app/src/ui/multi-commit-operation/base-rebase.tsx @@ -1,4 +1,4 @@ -import { RebaseConflictState } from '../../lib/app-state' +import { isRebaseConflictState, RebaseConflictState } from '../../lib/app-state' import { instanceOfIBaseRebaseDetails, MultiCommitOperationKind, @@ -17,11 +17,20 @@ export abstract class BaseRebase extends BaseMultiCommitOperation { state, conflictState, } = this.props - const { operationDetail, targetBranch, originalBranchTip } = state + const { operationDetail, originalBranchTip } = state + + const targetBranchName = + conflictState !== null && isRebaseConflictState(conflictState) + ? conflictState.targetBranch + : null + const baseBranchName = + conflictState !== null && isRebaseConflictState(conflictState) + ? conflictState.baseBranch || '' + : '' if ( conflictState === null || - targetBranch === null || + targetBranchName === null || originalBranchTip === null || !instanceOfIBaseRebaseDetails(operationDetail) ) { @@ -36,7 +45,7 @@ export abstract class BaseRebase extends BaseMultiCommitOperation { const rebaseConflictState: RebaseConflictState = { kind: 'rebase', currentTip, - targetBranch: targetBranch.name, + targetBranch: targetBranchName, baseBranch: undefined, originalBranchTip, baseBranchTip: currentTip, @@ -50,12 +59,15 @@ export abstract class BaseRebase extends BaseMultiCommitOperation { rebaseConflictState ) - return dispatcher.processMultiCommitOperationRebaseResult( + await dispatcher.processMultiCommitOperationRebaseResult( this.rebaseKind, repository, rebaseResult, commits.length + 1, - targetBranch.name + targetBranchName, + this.rebaseKind === MultiCommitOperationKind.Rebase + ? baseBranchName + : `${this.rebaseKind.toLowerCase()} commit` ) } diff --git a/app/src/ui/multi-commit-operation/choose-branch/base-choose-branch-dialog.tsx b/app/src/ui/multi-commit-operation/choose-branch/base-choose-branch-dialog.tsx index e826b07dbf..d31d67aa1f 100644 --- a/app/src/ui/multi-commit-operation/choose-branch/base-choose-branch-dialog.tsx +++ b/app/src/ui/multi-commit-operation/choose-branch/base-choose-branch-dialog.tsx @@ -173,15 +173,12 @@ export abstract class BaseChooseBranchDialog extends React.Component< const { selectedBranch } = this.state switch (value) { case MultiCommitOperationKind.Merge: - dispatcher.endRebaseFlow(repository) dispatcher.startMergeBranchOperation(repository, false, selectedBranch) break case MultiCommitOperationKind.Squash: - dispatcher.endRebaseFlow(repository) dispatcher.startMergeBranchOperation(repository, true, selectedBranch) break case MultiCommitOperationKind.Rebase: - dispatcher.endMultiCommitOperation(repository) dispatcher.showRebaseDialog(repository, selectedBranch) break case MultiCommitOperationKind.CherryPick: diff --git a/app/src/ui/multi-commit-operation/multi-commit-operation.tsx b/app/src/ui/multi-commit-operation/multi-commit-operation.tsx index 79bf4410a9..9d94f03908 100644 --- a/app/src/ui/multi-commit-operation/multi-commit-operation.tsx +++ b/app/src/ui/multi-commit-operation/multi-commit-operation.tsx @@ -6,6 +6,7 @@ import { IMultiCommitOperationProps } from './base-multi-commit-operation' import { Merge } from './merge' import { Reorder } from './reorder' import { CherryPick } from './cherry-pick' +import { Rebase } from './rebase' /** A component for managing the views of a multi commit operation. */ export class MultiCommitOperation extends React.Component< @@ -17,7 +18,7 @@ export class MultiCommitOperation extends React.Component< case MultiCommitOperationKind.CherryPick: return case MultiCommitOperationKind.Rebase: - return null + return case MultiCommitOperationKind.Merge: return ( { const { repository, dispatcher, state } = this.props @@ -28,4 +34,37 @@ export abstract class Rebase extends BaseRebase { { continueWithForcePush: true } ) } + + protected renderChooseBranch = (): JSX.Element | null => { + const { repository, dispatcher, state } = this.props + const { step } = state + + if (step.kind !== MultiCommitOperationStepKind.ChooseBranch) { + this.endFlowInvalidState() + return null + } + + const { + defaultBranch, + currentBranch, + allBranches, + recentBranches, + initialBranch, + } = step + + return ( + + ) + } } From 4f2bb2d0ee68db2934581abfddb62b1812ae69b3 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Mon, 16 Aug 2021 15:34:19 -0400 Subject: [PATCH 05/22] remove old rebase logic --- app/src/models/rebase-flow-step.ts | 125 -------- app/src/ui/rebase/confirm-abort-dialog.tsx | 105 ------ app/src/ui/rebase/index.ts | 2 - app/src/ui/rebase/progress-dialog.tsx | 70 ---- app/src/ui/rebase/rebase-flow.tsx | 317 ------------------- app/src/ui/rebase/warn-force-push-dialog.tsx | 106 ------- 6 files changed, 725 deletions(-) delete mode 100644 app/src/models/rebase-flow-step.ts delete mode 100644 app/src/ui/rebase/confirm-abort-dialog.tsx delete mode 100644 app/src/ui/rebase/index.ts delete mode 100644 app/src/ui/rebase/progress-dialog.tsx delete mode 100644 app/src/ui/rebase/rebase-flow.tsx delete mode 100644 app/src/ui/rebase/warn-force-push-dialog.tsx diff --git a/app/src/models/rebase-flow-step.ts b/app/src/models/rebase-flow-step.ts deleted file mode 100644 index 6af524fba5..0000000000 --- a/app/src/models/rebase-flow-step.ts +++ /dev/null @@ -1,125 +0,0 @@ -import { Branch } from './branch' -import { RebaseConflictState } from '../lib/app-state' -import { CommitOneLine } from './commit' - -/** Union type representing the possible states of the rebase flow */ -export type RebaseFlowStep = - | ChooseBranchesStep - | WarnForcePushStep - | ShowProgressStep - | ShowConflictsStep - | HideConflictsStep - | ConfirmAbortStep - | CompletedStep - -export const enum RebaseStep { - /** - * The initial state of a rebase - the user choosing the start point. - * - * This is not encountered if the user tries to 'pull with rebase' and - * encounters conflicts, because the rebase happens as part of the pull - * operation and the only remaining work for the user is to resolve any - * conflicts. - */ - ChooseBranch = 'ChooseBranch', - /** - * The initial state of a rebase - the user choosing the start point. - * - * This is not encountered if the user tries to 'pull with rebase' and - * encounters conflicts, because the rebase happens as part of the pull - * operation and the only remaining work for the user is to resolve any - * conflicts. - */ - WarnForcePush = 'WarnForcePush', - /** - * After the user chooses which branch to use as the base branch for the - * rebase, the progress view is shown indicating how the rebase work is - * progressing. - * - * This should be the default view when there are no conflicts to address. - */ - ShowProgress = 'ShowProgress', - /** - * The rebase has encountered a problem requiring the user to intervene and - * resolve conflicts. This will be shown as a list of files and the conflict - * state. - * - * Once the conflicts are resolved, the user can continue the rebase and the - * view will switch back to `ShowProgress`. - */ - ShowConflicts = 'ShowConflicts', - /** - * The user may wish to leave the conflict dialog and view the files in - * the Changes tab to get a better context. In this situation, the application - * will show a banner to indicate this context and help the user return to the - * conflicted list. - */ - HideConflicts = 'HideConflicts', - /** - * If the user wishes to abort the in-progress rebase, and the user has - * resolved conflicts at any point of the rebase, the application should ask - * the user to confirm that they wish to abort. - * - * This is to ensure the user doesn't throw away their work attempting to - * rebase the current branch. - */ - ConfirmAbort = 'ConfirmAbort', - /** - * When the rebase is completed, the dialog should be closed and a success - * banner shown to the user. - */ - Completed = 'Completed', -} - -/** Shape of data needed to choose the base branch for a rebase */ -export type ChooseBranchesStep = { - readonly kind: RebaseStep.ChooseBranch - readonly defaultBranch: Branch | null - readonly currentBranch: Branch - readonly allBranches: ReadonlyArray - readonly recentBranches: ReadonlyArray - readonly initialBranch?: Branch -} - -export type WarnForcePushStep = { - readonly kind: RebaseStep.WarnForcePush - readonly baseBranch: Branch - readonly targetBranch: Branch - readonly commits: ReadonlyArray -} - -/** Shape of data to show progress of the current rebase */ -export type ShowProgressStep = { - readonly kind: RebaseStep.ShowProgress - - /** - * An optional action to run when the component is mounted. - * - * This is provided to the component because a rebase can be very fast, and we - * want to defer the rebase action until after _something_ is shown to the - * user. - */ - readonly rebaseAction: (() => Promise) | null -} - -/** Shape of data to show conflicts that need to be resolved by the user */ -export type ShowConflictsStep = { - readonly kind: RebaseStep.ShowConflicts - readonly conflictState: RebaseConflictState -} - -/** Shape of data to track when user hides conflicts dialog */ -export type HideConflictsStep = { - readonly kind: RebaseStep.HideConflicts -} - -/** Shape of data to use when confirming user should abort rebase */ -export type ConfirmAbortStep = { - readonly kind: RebaseStep.ConfirmAbort - readonly conflictState: RebaseConflictState -} - -/** Shape of data to track when rebase has completed successfully */ -export type CompletedStep = { - readonly kind: RebaseStep.Completed -} diff --git a/app/src/ui/rebase/confirm-abort-dialog.tsx b/app/src/ui/rebase/confirm-abort-dialog.tsx deleted file mode 100644 index 04ecc0255c..0000000000 --- a/app/src/ui/rebase/confirm-abort-dialog.tsx +++ /dev/null @@ -1,105 +0,0 @@ -import * as React from 'react' - -import { Dialog, DialogContent, DialogFooter } from '../dialog' -import { ConfirmAbortStep } from '../../models/rebase-flow-step' -import { Ref } from '../lib/ref' -import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group' - -interface IConfirmAbortDialogProps { - readonly step: ConfirmAbortStep - - readonly onReturnToConflicts: (step: ConfirmAbortStep) => void - readonly onConfirmAbort: () => Promise -} - -interface IConfirmAbortDialogState { - readonly isAborting: boolean -} - -export class ConfirmAbortDialog extends React.Component< - IConfirmAbortDialogProps, - IConfirmAbortDialogState -> { - public constructor(props: IConfirmAbortDialogProps) { - super(props) - this.state = { - isAborting: false, - } - } - - private onSubmit = async () => { - this.setState({ - isAborting: true, - }) - - await this.props.onConfirmAbort() - - this.setState({ - isAborting: false, - }) - } - - /** - * Dismisses the modal and shows the rebase conflicts modal - */ - private onCancel = async () => { - await this.props.onReturnToConflicts(this.props.step) - } - - private renderTextContent(targetBranch: string, baseBranch?: string) { - let firstParagraph - - if (baseBranch !== undefined) { - firstParagraph = ( -

- {'Are you sure you want to abort rebasing '} - {baseBranch} - {' onto '} - {targetBranch}? -

- ) - } else { - firstParagraph = ( -

- {'Are you sure you want to abort rebasing '} - {targetBranch}? -

- ) - } - - return ( -
- {firstParagraph} -

- Aborting this rebase will take you back to the original branch state - and and the conflicts you have already resolved will be discarded. -

-
- ) - } - - public render() { - const { targetBranch, baseBranch } = this.props.step.conflictState - - return ( - - - {this.renderTextContent(targetBranch, baseBranch)} - - - - - - ) - } -} diff --git a/app/src/ui/rebase/index.ts b/app/src/ui/rebase/index.ts deleted file mode 100644 index 4794798092..0000000000 --- a/app/src/ui/rebase/index.ts +++ /dev/null @@ -1,2 +0,0 @@ -export { RebaseFlow } from './rebase-flow' -export { ConfirmForcePush } from './confirm-force-push' diff --git a/app/src/ui/rebase/progress-dialog.tsx b/app/src/ui/rebase/progress-dialog.tsx deleted file mode 100644 index 671f3a4a66..0000000000 --- a/app/src/ui/rebase/progress-dialog.tsx +++ /dev/null @@ -1,70 +0,0 @@ -import * as React from 'react' - -import { formatRebaseValue } from '../../lib/rebase' - -import { RichText } from '../lib/rich-text' - -import { Dialog, DialogContent } from '../dialog' -import { Octicon } from '../octicons' -import * as OcticonSymbol from '../octicons/octicons.generated' -import { IMultiCommitOperationProgress } from '../../models/progress' - -interface IRebaseProgressDialogProps { - /** Progress information about the current rebase */ - readonly progress: IMultiCommitOperationProgress - - readonly emoji: Map -} - -export class RebaseProgressDialog extends React.Component< - IRebaseProgressDialogProps -> { - private onDismissed = () => { - // this dialog is undismissable, but I need to handle the event - } - - public render() { - const { - position, - totalCommitCount, - value, - currentCommitSummary, - } = this.props.progress - - // ensure progress always starts from 1 - const count = position <= 1 ? 1 : position - - const progressValue = formatRebaseValue(value) - return ( - - -
- - -
-
- -
-
-
- Commit {count} of {totalCommitCount} -
-
- -
-
-
-
- - - ) - } -} diff --git a/app/src/ui/rebase/rebase-flow.tsx b/app/src/ui/rebase/rebase-flow.tsx deleted file mode 100644 index 11ba95bc7d..0000000000 --- a/app/src/ui/rebase/rebase-flow.tsx +++ /dev/null @@ -1,317 +0,0 @@ -import * as React from 'react' - -import { assertNever } from '../../lib/fatal-error' - -import { Repository } from '../../models/repository' -import { - RebaseStep, - RebaseFlowStep, - ConfirmAbortStep, -} from '../../models/rebase-flow-step' -import { WorkingDirectoryStatus } from '../../models/status' - -import { Dispatcher } from '../dispatcher' - -import { RebaseProgressDialog } from './progress-dialog' -import { ConfirmAbortDialog } from './confirm-abort-dialog' -import { getResolvedFiles } from '../../lib/status' -import { WarnForcePushDialog } from './warn-force-push-dialog' -import { ConflictsDialog } from '../multi-commit-operation/dialog/conflicts-dialog' -import { IMultiCommitOperationProgress } from '../../models/progress' -import { RebaseChooseBranchDialog } from '../multi-commit-operation/choose-branch/rebase-choose-branch-dialog' -import { MultiCommitOperationKind } from '../../models/multi-commit-operation' - -interface IRebaseFlowProps { - readonly repository: Repository - readonly dispatcher: Dispatcher - readonly emoji: Map - - /** The current state of the working directory */ - readonly workingDirectory: WorkingDirectoryStatus - - /** - * The current step in the rebase flow, containing application-specific - * state needed for the UI components. - */ - readonly step: RebaseFlowStep - - /** Git progress information about the current rebase */ - readonly progress: IMultiCommitOperationProgress | null - - /** - * Track whether the user has done work to resolve conflicts as part of this - * rebase, as the component should confirm with the user that they wish to - * abort the rebase and lose that work. - */ - readonly userHasResolvedConflicts: boolean - - readonly askForConfirmationOnForcePush: boolean - - /** - * Callback to hide the rebase flow and show a banner about the current state - * of conflicts, because this component will be unmounted by the runtime. - */ - readonly onShowRebaseConflictsBanner: ( - repository: Repository, - targetBranch: string - ) => void - - /** - * Callback to fire to signal to the application that the rebase flow has - * either ended in success or has been aborted and the flow can be closed. - */ - readonly onFlowEnded: (repository: Repository) => void - - /** - * Callbacks for the conflict selection components to let the user jump out - * to their preferred editor. - */ - readonly openFileInExternalEditor: (path: string) => void - readonly resolvedExternalEditor: string | null - readonly openRepositoryInShell: (repository: Repository) => void - readonly onDismissed: () => void -} - -/** A component for initiating and performing a rebase of the current branch. */ -export class RebaseFlow extends React.Component { - private moveToShowConflictedFileState = (step: ConfirmAbortStep) => { - const { conflictState } = step - this.props.dispatcher.setRebaseFlowStep(this.props.repository, { - kind: RebaseStep.ShowConflicts, - conflictState, - }) - } - - private onContinueRebase = async () => { - const { dispatcher, repository, workingDirectory, step } = this.props - if (step.kind !== RebaseStep.ShowConflicts) { - // This shouldn't happen, but needed the type checking. - log.error( - '[Rebase] Invoked continue of rebase without being in a conflict step.' - ) - this.onFlowEnded() - return - } - const { conflictState } = step - - const continueRebaseAction = async () => { - const rebaseResult = await dispatcher.continueRebase( - MultiCommitOperationKind.Rebase, - repository, - workingDirectory, - conflictState - ) - return dispatcher.processContinueRebaseResult( - rebaseResult, - conflictState, - repository - ) - } - - return dispatcher.setRebaseFlowStep(repository, { - kind: RebaseStep.ShowProgress, - rebaseAction: continueRebaseAction, - }) - } - - private onConflictsDialogDismissed = () => { - const { dispatcher, repository, step } = this.props - if (step.kind !== RebaseStep.ShowConflicts) { - // This shouldn't happen, but needed the type checking. - log.error( - '[Rebase] Cannot show rebase conflict banner without being in a conflict step.' - ) - this.onFlowEnded() - return - } - - dispatcher.setRebaseFlowStep(repository, { - kind: RebaseStep.HideConflicts, - }) - - const { targetBranch } = step.conflictState - - this.props.onShowRebaseConflictsBanner(repository, targetBranch) - } - - private onConfirmAbortRebase = async () => { - const { workingDirectory, userHasResolvedConflicts, step } = this.props - if (step.kind !== RebaseStep.ShowConflicts) { - // This shouldn't happen, but needed the type checking. - log.error( - '[Rebase] Invoked abort of rebase without being in a conflict step.' - ) - this.onFlowEnded() - return - } - - const { conflictState } = step - const { manualResolutions } = conflictState - - const resolvedConflicts = getResolvedFiles( - workingDirectory, - manualResolutions - ) - - if (userHasResolvedConflicts || resolvedConflicts.length > 0) { - // a previous commit was resolved by the user - this.props.dispatcher.setRebaseFlowStep(this.props.repository, { - kind: RebaseStep.ConfirmAbort, - conflictState, - }) - return - } - - return this.onAbortRebase() - } - - private onAbortRebase = async () => { - await this.props.dispatcher.abortRebase(this.props.repository) - this.onFlowEnded() - } - - private onFlowEnded = () => { - this.props.onDismissed() - this.props.onFlowEnded(this.props.repository) - } - - private setConflictsHaveBeenResolved = () => { - this.props.dispatcher.setConflictsResolved(this.props.repository) - } - - private renderConflictsHeaderTitle( - targetBranch: string, - baseBranch?: string - ) { - const baseBranchOutput = ( - <> - {' on '} - {baseBranch} - - ) - - return ( - - {`Resolve conflicts before rebasing `} - {targetBranch} - {baseBranch !== undefined && baseBranchOutput} - - ) - } - - public render() { - const { step } = this.props - - switch (step.kind) { - case RebaseStep.ChooseBranch: { - const { repository, dispatcher } = this.props - const { - allBranches, - defaultBranch, - currentBranch, - recentBranches, - initialBranch, - } = step - return ( - - ) - } - case RebaseStep.ShowProgress: - const { progress, emoji } = this.props - - if (progress === null) { - log.error( - '[RebaseFlow] progress is null despite trying to show the progress view. Skipping rendering...' - ) - return null - } - - return - case RebaseStep.ShowConflicts: { - const { - repository, - resolvedExternalEditor, - openFileInExternalEditor, - openRepositoryInShell, - dispatcher, - workingDirectory, - userHasResolvedConflicts, - } = this.props - - const { conflictState } = step - const { manualResolutions, targetBranch, baseBranch } = conflictState - - const submit = __DARWIN__ ? 'Continue Rebase' : 'Continue rebase' - const abort = __DARWIN__ ? 'Abort Rebase' : 'Abort rebase' - - return ( - - ) - } - - case RebaseStep.ConfirmAbort: - return ( - - ) - case RebaseStep.WarnForcePush: - const { - repository, - dispatcher, - askForConfirmationOnForcePush, - } = this.props - - return ( - - ) - - case RebaseStep.HideConflicts: - case RebaseStep.Completed: - // there is no UI to display at this point in the flow - return null - default: - return assertNever(step, 'Unknown rebase step found') - } - } -} diff --git a/app/src/ui/rebase/warn-force-push-dialog.tsx b/app/src/ui/rebase/warn-force-push-dialog.tsx deleted file mode 100644 index 09d990dc5d..0000000000 --- a/app/src/ui/rebase/warn-force-push-dialog.tsx +++ /dev/null @@ -1,106 +0,0 @@ -import * as React from 'react' - -import { Repository } from '../../models/repository' -import { WarnForcePushStep } from '../../models/rebase-flow-step' -import { Checkbox, CheckboxValue } from '../lib/checkbox' -import { Dispatcher } from '../dispatcher' -import { DialogFooter, DialogContent, Dialog } from '../dialog' -import { Ref } from '../lib/ref' -import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group' - -interface IWarnForcePushProps { - readonly dispatcher: Dispatcher - readonly repository: Repository - readonly step: WarnForcePushStep - readonly askForConfirmationOnForcePush: boolean - readonly onDismissed: () => void -} - -interface IWarnForcePushState { - readonly askForConfirmationOnForcePush: boolean -} - -export class WarnForcePushDialog extends React.Component< - IWarnForcePushProps, - IWarnForcePushState -> { - public constructor(props: IWarnForcePushProps) { - super(props) - - this.state = { - askForConfirmationOnForcePush: props.askForConfirmationOnForcePush, - } - } - - public render() { - const { baseBranch, targetBranch } = this.props.step - - const title = __DARWIN__ - ? 'Rebase Will Require Force Push' - : 'Rebase will require force push' - - return ( - - -

- Are you sure you want to rebase {targetBranch.name} onto{' '} - {baseBranch.name}? -

-

- At the end of the rebase flow, GitHub Desktop will enable you to - force push the branch to update the upstream branch. Force pushing - will alter the history on the remote and potentially cause problems - for others collaborating on this branch. -

-
- -
- - - - - - ) - } - - private onAskForConfirmationOnForcePushChanged = ( - event: React.FormEvent - ) => { - const value = !event.currentTarget.checked - - this.setState({ askForConfirmationOnForcePush: value }) - } - - private onBeginRebase = async () => { - this.props.dispatcher.setConfirmForcePushSetting( - this.state.askForConfirmationOnForcePush - ) - - const { baseBranch, targetBranch, commits } = this.props.step - - await this.props.dispatcher.startRebase( - this.props.repository, - baseBranch, - targetBranch, - commits, - { continueWithForcePush: true } - ) - } -} From d81e0a6a7b8bb61983c3293b602f19f99007fc1f Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 17 Aug 2021 08:45:34 -0400 Subject: [PATCH 06/22] Move initialization to shared method so dialog dropdown works --- app/src/ui/app.tsx | 26 -------------------------- app/src/ui/dispatcher/dispatcher.ts | 24 ++++++++++++++++++++++++ 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/app/src/ui/app.tsx b/app/src/ui/app.tsx index d76baf6f29..d01c819dec 100644 --- a/app/src/ui/app.tsx +++ b/app/src/ui/app.tsx @@ -1112,32 +1112,6 @@ export class App extends React.Component { return } - const repositoryState = this.props.repositoryStateManager.get(repository) - - const { tip } = repositoryState.branchesState - let currentBranch: Branch | null = null - - if (tip.kind === TipState.Valid) { - currentBranch = tip.branch - } else { - throw new Error( - 'Tip is not in a valid state, which is required to start the rebase flow' - ) - } - - this.props.dispatcher.initializeMultiCommitOperation( - repository, - { - kind: MultiCommitOperationKind.Rebase, - sourceBranch: null, - commits: [], - currentTip: tip.branch.tip.sha, - }, - currentBranch, - [], - currentBranch.tip.sha - ) - this.props.dispatcher.showRebaseDialog(repository) } diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index 4983b70a95..16b09a1fde 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -442,6 +442,30 @@ export class Dispatcher { initialBranch ) + const { tip } = repositoryState.branchesState + let currentBranch: Branch | null = null + + if (tip.kind === TipState.Valid) { + currentBranch = tip.branch + } else { + throw new Error( + 'Tip is not in a valid state, which is required to start the rebase flow' + ) + } + + this.initializeMultiCommitOperation( + repository, + { + kind: MultiCommitOperationKind.Rebase, + sourceBranch: null, + commits: [], + currentTip: tip.branch.tip.sha, + }, + currentBranch, + [], + currentBranch.tip.sha + ) + this.setMultiCommitOperationStep(repository, initialStep) this.showPopup({ From 23c2249db5a0000f47014bdbbb159c15a155ac17 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 17 Aug 2021 08:54:11 -0400 Subject: [PATCH 07/22] Wait for multiCommitOperation to be set --- app/src/lib/stores/app-store.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 0b3d3c4c68..035c071386 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -2003,7 +2003,10 @@ export class AppStore extends TypedBaseStore { })) this.updateMultiCommitOperationConflictsIfFound(repository) - this.initializeMultiCommitOperationIfConflictsFound(repository, status) + await this.initializeMultiCommitOperationIfConflictsFound( + repository, + status + ) if (this.selectedRepository === repository) { this._triggerConflictsFlow(repository, status) From dafc2053ae3e7199efb8a4d510b4d3a0aa6ebe31 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 19 Aug 2021 11:47:41 -0400 Subject: [PATCH 08/22] Cleanup per pr comments Co-Authored-By: Sergio Padrino <1083228+sergiou87@users.noreply.github.com> --- app/src/lib/multi-commit-operation.ts | 8 +++--- app/src/models/multi-commit-operation.ts | 7 ++++- .../ui/multi-commit-operation/base-rebase.tsx | 27 ++++++++----------- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/app/src/lib/multi-commit-operation.ts b/app/src/lib/multi-commit-operation.ts index 406d3f0124..93dbf6a09c 100644 --- a/app/src/lib/multi-commit-operation.ts +++ b/app/src/lib/multi-commit-operation.ts @@ -15,7 +15,7 @@ import { IMultiCommitOperationState, IRepositoryState } from './app-state' export function getMultiCommitOperationChooseBranchStep( state: IRepositoryState, initialBranch?: Branch | null -) { +): ChooseBranchStep { const { defaultBranch, allBranches, @@ -28,11 +28,11 @@ export function getMultiCommitOperationChooseBranchStep( currentBranch = tip.branch } else { throw new Error( - 'Tip is not in a valid state, which is required to start the rebase flow' + 'Tip is not in a valid state, which is required to start the multi commit operation' ) } - const initialState: ChooseBranchStep = { + return { kind: MultiCommitOperationStepKind.ChooseBranch, defaultBranch, currentBranch, @@ -40,8 +40,6 @@ export function getMultiCommitOperationChooseBranchStep( recentBranches, initialBranch: initialBranch !== null ? initialBranch : undefined, } - - return initialState } export function isConflictsFlow( diff --git a/app/src/models/multi-commit-operation.ts b/app/src/models/multi-commit-operation.ts index 4d99b530e7..d1c565fa4f 100644 --- a/app/src/models/multi-commit-operation.ts +++ b/app/src/models/multi-commit-operation.ts @@ -228,7 +228,12 @@ export type MultiCommitOperationDetail = export function instanceOfIBaseRebaseDetails( object: any ): object is IBaseInteractiveRebaseDetails { - return 'commits' in object && 'currentTip' in object + const objectWithRequiredFields: IBaseInteractiveRebaseDetails = { + commits: [], + currentTip: '', + } + + return Object.keys(objectWithRequiredFields).every(key => key in object) } export const conflictSteps = [ diff --git a/app/src/ui/multi-commit-operation/base-rebase.tsx b/app/src/ui/multi-commit-operation/base-rebase.tsx index af8d39acfa..f847be5cf8 100644 --- a/app/src/ui/multi-commit-operation/base-rebase.tsx +++ b/app/src/ui/multi-commit-operation/base-rebase.tsx @@ -19,25 +19,17 @@ export abstract class BaseRebase extends BaseMultiCommitOperation { } = this.props const { operationDetail, originalBranchTip } = state - const targetBranchName = - conflictState !== null && isRebaseConflictState(conflictState) - ? conflictState.targetBranch - : null - const baseBranchName = - conflictState !== null && isRebaseConflictState(conflictState) - ? conflictState.baseBranch || '' - : '' - if ( conflictState === null || - targetBranchName === null || originalBranchTip === null || + !isRebaseConflictState(conflictState) || !instanceOfIBaseRebaseDetails(operationDetail) ) { this.endFlowInvalidState() return } + const { targetBranch, baseBranch } = conflictState const { commits, currentTip } = operationDetail await dispatcher.switchMultiCommitOperationToShowProgress(repository) @@ -45,8 +37,8 @@ export abstract class BaseRebase extends BaseMultiCommitOperation { const rebaseConflictState: RebaseConflictState = { kind: 'rebase', currentTip, - targetBranch: targetBranchName, - baseBranch: undefined, + targetBranch, + baseBranch, originalBranchTip, baseBranchTip: currentTip, manualResolutions: conflictState.manualResolutions, @@ -59,15 +51,18 @@ export abstract class BaseRebase extends BaseMultiCommitOperation { rebaseConflictState ) + const thierBranch = + this.rebaseKind === MultiCommitOperationKind.Rebase + ? baseBranch || '' + : `${this.rebaseKind.toLowerCase()} commit` + await dispatcher.processMultiCommitOperationRebaseResult( this.rebaseKind, repository, rebaseResult, commits.length + 1, - targetBranchName, - this.rebaseKind === MultiCommitOperationKind.Rebase - ? baseBranchName - : `${this.rebaseKind.toLowerCase()} commit` + targetBranch, + thierBranch ) } From 6046c402a3f0dcb8508b2e3f40ff9439c40b63eb Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Thu, 19 Aug 2021 12:20:31 -0400 Subject: [PATCH 09/22] Need to know what kind they are --- app/src/ui/multi-commit-operation/reorder.tsx | 1 + app/src/ui/multi-commit-operation/squash.tsx | 1 + 2 files changed, 2 insertions(+) diff --git a/app/src/ui/multi-commit-operation/reorder.tsx b/app/src/ui/multi-commit-operation/reorder.tsx index 46a1a860ba..8f9512fcec 100644 --- a/app/src/ui/multi-commit-operation/reorder.tsx +++ b/app/src/ui/multi-commit-operation/reorder.tsx @@ -3,6 +3,7 @@ import { BaseRebase } from './base-rebase' export abstract class Reorder extends BaseRebase { protected conflictDialogOperationPrefix = 'reordering commits on' + protected rebaseKind = MultiCommitOperationKind.Reorder protected onBeginOperation = () => { const { repository, dispatcher, state } = this.props diff --git a/app/src/ui/multi-commit-operation/squash.tsx b/app/src/ui/multi-commit-operation/squash.tsx index 2db58f2ae3..5de8fc9274 100644 --- a/app/src/ui/multi-commit-operation/squash.tsx +++ b/app/src/ui/multi-commit-operation/squash.tsx @@ -3,6 +3,7 @@ import { BaseRebase } from './base-rebase' export abstract class Squash extends BaseRebase { protected conflictDialogOperationPrefix = 'squashing commits on' + protected rebaseKind = MultiCommitOperationKind.Squash protected onBeginOperation = () => { const { repository, dispatcher, state } = this.props From 08058cc1de5114c7ca86497f15ef3f559fd5b841 Mon Sep 17 00:00:00 2001 From: Steven Yeh Date: Mon, 23 Aug 2021 21:05:09 -0500 Subject: [PATCH 10/22] Fix order --- docs/technical/syntax-highlighting.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/technical/syntax-highlighting.md b/docs/technical/syntax-highlighting.md index bc3d858206..fb49b184e9 100644 --- a/docs/technical/syntax-highlighting.md +++ b/docs/technical/syntax-highlighting.md @@ -8,7 +8,7 @@ We introduced syntax highlighted diffs in [#3101](https://github.com/desktop/des We currently support syntax highlighting for the following languages and file types. -JavaScript, JSON, TypeScript, Coffeescript, HTML, Asp, JavaServer Pages, CSS, SCSS, LESS, VUE, Markdown, Toml, Yaml, XML, Diff, Objective-C, Scala, C#, Java, C, C++, Kotlin, Ocaml, F#, Swift, sh/bash, SQL, CYPHER, Go, Perl, PHP, Python, Ruby, Clojure, Rust, Elixir, Haxe, R, PowerShell, Visual Basic, Fortran, Lua, Crystal, Julia, sTex, SPARQL, Stylus, Soy, Smalltalk, Slim, Sieve, Scheme, ReStructuredText, RPM, Q, Puppet, Pug, Protobuf, Properties, Apache Pig, ASCII Armor (PGP), Oz, Pascal and Docker. +JavaScript, JSON, TypeScript, Coffeescript, HTML, Asp, JavaServer Pages, CSS, SCSS, LESS, VUE, Markdown, Yaml, XML, Diff, Objective-C, Scala, C#, Java, C, C++, Kotlin, Ocaml, F#, Swift, sh/bash, SQL, CYPHER, Go, Perl, PHP, Python, Ruby, Clojure, Rust, Elixir, Haxe, R, PowerShell, Visual Basic, Fortran, Lua, Crystal, Julia, sTex, SPARQL, Stylus, Soy, Smalltalk, Slim, Sieve, Scheme, ReStructuredText, RPM, Q, Puppet, Pug, Protobuf, Properties, Apache Pig, ASCII Armor (PGP), Oz, Pascal, Toml and Docker. This list was never meant to be exhaustive, we expect to add more languages going forward but this seemed like a good first step. From 94452dade450ea3b60252e93841f4e1b06efc523 Mon Sep 17 00:00:00 2001 From: Steven Yeh Date: Mon, 23 Aug 2021 21:13:10 -0500 Subject: [PATCH 11/22] Add Dart --- app/src/highlighter/index.ts | 6 ++++++ docs/technical/syntax-highlighting.md | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/src/highlighter/index.ts b/app/src/highlighter/index.ts index 2529d3d045..8b8be402ca 100644 --- a/app/src/highlighter/index.ts +++ b/app/src/highlighter/index.ts @@ -403,6 +403,12 @@ const extensionModes: ReadonlyArray = [ '.toml': 'text/x-toml', }, }, + { + install: () => import('codemirror/mode/dart/dart'), + mappings: { + '.dart': 'application/dart', + }, + }, ] /** diff --git a/docs/technical/syntax-highlighting.md b/docs/technical/syntax-highlighting.md index fb49b184e9..aaaae9df2f 100644 --- a/docs/technical/syntax-highlighting.md +++ b/docs/technical/syntax-highlighting.md @@ -8,7 +8,7 @@ We introduced syntax highlighted diffs in [#3101](https://github.com/desktop/des We currently support syntax highlighting for the following languages and file types. -JavaScript, JSON, TypeScript, Coffeescript, HTML, Asp, JavaServer Pages, CSS, SCSS, LESS, VUE, Markdown, Yaml, XML, Diff, Objective-C, Scala, C#, Java, C, C++, Kotlin, Ocaml, F#, Swift, sh/bash, SQL, CYPHER, Go, Perl, PHP, Python, Ruby, Clojure, Rust, Elixir, Haxe, R, PowerShell, Visual Basic, Fortran, Lua, Crystal, Julia, sTex, SPARQL, Stylus, Soy, Smalltalk, Slim, Sieve, Scheme, ReStructuredText, RPM, Q, Puppet, Pug, Protobuf, Properties, Apache Pig, ASCII Armor (PGP), Oz, Pascal, Toml and Docker. +JavaScript, JSON, TypeScript, Coffeescript, HTML, Asp, JavaServer Pages, CSS, SCSS, LESS, VUE, Markdown, Yaml, XML, Diff, Objective-C, Scala, C#, Java, C, C++, Kotlin, Ocaml, F#, Swift, sh/bash, SQL, CYPHER, Go, Perl, PHP, Python, Ruby, Clojure, Rust, Elixir, Haxe, R, PowerShell, Visual Basic, Fortran, Lua, Crystal, Julia, sTex, SPARQL, Stylus, Soy, Smalltalk, Slim, Sieve, Scheme, ReStructuredText, RPM, Q, Puppet, Pug, Protobuf, Properties, Apache Pig, ASCII Armor (PGP), Oz, Pascal, Toml, Dart and Docker. This list was never meant to be exhaustive, we expect to add more languages going forward but this seemed like a good first step. From 7595a19525f2e48253a35351a8bca2c77b460c04 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Tue, 24 Aug 2021 11:09:27 -0400 Subject: [PATCH 12/22] Fix (or vastly improve) performance issue from dynamic diff gutter line width application --- app/src/ui/diff/text-diff.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/src/ui/diff/text-diff.tsx b/app/src/ui/diff/text-diff.tsx index 47bdd73676..0e10447bb8 100644 --- a/app/src/ui/diff/text-diff.tsx +++ b/app/src/ui/diff/text-diff.tsx @@ -1016,10 +1016,15 @@ export class TextDiff extends React.Component { const gutterParentElement = cm.getGutterElement() const gutterElement = gutterParentElement.getElementsByClassName( - 'diff-gutter' + diffGutterName )[0] - gutterElement.setAttribute('style', `width: ${diffSize * 2}px;`) - cm.refresh() + + const newStyle = `width: ${diffSize * 2}px;` + const currentStyle = gutterElement.getAttribute('style') + if (newStyle !== currentStyle) { + gutterElement.setAttribute('style', newStyle) + cm.refresh() + } } /** From 36ed73af38d587325685d16b41c855a60a90d7df Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 25 Aug 2021 08:53:50 -0400 Subject: [PATCH 13/22] Fix Rebase Succes Banner --- app/src/lib/stores/app-store.ts | 13 +++++++++++++ app/src/ui/dispatcher/dispatcher.ts | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 035c071386..954f39c09d 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -154,6 +154,7 @@ import { reset, getBranchAheadBehind, getRebaseInternalState, + getCommit, } from '../git' import { installGlobalLFSFilters, @@ -2077,6 +2078,18 @@ export class AppStore extends TypedBaseStore { commits, currentTip: rebaseState.baseBranchTip, } + + const commit = await getCommit(repository, rebaseState.originalBranchTip) + + if (commit !== null) { + targetBranch = new Branch( + rebaseState.targetBranch, + null, + commit, + BranchType.Local, + `refs/heads/${rebaseState.targetBranch}` + ) + } } else if (isCherryPickConflictState(conflictState)) { const snapshot = await getCherryPickSnapshot(repository) if (snapshot === null) { diff --git a/app/src/ui/dispatcher/dispatcher.ts b/app/src/ui/dispatcher/dispatcher.ts index 412bbe3cc3..5348b11854 100644 --- a/app/src/ui/dispatcher/dispatcher.ts +++ b/app/src/ui/dispatcher/dispatcher.ts @@ -3484,7 +3484,7 @@ export class Dispatcher { banner = { type: BannerType.SuccessfulRebase, targetBranch: targetBranch !== null ? targetBranch.name : '', - baseBranch: sourceBranch !== null ? sourceBranch.name : '', + baseBranch: sourceBranch !== null ? sourceBranch.name : undefined, } break case MultiCommitOperationKind.Merge: From 0dbd6d0e32501870a7527988d5446a7e8043221e Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 25 Aug 2021 12:13:37 -0400 Subject: [PATCH 14/22] Silently error when no conflicts present for dismissing conflict dialog --- .../base-multi-commit-operation.tsx | 12 ++++++++---- app/src/ui/multi-commit-operation/merge.tsx | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/src/ui/multi-commit-operation/base-multi-commit-operation.tsx b/app/src/ui/multi-commit-operation/base-multi-commit-operation.tsx index 1722677cc4..8e04f114fd 100644 --- a/app/src/ui/multi-commit-operation/base-multi-commit-operation.tsx +++ b/app/src/ui/multi-commit-operation/base-multi-commit-operation.tsx @@ -73,11 +73,15 @@ export abstract class BaseMultiCommitOperation extends React.Component< * needed for typing purposes. Thus it should never happen, so throw error if * does. */ - protected endFlowInvalidState(): void { + protected endFlowInvalidState(isSilent: boolean = false): void { const { step, operationDetail } = this.props.state - throw new Error( - `[${operationDetail.kind}] - Invalid state - ${operationDetail.kind} ended during ${step.kind}.` - ) + const errorMessage = `[${operationDetail.kind}] - Invalid state - ${operationDetail.kind} ended during ${step.kind}.` + if (isSilent) { + this.onFlowEnded() + log.error(errorMessage) + return + } + throw new Error(errorMessage) } protected onInvokeConflictsDialogDismissed = (operationPrefix: string) => { diff --git a/app/src/ui/multi-commit-operation/merge.tsx b/app/src/ui/multi-commit-operation/merge.tsx index 763907a311..719dec7d65 100644 --- a/app/src/ui/multi-commit-operation/merge.tsx +++ b/app/src/ui/multi-commit-operation/merge.tsx @@ -72,7 +72,7 @@ export abstract class Merge extends BaseMultiCommitOperation { protected onConflictsDialogDismissed = () => { const { dispatcher, workingDirectory, conflictState } = this.props if (conflictState === null || !isMergeConflictState(conflictState)) { - this.endFlowInvalidState() + this.endFlowInvalidState(true) return } dispatcher.recordMergeConflictsDialogDismissal() From 383143fdcb2ea52ce12d0cfa2a7f6407065cee74 Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Wed, 25 Aug 2021 12:23:55 -0400 Subject: [PATCH 15/22] Release 2.9.3-beta1 --- app/package.json | 2 +- changelog.json | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/package.json b/app/package.json index f9207a0e7f..89bd5f062c 100644 --- a/app/package.json +++ b/app/package.json @@ -3,7 +3,7 @@ "productName": "GitHub Desktop", "bundleID": "com.github.GitHubClient", "companyName": "GitHub, Inc.", - "version": "2.9.2", + "version": "2.9.3-beta1", "main": "./main.js", "repository": { "type": "git", diff --git a/changelog.json b/changelog.json index 8f4f27ec28..3d5611c745 100644 --- a/changelog.json +++ b/changelog.json @@ -1,5 +1,9 @@ { "releases": { + "2.9.3-beta1": [ + "[Added] Adds syntax highlighting for dart - #12827. Thanks @say25!", + "[Fixed] Fix scrolling performance issue for large diffs." + ], "2.9.2": ["[Fixed] Fix scrolling performance issue for large diffs."], "2.9.1": [ "[Added] Add Fluent Terminal shell support - #12305. Thanks @Idered!", From 2f6fc3d9dd9b6a0e617be468c34566da6071abb0 Mon Sep 17 00:00:00 2001 From: Billy Griffin <5091167+billygriffin@users.noreply.github.com> Date: Wed, 25 Aug 2021 11:19:00 -0600 Subject: [PATCH 16/22] Update changelog.json --- changelog.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.json b/changelog.json index 3d5611c745..392c631a54 100644 --- a/changelog.json +++ b/changelog.json @@ -1,7 +1,7 @@ { "releases": { "2.9.3-beta1": [ - "[Added] Adds syntax highlighting for dart - #12827. Thanks @say25!", + "[Added] Add syntax highlighting for dart - #12827. Thanks @say25!", "[Fixed] Fix scrolling performance issue for large diffs." ], "2.9.2": ["[Fixed] Fix scrolling performance issue for large diffs."], From 0b12391a2ee11542eefed89cf8d45ee6568c95fc Mon Sep 17 00:00:00 2001 From: Tsvetilian Yankov Date: Fri, 27 Aug 2021 12:27:39 +0300 Subject: [PATCH 17/22] Add minor version support for JetBrains IDEs --- app/src/lib/editors/win32.ts | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/app/src/lib/editors/win32.ts b/app/src/lib/editors/win32.ts index fae386abeb..53b01d6f48 100644 --- a/app/src/lib/editors/win32.ts +++ b/app/src/lib/editors/win32.ts @@ -79,21 +79,36 @@ const Wow64LocalMachineUninstallKey = (subKey: string) => registryKey(HKEY.HKEY_LOCAL_MACHINE, wow64UninstallSubKey, subKey) // This function generates registry keys for a given JetBrains product for the -// last 2 years, assuming JetBrains makes no more than 5 releases per year. +// last 2 years, assuming JetBrains makes no more than 5 major releases and +// no more than 5 minor releases per year const registryKeysForJetBrainsIDE = ( product: string ): ReadonlyArray => { - const maxReleasesPerYear = 5 + const maxMajorReleasesPerYear = 5 + const maxMinorReleasesPerYear = 5 const lastYear = new Date().getFullYear() const firstYear = lastYear - 2 const result = new Array() for (let year = firstYear; year <= lastYear; year++) { - for (let release = 1; release <= maxReleasesPerYear; release++) { - const key = `${product} ${year}.${release}` - result.push(Wow64LocalMachineUninstallKey(key)) - result.push(CurrentUserUninstallKey(key)) + for ( + let majorRelease = 1; + majorRelease <= maxMajorReleasesPerYear; + majorRelease++ + ) { + for ( + let minorRelease = 0; + minorRelease <= maxMinorReleasesPerYear; + minorRelease++ + ) { + let key = `${product} ${year}.${majorRelease}` + if (minorRelease > 0) { + key = `${key}.${minorRelease}` + } + result.push(Wow64LocalMachineUninstallKey(key)) + result.push(CurrentUserUninstallKey(key)) + } } } From 491e4f0fb65e3d6384742fa18595c0c27671358d Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Mon, 30 Aug 2021 10:36:14 +0200 Subject: [PATCH 18/22] Fix Notepad++ and RStudio integration on Windows --- app/src/lib/editors/win32.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/lib/editors/win32.ts b/app/src/lib/editors/win32.ts index fae386abeb..450df9a2e9 100644 --- a/app/src/lib/editors/win32.ts +++ b/app/src/lib/editors/win32.ts @@ -298,7 +298,7 @@ const editors: IWindowsExternalEditor[] = [ // 32-bit version of Notepad++ Wow64LocalMachineUninstallKey('Notepad++'), ], - executableShimPaths: [], + executableShimPaths: [[]], installLocationRegistryKey: 'DisplayIcon', expectedInstallationChecker: (displayName, publisher) => displayName.startsWith('Notepad++') && publisher === 'Notepad++ Team', @@ -314,7 +314,7 @@ const editors: IWindowsExternalEditor[] = [ { name: 'RStudio', registryKeys: [Wow64LocalMachineUninstallKey('RStudio')], - executableShimPaths: [], + executableShimPaths: [[]], installLocationRegistryKey: 'DisplayIcon', expectedInstallationChecker: (displayName, publisher) => displayName === 'RStudio' && publisher === 'RStudio', From f21fde08ee22f9308f3c269a3e2f955d482ed85b Mon Sep 17 00:00:00 2001 From: tidy-dev Date: Mon, 30 Aug 2021 09:29:27 -0400 Subject: [PATCH 19/22] Remove S3 bucket refs --- .github/workflows/ci.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4c1b4aca65..33271ded2d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -98,9 +98,6 @@ jobs: WINDOWS_CERT_PASSWORD: ${{ secrets.WINDOWS_CERT_PASSWORD }} KEY_PASSWORD: ${{ secrets.KEY_PASSWORD }} DEPLOYMENT_SECRET: ${{ secrets.DEPLOYMENT_SECRET }} - S3_KEY: ${{ secrets.S3_KEY }} - S3_SECRET: ${{ secrets.S3_SECRET }} - S3_BUCKET: github-desktop AZURE_STORAGE_ACCOUNT: ${{ secrets.AZURE_STORAGE_ACCOUNT }} AZURE_STORAGE_ACCESS_KEY: ${{ secrets.AZURE_STORAGE_ACCESS_KEY }} AZURE_BLOB_CONTAINER: ${{ secrets.AZURE_BLOB_CONTAINER }} From 5618f61da27c521edbdfa2cbac8b85e442e1054c Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Tue, 31 Aug 2021 11:14:09 +0200 Subject: [PATCH 20/22] Get rid of [Object object] strings --- app/src/lib/editors/darwin.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/src/lib/editors/darwin.ts b/app/src/lib/editors/darwin.ts index 24744981a4..76961c337f 100644 --- a/app/src/lib/editors/darwin.ts +++ b/app/src/lib/editors/darwin.ts @@ -137,9 +137,11 @@ async function findApplication( return installPath } - log.debug(`App installation for ${editor} not found at '${installPath}'`) + log.debug( + `App installation for ${editor.name} not found at '${installPath}'` + ) } catch (error) { - log.debug(`Unable to locate ${editor} installation`, error) + log.debug(`Unable to locate ${editor.name} installation`, error) } } From 51ac2c541e4a4ec108f69b54bf68cb6406269aac Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Tue, 31 Aug 2021 11:14:29 +0200 Subject: [PATCH 21/22] Don't log if the editor isn't installed on the system --- app/src/lib/editors/darwin.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/app/src/lib/editors/darwin.ts b/app/src/lib/editors/darwin.ts index 76961c337f..3e5af72824 100644 --- a/app/src/lib/editors/darwin.ts +++ b/app/src/lib/editors/darwin.ts @@ -131,9 +131,20 @@ async function findApplication( ): Promise { for (const identifier of editor.bundleIdentifiers) { try { - const installPath = await appPath(identifier) - const exists = await pathExists(installPath) - if (exists) { + // app-path not finding the app isn't an error, it just means the + // bundle isn't registered on the machine. + // https://github.com/sindresorhus/app-path/blob/0e776d4e132676976b4a64e09b5e5a4c6e99fcba/index.js#L7-L13 + const installPath = await appPath(identifier).catch(e => + e.message === "Couldn't find the app" + ? Promise.resolve(null) + : Promise.reject(e) + ) + + if (installPath === null) { + return null + } + + if (await pathExists(installPath)) { return installPath } From a6e659573fae8a5e1c4ec4efaeab2bbf3f280d51 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Tue, 31 Aug 2021 16:11:57 +0200 Subject: [PATCH 22/22] Bump version and changelog to 2.9.3-beta2 --- app/package.json | 2 +- changelog.json | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/package.json b/app/package.json index 89bd5f062c..b955e79932 100644 --- a/app/package.json +++ b/app/package.json @@ -3,7 +3,7 @@ "productName": "GitHub Desktop", "bundleID": "com.github.GitHubClient", "companyName": "GitHub, Inc.", - "version": "2.9.3-beta1", + "version": "2.9.3-beta2", "main": "./main.js", "repository": { "type": "git", diff --git a/changelog.json b/changelog.json index 392c631a54..e02e5d8a8b 100644 --- a/changelog.json +++ b/changelog.json @@ -1,5 +1,9 @@ { "releases": { + "2.9.3-beta2": [ + "[Fixed] Fix Notepad++ and RStudio integration on Windows - #12841", + "[Fixed] Add minor version support for JetBrains IDEs on Windows - #12847. Thanks @tsvetilian-ty!" + ], "2.9.3-beta1": [ "[Added] Add syntax highlighting for dart - #12827. Thanks @say25!", "[Fixed] Fix scrolling performance issue for large diffs."