BaseBranch: Don't be scary when pushing

Only display the warnings and confirmation modals when actual changes can get lost (resting to local or remote). Otherwise, display less scary notifications and let the user push if needed
gitbutlerapp · Sep 13, 2024 · 72d0050 · 72d0050
1 parent 99ff335
commit 72d0050
Show file tree

Hide file tree

Showing 2 changed files with 109 additions and 37 deletions.
diff --git a/apps/desktop/src/lib/components/BaseBranch.svelte b/apps/desktop/src/lib/components/BaseBranch.svelte
@@ -1,5 +1,6 @@
 <script lang="ts">
 	import { Project } from '$lib/backend/projects';
+	import { BaseBranchService } from '$lib/baseBranch/baseBranchService';
 	import CommitAction from '$lib/commit/CommitAction.svelte';
 	import CommitCard from '$lib/commit/CommitCard.svelte';
 	import { transformAnyCommit } from '$lib/commitLines/transformers';
@@ -17,6 +18,7 @@
 	import Modal from '@gitbutler/ui/Modal.svelte';
 	import LineGroup from '@gitbutler/ui/commitLines/LineGroup.svelte';
 	import { LineManagerFactory, LineSpacer } from '@gitbutler/ui/commitLines/lineManager';
+	import { tick } from 'svelte';
 	import type { BaseBranch } from '$lib/baseBranch/baseBranch';
 
 	const resetBranchTo = {
@@ -36,7 +38,7 @@
 			tooltip:
 				'Resets the upstream branch to the local target branch. Will lose all the remote changes not in the local branch.',
 			color: 'pop',
-			action: updateBaseBranch
+			action: async () => await pushBaseBranch()
 		}
 	} as const;
 
@@ -48,6 +50,7 @@
 
 	const { base }: Props = $props();
 
+	const baseBranchService = getContext(BaseBranchService);
 	const branchController = getContext(BranchController);
 	const modeService = getContext(ModeService);
 	const gitHost = getGitHost();
@@ -59,20 +62,25 @@
 		projectMergeUpstreamWarningDismissed(branchController.projectId)
 	);
 
+	let baseBranchIsUpdating = $state<boolean>(false);
 	let updateTargetModal = $state<Modal>();
 	let confirmResetModal = $state<Modal>();
 	let resetActionType = $state<ResetActionType | undefined>(undefined);
 	let mergeUpstreamWarningDismissedCheckbox = $state<boolean>(false);
 	let updateBaseButton = $state<UpdateBaseButton | undefined>();
 
-	const { satisfied: commitsAhead, rest: localAndRemoteCommits } = $derived(
-		groupByCondition(base.recentCommits, (c) => base.divergedAhead.includes(c.id))
-	);
-
 	const multiple = $derived(
 		base ? base.upstreamCommits.length > 1 || base.upstreamCommits.length === 0 : false
 	);
 
+	const onlyLocalAhead = $derived(
+		base.diverged && base.divergedBehind.length === 0 && base.divergedAhead.length > 0
+	);
+
+	const { satisfied: commitsAhead, rest: localAndRemoteCommits } = $derived(
+		groupByCondition(base.recentCommits, (c) => base.divergedAhead.includes(c.id))
+	);
+
 	const mappedRemoteCommits = $derived(
 		base.upstreamCommits.length > 0
 			? [...base.upstreamCommits.map(transformAnyCommit), { id: LineSpacer.Remote }]
@@ -105,35 +113,58 @@
 	);
 
 	async function updateBaseBranch() {
+		baseBranchIsUpdating = true;
 		const infoText = await branchController.updateBaseBranch();
 		if (infoText) {
 			showInfo('Stashed conflicting branches', infoText);
 		}
+		await tick();
+		baseBranchIsUpdating = false;
+	}
+
+	async function pushBaseBranch() {
+		baseBranchIsUpdating = true;
+		await baseBranchService.push(!onlyLocalAhead);
+		await tick();
+		baseBranchIsUpdating = false;
 	}
 
