#3534: Change rebase terminal-run commands into normal commands w/ proper error handling

src/commands/git/rebase.ts

@@ -1,13 +1,16 @@
 import type { Container } from '../../container';
+import type { RebaseOptions } from '../../git/gitProvider';
 import type { GitBranch } from '../../git/models/branch';
 import type { GitLog } from '../../git/models/log';
 import type { GitReference } from '../../git/models/reference';
 import { createRevisionRange, getReferenceLabel, isRevisionReference } from '../../git/models/reference';
 import type { Repository } from '../../git/models/repository';
+import { showGenericErrorMessage } from '../../messages';
 import type { DirectiveQuickPickItem } from '../../quickpicks/items/directive';
 import { createDirectiveQuickPickItem, Directive } from '../../quickpicks/items/directive';
 import type { FlagsQuickPickItem } from '../../quickpicks/items/flags';
 import { createFlagsQuickPickItem } from '../../quickpicks/items/flags';
+import { Logger } from '../../system/logger';
 import { pluralize } from '../../system/string';
 import { getEditorCommand } from '../../system/vscode/utils';
 import type { ViewsWithRepositoryFolders } from '../../views/viewBase';
@@ -36,12 +39,10 @@ interface Context {
 	title: string;
 }
 
-type Flags = '--interactive';
-
 interface State {
 	repo: string | Repository;
 	destination: GitReference;
-	flags: Flags[];
+	options: RebaseOptions;
 }
 
 export interface RebaseGitCommandArgs {
@@ -79,15 +80,18 @@ export class RebaseGitCommand extends QuickCommand<State> {
 	}
 
 	async execute(state: RebaseStepState) {
-		let configs: string[] | undefined;
-		if (state.flags.includes('--interactive')) {
+		const configs: { sequenceEditor?: string } = {};
+		if (state.options?.interactive) {
 			await this.container.rebaseEditor.enableForNextUse();
-
-			const editor = getEditorCommand();
-			configs = ['-c', `"sequence.editor=${editor}"`];
+			configs.sequenceEditor = getEditorCommand();
 		}
 
-		state.repo.rebase(configs, ...state.flags, state.destination.ref);
+		try {
+			await state.repo.git.rebase(null, state.destination.ref, configs, state.options);
+		} catch (ex) {
+			Logger.error(ex, this.title);
+			void showGenericErrorMessage(ex);
+		}
 	}
 
 	protected async *steps(state: PartialStepState<State>): StepGenerator {
@@ -103,8 +107,10 @@ export class RebaseGitCommand extends QuickCommand<State> {
 			title: this.title,
 		};
 
-		if (state.flags == null) {
-			state.flags = [];
+		if (state.options == null) {
+			state.options = {
+				autostash: true,
+			};
 		}
 
 		let skippedStepOne = false;
@@ -207,7 +213,7 @@ export class RebaseGitCommand extends QuickCommand<State> {
 			const result = yield* this.confirmStep(state as RebaseStepState, context);
 			if (result === StepResultBreak) continue;
 
-			state.flags = result;
+			state.options = Object.assign({ autostash: true }, ...result);
 
 			endSteps(state);
 			void this.execute(state as RebaseStepState);
@@ -216,7 +222,7 @@ export class RebaseGitCommand extends QuickCommand<State> {
 		return state.counter < 0 ? StepResultBreak : undefined;
 	}
 
-	private async *confirmStep(state: RebaseStepState, context: Context): AsyncStepResultGenerator<Flags[]> {
+	private async *confirmStep(state: RebaseStepState, context: Context): AsyncStepResultGenerator<RebaseOptions[]> {
 		const counts = await this.container.git.getLeftRightCommitCount(
 			state.repo.path,
 			createRevisionRange(state.destination.ref, context.branch.ref, '...'),
@@ -248,8 +254,40 @@ export class RebaseGitCommand extends QuickCommand<State> {
 			return StepResultBreak;
 		}
 
+		try {
+			await state.repo.git.rebase(null, null, undefined, { checkActiveRebase: true });
+		} catch {
+			const step: QuickPickStep<FlagsQuickPickItem<RebaseOptions>> = this.createConfirmStep(
+				appendReposToTitle(title, state, context),
+				[
+					createFlagsQuickPickItem<RebaseOptions>([], [{ abort: true }], {
+						label: 'Abort Rebase',
+						description: '--abort',
+						detail: 'Will abort the current rebase',
+					}),
+					createFlagsQuickPickItem<RebaseOptions>([], [{ continue: true }], {
+						label: 'Continue Rebase',
+						description: '--continue',
+						detail: 'Will continue the current rebase',
+					}),
+					createFlagsQuickPickItem<RebaseOptions>([], [{ skip: true }], {
+						label: 'Skip Rebase',
+						description: '--skip',
+						detail: 'Will skip the current commit and continue the rebase',
+					}),
+				],
+				createDirectiveQuickPickItem(Directive.Cancel, true, {
+					label: 'Do nothing. A rebase is already in progress',
+					detail: "If that is not the case, you can run `rm -rf '.git/rebase-merge'` and try again",
+				}),
+			);
+
+			const selection: StepSelection<typeof step> = yield step;
+			return canPickStepContinue(step, state, selection) ? selection[0].item : StepResultBreak;
+		}
+
 		const rebaseItems = [
-			createFlagsQuickPickItem<Flags>(state.flags, ['--interactive'], {
+			createFlagsQuickPickItem<RebaseOptions>([], [{ interactive: true }], {
 				label: `Interactive ${this.title}`,
 				description: '--interactive',
 				detail: `Will interactively update ${getReferenceLabel(context.branch, {
@@ -262,7 +300,7 @@ export class RebaseGitCommand extends QuickCommand<State> {
 
 		if (behind > 0) {
 			rebaseItems.unshift(
-				createFlagsQuickPickItem<Flags>(state.flags, [], {
+				createFlagsQuickPickItem<RebaseOptions>([], [{}], {
 					label: this.title,
 					detail: `Will update ${getReferenceLabel(context.branch, {
 						label: false,
@@ -273,10 +311,11 @@ export class RebaseGitCommand extends QuickCommand<State> {
 			);
 		}
 
-		const step: QuickPickStep<FlagsQuickPickItem<Flags>> = this.createConfirmStep(
+		const step: QuickPickStep<FlagsQuickPickItem<RebaseOptions>> = this.createConfirmStep(
 			appendReposToTitle(`Confirm ${title}`, state, context),
 			rebaseItems,
 		);
+
 		const selection: StepSelection<typeof step> = yield step;
 		return canPickStepContinue(step, state, selection) ? selection[0].item : StepResultBreak;
 	}

src/env/node/git/git.ts

@@ -20,6 +20,8 @@ import {
 	PullErrorReason,
 	PushError,
 	PushErrorReason,
+	RebaseError,
+	RebaseErrorReason,
 	StashPushError,
 	StashPushErrorReason,
 	TagError,
@@ -105,6 +107,8 @@ export const GitErrors = {
 	tagNotFound: /tag .* not found/i,
 	invalidTagName: /invalid tag name/i,
 	remoteRejected: /rejected because the remote contains work/i,
+	unresolvedConflicts: /^error: could not apply .*\n^hint: Resolve all conflicts.*$/im,
+	rebaseMergeInProgress: /^fatal: It seems that there is already a rebase-merge directory/i,
 };
 
 const GitWarnings = {
@@ -173,6 +177,13 @@ const tagErrorAndReason: [RegExp, TagErrorReason][] = [
 	[GitErrors.remoteRejected, TagErrorReason.RemoteRejected],
 ];
 
+const rebaseErrorAndReason: [RegExp, RebaseErrorReason][] = [
+	[GitErrors.uncommittedChanges, RebaseErrorReason.WorkingChanges],
+	[GitErrors.changesWouldBeOverwritten, RebaseErrorReason.OverwrittenChanges],
+	[GitErrors.unresolvedConflicts, RebaseErrorReason.UnresolvedConflicts],
+	[GitErrors.rebaseMergeInProgress, RebaseErrorReason.RebaseMergeInProgress],
+];
+
 export class Git {
 	/** Map of running git commands -- avoids running duplicate overlaping commands */
 	private readonly pendingCommands = new Map<string, Promise<string | Buffer>>();
@@ -1092,6 +1103,57 @@ export class Git {
 		}
 	}
 
+	async rebase(repoPath: string, args: string[] | undefined = [], configs: string[] | undefined = []): Promise<void> {
+		try {
+			void (await this.git<string>({ cwd: repoPath }, ...configs, 'rebase', ...args));
+		} catch (ex) {
+			const msg: string = ex?.toString() ?? '';
+			for (const [regex, reason] of rebaseErrorAndReason) {
+				if (regex.test(msg) || regex.test(ex.stderr ?? '')) {
+					throw new RebaseError(reason, ex);
+				}
+			}
+
+			throw new RebaseError(RebaseErrorReason.Other, ex);
+		}
+	}
+
+	async check_active_rebase(repoPath: string): Promise<boolean> {
+		try {
+			const data = await this.git<string>({ cwd: repoPath }, 'rev-parse', '--verify', 'REBASE_HEAD');
+			return Boolean(data.length);
+		} catch {
+			return false;
+		}
+	}
+
+	async check_active_cherry_pick(repoPath: string): Promise<boolean> {
+		try {
+			const data = await this.git<string>({ cwd: repoPath }, 'rev-parse', '--verify', 'CHERRY_PICK_HEAD');
+			return Boolean(data.length);
+		} catch (_ex) {
+			return true;
+		}
+	}
+
+	async check_active_merge(repoPath: string): Promise<boolean> {
+		try {
+			const data = await this.git<string>({ cwd: repoPath }, 'rev-parse', '--verify', 'MERGE_HEAD');
+			return Boolean(data.length);
+		} catch (_ex) {
+			return true;
+		}
+	}
+
+	async check_active_cherry_revert(repoPath: string): Promise<boolean> {
+		try {
+			const data = await this.git<string>({ cwd: repoPath }, 'rev-parse', '--verify', 'REVERT_HEAD');
+			return Boolean(data.length);
+		} catch (_ex) {
+			return true;
+		}
+	}
+
 	for_each_ref__branch(repoPath: string, options: { all: boolean } = { all: false }) {
 		const params = ['for-each-ref', `--format=${parseGitBranchesDefaultFormat}`, 'refs/heads'];
 		if (options.all) {

src/env/node/git/localGitProvider.ts

@@ -31,6 +31,7 @@ import {
 	PullError,
 	PushError,
 	PushErrorReason,
+	RebaseError,
 	StashApplyError,
 	StashApplyErrorReason,
 	StashPushError,
@@ -52,6 +53,7 @@ import type {
 	PagingOptions,
 	PreviousComparisonUrisResult,
 	PreviousLineComparisonUrisResult,
+	RebaseOptions,
 	RepositoryCloseEvent,
 	RepositoryInitWatcher,
 	RepositoryOpenEvent,
@@ -1658,6 +1660,65 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		}
 	}
 
+	@log()
+	async rebase(
+		repoPath: string,
+		upstream: string | null,
+		ref: string | null,
+		configs?: { sequenceEditor?: string },
+		options?: RebaseOptions = {},
+	): Promise<void> {
+		const configFlags = [];
+		const args = [];
+
+		if (options?.checkActiveRebase) {
+			if (await this.git.check_active_rebase(repoPath)) {
+				throw new RebaseError(RebaseErrorReason.RebaseMergeInProgress);
+			}
+
+			return;
+		}
+
+		if (configs?.sequenceEditor != null) {
+			configFlags.push('-c', `sequence.editor="${configs.sequenceEditor}"`);
+		}
+
+		// These options can only be used on their own
+		if (options?.abort) {
+			args.push('--abort');
+		} else if (options?.continue) {
+			args.push('--continue');
+		} else if (options?.skip) {
+			args.push('--skip');
+		} else {
+			if (options?.autostash) {
+				args.push('--autostash');
+			}
+
+			if (options?.interactive) {
+				args.push('--interactive');
+			}
+
+			if (upstream) {
+				args.push(upstream);
+			}
+
+			if (ref) {
+				args.push(ref);
+			}
+		}
+
+		try {
+			await this.git.rebase(repoPath, args, configFlags);
+		} catch (ex) {
+			if (RebaseError.is(ex)) {
+				throw ex.WithRef(ref);
+			}
+
+			throw ex;
+		}
+	}
+
 	private readonly toCanonicalMap = new Map<string, Uri>();
 	private readonly fromCanonicalMap = new Map<string, Uri>();
 	protected readonly unsafePaths = new Set<string>();

src/git/actions/repository.ts

@@ -34,7 +34,7 @@ export function push(repos?: string | string[] | Repository | Repository[], forc
 export function rebase(repo?: string | Repository, ref?: GitReference, interactive: boolean = true) {
 	return executeGitCommand({
 		command: 'rebase',
-		state: { repo: repo, destination: ref, flags: interactive ? ['--interactive'] : [] },
+		state: { repo: repo, destination: ref, options: { interactive: interactive, autostash: true } },
 	});
 }