Closes #3531 improves cherry-pick

Author: eamodio

CHANGELOG.md

@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 
 - Improves editor revision navigation ([#4200](https://github.com/gitkraken/vscode-gitlens/issues/4200))
 - Improves AI-related error messages ([#4227](https://github.com/gitkraken/vscode-gitlens/issues/4227))
+- Changes cherry-pick command no longer use/open a terminal ([#3531](https://github.com/gitkraken/vscode-gitlens/issues/3531))
 
 ### Fixed
 

src/commands/git/cherry-pick.ts

@@ -1,12 +1,19 @@
+import { window } from 'vscode';
 import type { Container } from '../../container';
+import { skipPausedOperation } from '../../git/actions/pausedOperation';
+import { CherryPickError, CherryPickErrorReason } from '../../git/errors';
 import type { GitBranch } from '../../git/models/branch';
 import type { GitLog } from '../../git/models/log';
+import type { GitPausedOperationStatus } from '../../git/models/pausedOperationStatus';
 import type { GitReference } from '../../git/models/reference';
 import type { Repository } from '../../git/models/repository';
 import { getReferenceLabel, isRevisionReference } from '../../git/utils/reference.utils';
 import { createRevisionRange } from '../../git/utils/revision.utils';
+import { showGenericErrorMessage } from '../../messages';
 import type { FlagsQuickPickItem } from '../../quickpicks/items/flags';
 import { createFlagsQuickPickItem } from '../../quickpicks/items/flags';
+import { executeCommand } from '../../system/-webview/command';
+import { Logger } from '../../system/logger';
 import type { ViewsWithRepositoryFolders } from '../../views/viewBase';
 import type {
 	PartialStepState,
@@ -81,8 +88,50 @@ export class CherryPickGitCommand extends QuickCommand<State> {
 		return false;
 	}
 
-	private execute(state: CherryPickStepState<State<GitReference[]>>) {
-		state.repo.cherryPick(...state.flags, ...state.references.map(c => c.ref).reverse());
+	private async execute(state: CherryPickStepState<State<GitReference[]>>) {
+		try {
+			await state.repo.git.commits().cherryPick?.(
+				state.references.map(c => c.ref),
+				{
+					edit: state.flags.includes('--edit'),
+					noCommit: state.flags.includes('--no-commit'),
+				},
+			);
+		} catch (ex) {
+			Logger.error(ex, this.title);
+			if (ex instanceof CherryPickError && ex.reason === CherryPickErrorReason.EmptyCommit) {
+				let pausedOperation: GitPausedOperationStatus | undefined;
+				try {
+					pausedOperation = await state.repo.git.status().getPausedOperationStatus?.();
+					pausedOperation ??= await state.repo
+						.waitForRepoChange(500)
+						.then(() => state.repo.git.status().getPausedOperationStatus?.());
+				} catch {}
+
+				const pausedAt = pausedOperation
+					? getReferenceLabel(pausedOperation?.incoming, { icon: false, label: true, quoted: true })
+					: undefined;
+
+				const skip = { title: 'Skip' };
+				const cancel = { title: 'Cancel', isCloseAffordance: true };
+				const result = await window.showInformationMessage(
+					`The cherry-pick operation cannot be completed because ${
+						pausedAt ?? 'it'
+					} resulted in an empty commit.\n\nDo you want to skip ${pausedAt ?? 'this commit'}?`,
+					{ modal: true },
+					skip,
+					cancel,
+				);
+				if (result === skip) {
+					return void skipPausedOperation(state.repo);
+				}
+
+				void executeCommand('gitlens.showCommitsView');
+				return;
+			}
+
+			void showGenericErrorMessage(ex.message);
+		}
 	}
 
 	override isFuzzyMatch(name: string): boolean {
@@ -143,7 +192,7 @@ export class CherryPickGitCommand extends QuickCommand<State> {
 				label: false,
 			})}`;
 
-			if (state.counter < 2 || state.references == null || state.references.length === 0) {
+			if (state.counter < 2 || !state.references?.length) {
 				const result: StepResult<GitReference> = yield* pickBranchOrTagStep(
 					state as CherryPickStepState,
 					context,
@@ -225,7 +274,7 @@ export class CherryPickGitCommand extends QuickCommand<State> {
 			}
 
 			endSteps(state);
-			this.execute(state as CherryPickStepState<State<GitReference[]>>);
+			void this.execute(state as CherryPickStepState<State<GitReference[]>>);
 		}
 
 		return state.counter < 0 ? StepResultBreak : undefined;

src/env/node/git/git.ts

@@ -14,8 +14,6 @@ import { GitErrorHandling } from '../../../git/commandOptions';
 import {
 	BlameIgnoreRevsFileBadRevisionError,
 	BlameIgnoreRevsFileError,
-	CherryPickError,
-	CherryPickErrorReason,
 	FetchError,
 	FetchErrorReason,
 	PullError,
@@ -83,6 +81,7 @@ export const GitErrors = {
 	commitChangesFirst: /Please, commit your changes before you can/i,
 	conflict: /^CONFLICT \([^)]+\): \b/m,
 	detachedHead: /You are in 'detached HEAD' state/i,
+	emptyPreviousCherryPick: /The previous cherry-pick is now empty/i,
 	entryNotUpToDate: /error:\s*Entry ['"].+['"] not uptodate\. Cannot merge\./i,
 	failedToDeleteDirectoryNotEmpty: /failed to delete '(.*?)': Directory not empty/i,
 	invalidLineCount: /file .+? has only \d+ lines/i,
@@ -568,35 +567,6 @@ export class Git {
 		return this.exec({ cwd: repoPath }, ...params);
 	}
 
-	async cherrypick(
-		repoPath: string,
-		sha: string,
-		options: { noCommit?: boolean; errors?: GitErrorHandling } = {},
-	): Promise<void> {
-		const params = ['cherry-pick'];
-		if (options?.noCommit) {
-			params.push('-n');
-		}
-		params.push(sha);
-
-		try {
-			await this.exec({ cwd: repoPath, errors: options?.errors }, ...params);
-		} catch (ex) {
-			const msg: string = ex?.toString() ?? '';
-			let reason: CherryPickErrorReason = CherryPickErrorReason.Other;
-			if (
-				GitErrors.changesWouldBeOverwritten.test(msg) ||
-				GitErrors.changesWouldBeOverwritten.test(ex.stderr ?? '')
-			) {
-				reason = CherryPickErrorReason.AbortedWouldOverwrite;
-			} else if (GitErrors.conflict.test(msg) || GitErrors.conflict.test(ex.stdout ?? '')) {
-				reason = CherryPickErrorReason.Conflicts;
-			}
-
-			throw new CherryPickError(reason, ex, sha);
-		}
-	}
-
 	// TODO: Expand to include options and other params
 	async clone(url: string, parentPath: string): Promise<string | undefined> {
 		let count = 0;

src/env/node/git/sub-providers/commits.ts

@@ -4,6 +4,7 @@ import type { SearchQuery } from '../../../../constants.search';
 import type { Container } from '../../../../container';
 import type { GitCache } from '../../../../git/cache';
 import { GitErrorHandling } from '../../../../git/commandOptions';
+import { CherryPickError, CherryPickErrorReason } from '../../../../git/errors';
 import type { GitCommitsSubProvider, LeftRightCommitCountResult } from '../../../../git/gitProvider';
 import { GitUri } from '../../../../git/gitUri';
 import type { GitBlame } from '../../../../git/models/blame';
@@ -21,6 +22,7 @@ import { parseGitDiffNameStatusFiles } from '../../../../git/parsers/diffParser'
 import {
 	createLogParserSingle,
 	createLogParserWithFilesAndStats,
+	getShaAndDatesLogParser,
 	LogType,
 	parseGitLog,
 	parseGitLogAllFormat,
@@ -41,7 +43,7 @@ import { isFolderGlob } from '../../../../system/path';
 import type { CachedLog, TrackedGitDocument } from '../../../../trackers/trackedDocument';
 import { GitDocumentState } from '../../../../trackers/trackedDocument';
 import type { Git } from '../git';
-import { gitLogDefaultConfigs, gitLogDefaultConfigsWithFiles } from '../git';
+import { GitErrors, gitLogDefaultConfigs, gitLogDefaultConfigsWithFiles } from '../git';
 import type { LocalGitProvider } from '../localGitProvider';
 
 const emptyPromise: Promise<GitBlame | ParsedGitDiffHunks | GitLog | undefined> = Promise.resolve(undefined);
@@ -59,6 +61,61 @@ export class CommitsGitSubProvider implements GitCommitsSubProvider {
 		return configuration.get('advanced.caching.enabled');
 	}
 
+	@log()
+	async cherryPick(
+		repoPath: string,
+		revs: string[],
+		options?: { edit?: boolean; noCommit?: boolean },
+	): Promise<void> {
+		const args = ['cherry-pick'];
+		if (options?.edit) {
+			args.push('-e');
+		}
+		if (options?.noCommit) {
+			args.push('-n');
+		}
+
+		if (revs.length > 1) {
+			const parser = getShaAndDatesLogParser();
+			// Ensure the revs are in reverse committer date order
+			const data = await this.git.exec(
+				{ cwd: repoPath, stdin: join(revs, '\n') },
+				'log',
+				'--no-walk',
+				'--stdin',
+				...parser.arguments,
+				'--',
+			);
+			const commits = [...parser.parse(data)].sort(
+				(c1, c2) => Number(c1.committerDate) - Number(c2.committerDate),
+			);
+			revs = commits.map(c => c.sha);
+		}
+
+		args.push(...revs);
+
+		try {
+			await this.git.exec({ cwd: repoPath, errors: GitErrorHandling.Throw }, ...args);
+		} catch (ex) {
+			const msg: string = ex?.toString() ?? '';
+
+			let reason: CherryPickErrorReason = CherryPickErrorReason.Other;
+			if (
+				GitErrors.changesWouldBeOverwritten.test(msg) ||
+				GitErrors.changesWouldBeOverwritten.test(ex.stderr ?? '')
+			) {
+				reason = CherryPickErrorReason.AbortedWouldOverwrite;
+			} else if (GitErrors.conflict.test(msg) || GitErrors.conflict.test(ex.stdout ?? '')) {
+				reason = CherryPickErrorReason.Conflicts;
+			} else if (GitErrors.emptyPreviousCherryPick.test(msg)) {
+				reason = CherryPickErrorReason.EmptyCommit;
+			}
+
+			debugger;
+			throw new CherryPickError(reason, ex, revs);
+		}
+	}
+
 	@log()
 	async getCommit(repoPath: string, rev: string): Promise<GitCommit | undefined> {
 		if (isUncommitted(rev, true)) {

src/env/node/git/sub-providers/graph.ts

@@ -25,8 +25,8 @@ import type { GitWorktree } from '../../../../git/models/worktree';
 import {
 	getGraphParser,
 	getGraphStatsParser,
-	getRefAndDateParser,
-	getRefParser,
+	getShaAndDatesLogParser,
+	getShaLogParser,
 } from '../../../../git/parsers/logParser';
 import type { GitGraphSearch, GitGraphSearchResultData, GitGraphSearchResults } from '../../../../git/search';
 import { getGitArgsFromSearchQuery, getSearchQueryComparisonKey } from '../../../../git/search';
@@ -86,12 +86,12 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
 		const deferStats = options?.include?.stats; // && defaultLimit > 1000;
 
 		const parser = getGraphParser(options?.include?.stats && !deferStats);
-		const refParser = getRefParser();
+		const shaParser = getShaLogParser();
 		const statsParser = getGraphStatsParser();
 
-		const [refResult, stashResult, branchesResult, remotesResult, currentUserResult, worktreesResult] =
+		const [shaResult, stashResult, branchesResult, remotesResult, currentUserResult, worktreesResult] =
 			await Promise.allSettled([
-				this.git.log(repoPath, undefined, undefined, ...refParser.arguments, '-n1', rev ?? 'HEAD'),
+				this.git.log(repoPath, undefined, undefined, ...shaParser.arguments, '-n1', rev ?? 'HEAD'),
 				this.provider.stash?.getStash(repoPath),
 				this.provider.branches.getBranches(repoPath),
 				this.provider.remotes.getRemotes(repoPath),
@@ -121,7 +121,7 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
 
 		const remotes = getSettledValue(remotesResult);
 		const remoteMap = remotes != null ? new Map(remotes.map(r => [r.name, r])) : new Map<string, GitRemote>();
-		const selectSha = first(refParser.parse(getSettledValue(refResult) ?? ''));
+		const selectSha = first(shaParser.parse(getSettledValue(shaResult) ?? ''));
 
 		const downstreamMap = new Map<string, string[]>();
 
@@ -660,7 +660,7 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
 
 		const comparisonKey = getSearchQueryComparisonKey(search);
 		try {
-			const refAndDateParser = getRefAndDateParser();
+			const parser = getShaAndDatesLogParser();
 
 			const currentUser = search.query.includes('@me')
 				? await this.provider.config.getCurrentUser(repoPath)
@@ -672,15 +672,15 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
 					{ cwd: repoPath, cancellation: options?.cancellation, configs: gitLogDefaultConfigs },
 					'show',
 					'-s',
-					...refAndDateParser.arguments,
+					...parser.arguments,
 					...shas.values(),
 					...searchArgs,
 					'--',
 				);
 
 				let i = 0;
 				const results: GitGraphSearchResults = new Map<string, GitGraphSearchResultData>(
-					map(refAndDateParser.parse(data), c => [
+					map(parser.parse(data), c => [
 						c.sha,
 						{
 							i: i++,
@@ -720,7 +720,7 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
 			}
 
 			const args = [
-				...refAndDateParser.arguments,
+				...parser.arguments,
 				`-M${similarityThreshold == null ? '' : `${similarityThreshold}%`}`,
 				'--use-mailmap',
 			];
@@ -770,7 +770,7 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
 
 				let count = total;
 
-				for (const r of refAndDateParser.parse(data)) {
+				for (const r of parser.parse(data)) {
 					if (includeOnlyStashes && !stashes?.has(r.sha)) continue;
 
 					if (results.has(r.sha)) {

src/env/node/git/sub-providers/patch.ts

@@ -1,7 +1,6 @@
 import { window } from 'vscode';
 import type { Container } from '../../../../container';
 import { CancellationError } from '../../../../errors';
-import { GitErrorHandling } from '../../../../git/commandOptions';
 import {
 	ApplyPatchCommitError,
 	ApplyPatchCommitErrorReason,
@@ -130,7 +129,7 @@ export class PatchGitSubProvider implements GitPatchSubProvider {
 
 		// Apply the patch using a cherry pick without committing
 		try {
-			await this.git.cherrypick(targetPath, rev, { noCommit: true, errors: GitErrorHandling.Throw });
+			await this.provider.commits.cherryPick(targetPath, [rev], { noCommit: true });
 		} catch (ex) {
 			Logger.error(ex, scope);
 			if (ex instanceof CherryPickError) {