-	function confirmResetBranch(type: ResetActionType) {
+	async function confirmResetBranch(type: ResetActionType) {
 		resetActionType = type;
+		await tick();
 		confirmResetModal?.show();
 	}
 </script>
 
 {#if base.diverged}
 	<div class="message-wrapper">
-		<InfoMessage style="warning" filled outlined={false}>
-			<svelte:fragment slot="content">
-				Your target branch has diverged from upstream.
-				<br />
-				Target branch is
-				<b>
-					{`ahead by ${base.divergedAhead.length}`}
-				</b>
-				commits and
-				<b>
-					{`behind by ${base.divergedBehind.length}`}
-				</b>
-				commits
-			</svelte:fragment>
-		</InfoMessage>
+		{#if !onlyLocalAhead}
+			<InfoMessage style="warning" filled outlined={false}>
+				<svelte:fragment slot="content">
+					Your local target branch has diverged from upstream.
+					<br />
+					Target branch is
+					<b>
+						{`ahead by ${base.divergedAhead.length}`}
+					</b>
+					commits and
+					<b>
+						{`behind by ${base.divergedBehind.length}`}
+					</b>
+					commits
+				</svelte:fragment>
+			</InfoMessage>
+		{:else}
+			<InfoMessage style="neutral" filled outlined={false}>
+				<svelte:fragment slot="content">
+					Your local target branch is
+					<b>
+						{`ahead by ${base.divergedAhead.length}`}
+					</b>
+					commits
+				</svelte:fragment>
+			</InfoMessage>
+		{/if}
 	</div>
 {/if}
 
@@ -198,8 +229,9 @@
 						icon="warning"
 						kind="solid"
 						tooltip={resetBranchTo.remote.tooltip}
-						disabled={$mode?.type !== 'OpenWorkspace'}
-						onclick={() => confirmResetBranch('remote')}
+						loading={baseBranchIsUpdating}
+						disabled={$mode?.type !== 'OpenWorkspace' || baseBranchIsUpdating}
+						onclick={async () => await confirmResetBranch('remote')}
 					>
 						{resetBranchTo.remote.title}
 					</Button>
@@ -232,16 +264,43 @@
 				<LineGroup lineGroup={lineManager.get(LineSpacer.Local)} topHeightPx={0} />
 			{/snippet}
 			{#snippet action()}
-				<Button
-					style={resetBranchTo.local.color}
-					icon="warning"
-					kind="solid"
-					tooltip={resetBranchTo.local.tooltip}
-					disabled={$mode?.type !== 'OpenWorkspace'}
-					onclick={() => confirmResetBranch('local')}
-				>
-					{resetBranchTo.local.title}
-				</Button>
+				<div class="local-actions-wrapper">
+					<Button
+						wide
+						style={resetBranchTo.local.color}
+						icon={onlyLocalAhead ? undefined : 'warning'}
+						kind="solid"
+						tooltip={onlyLocalAhead
+							? 'Push your local changes to upstream'
+							: resetBranchTo.local.tooltip}
+						loading={baseBranchIsUpdating}
+						disabled={$mode?.type !== 'OpenWorkspace' || baseBranchIsUpdating}
+						onclick={async () => {
+							if (onlyLocalAhead) {
+								await pushBaseBranch();
+								return;
+							}
+							await confirmResetBranch('local');
+						}}
+					>
+						{onlyLocalAhead ? 'Push' : resetBranchTo.local.title}
+					</Button>
+
+					{#if onlyLocalAhead}
+						<Button
+							wide
+							style={resetBranchTo.remote.color}
+							icon="warning"
+							kind="solid"
+							tooltip={resetBranchTo.remote.tooltip}
+							loading={baseBranchIsUpdating}
+							disabled={$mode?.type !== 'OpenWorkspace' || baseBranchIsUpdating}
+							onclick={async () => await confirmResetBranch('remote')}
+						>
+							{resetBranchTo.remote.title}
+						</Button>
+					{/if}
+				</div>
 			{/snippet}
 		</CommitAction>
 	{/if}
@@ -327,9 +386,10 @@
 				{:else}
 					You will force-push the local branch to the remote branch.
 					<br />
+					<br />
 					{base.divergedBehind.length > 1
-						? `The ${base.divergedBehind.length} changes in the remote branch will be lost.`
-						: 'The change in the remote branch will be lost.'}
+						? `The ${base.divergedBehind.length} commits in the remote branch will be lost.`
+						: 'The commit in the remote branch will be lost.'}
 				{/if}
 			</p>
 		</div>
@@ -366,6 +426,11 @@
 		opacity: 0.5;
 	}
 
+	.local-actions-wrapper {
+		display: flex;
+		width: 100%;
+		gap: 8px;
+	}
 	.modal-content {
 		display: flex;
 		flex-direction: column;

diff --git a/apps/desktop/src/lib/navigation/TargetCard.svelte b/apps/desktop/src/lib/navigation/TargetCard.svelte
@@ -19,6 +19,10 @@
 	$: base = baseBranchService.base;
 	$: selected = $page.url.href.endsWith('/base');
 	$: baseBranchDiverged = !!$base?.diverged;
+	$: baseBranchAheadOnly = baseBranchDiverged && !!$base?.divergedBehind?.length === false;
+	$: divergenceTooltip = baseBranchAheadOnly
+		? 'Your local target branch is ahead of its upstream'
+		: 'Your local target branch has diverged from its upstream';
 </script>
 
 <DomainButton
@@ -49,9 +53,12 @@
 					</Tooltip>
 				{/if}
 				{#if baseBranchDiverged}
-					<Tooltip text="Your branch has diverged from the target branch">
+					<Tooltip text={divergenceTooltip}>
 						<div>
-							<Icon name="warning" color="warning" />
+							<Icon
+								name={baseBranchAheadOnly ? 'info' : 'warning'}
+								color={baseBranchAheadOnly ? undefined : 'warning'}
+							/>
 						</div>
 					</Tooltip>
 				{/if}