From 87b29d14d0ca8cf70bc733a612603cd1818e0cfe Mon Sep 17 00:00:00 2001 From: Sergio Date: Tue, 3 Dec 2024 16:24:01 +0100 Subject: [PATCH] add git revert command --- src/commands/git/revert.ts | 45 ++++++++++++++++++-- src/env/node/git/git.ts | 25 +++++++++++ src/env/node/git/localGitProvider.ts | 19 +++++++++ src/git/errors.ts | 62 ++++++++++++++++++++++++++++ src/git/gitProvider.ts | 1 + src/git/gitProviderService.ts | 22 ++++++++++ src/git/models/repository.ts | 5 --- src/messages.ts | 16 +++++++ 8 files changed, 187 insertions(+), 8 deletions(-) diff --git a/src/commands/git/revert.ts b/src/commands/git/revert.ts index fdb832c2fc668..9cd8725a5469f 100644 --- a/src/commands/git/revert.ts +++ b/src/commands/git/revert.ts @@ -1,11 +1,16 @@ +import { Commands } from '../../constants.commands'; import type { Container } from '../../container'; +import { RevertError, RevertErrorReason } from '../../git/errors'; import type { GitBranch } from '../../git/models/branch'; import type { GitLog } from '../../git/models/log'; import type { GitRevisionReference } from '../../git/models/reference'; import { getReferenceLabel } from '../../git/models/reference'; import type { Repository } from '../../git/models/repository'; +import { showGenericErrorMessage, showShouldCommitOrStashPrompt } from '../../messages'; import type { FlagsQuickPickItem } from '../../quickpicks/items/flags'; import { createFlagsQuickPickItem } from '../../quickpicks/items/flags'; +import { Logger } from '../../system/logger'; +import { executeCommand, executeCoreCommand } from '../../system/vscode/command'; import type { ViewsWithRepositoryFolders } from '../../views/viewBase'; import type { PartialStepState, @@ -71,8 +76,42 @@ export class RevertGitCommand extends QuickCommand { return false; } - execute(state: RevertStepState>) { - state.repo.revert(...state.flags, ...state.references.map(c => c.ref).reverse()); + async execute(state: RevertStepState>) { + for (const ref of state.references.reverse()) { + try { + await state.repo.git.revert(ref.ref, state.flags); + } catch (ex) { + if (ex instanceof RevertError) { + let shouldRetry = false; + if (ex.reason === RevertErrorReason.LocalChangesWouldBeOverwritten) { + const response = await showShouldCommitOrStashPrompt(); + if (response === 'Stash') { + await executeCommand(Commands.GitCommandsStashPush); + shouldRetry = true; + } else if (response === 'Commit') { + await executeCoreCommand('workbench.view.scm'); + shouldRetry = true; + } else { + continue; + } + } + + if (shouldRetry) { + try { + await state.repo.git.revert(ref.ref, state.flags); + } catch (ex) { + Logger.error(ex, this.title); + void showGenericErrorMessage(ex.message); + } + } + + continue; + } + + Logger.error(ex, this.title); + void showGenericErrorMessage(ex.message); + } + } } protected async *steps(state: PartialStepState): StepGenerator { @@ -160,7 +199,7 @@ export class RevertGitCommand extends QuickCommand { state.flags = result; endSteps(state); - this.execute(state as RevertStepState>); + await this.execute(state as RevertStepState>); } return state.counter < 0 ? StepResultBreak : undefined; diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index f109e990e0532..dbae91222c5ab 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -20,6 +20,8 @@ import { PullErrorReason, PushError, PushErrorReason, + RevertError, + RevertErrorReason, StashPushError, StashPushErrorReason, TagError, @@ -105,6 +107,7 @@ export const GitErrors = { tagNotFound: /tag .* not found/i, invalidTagName: /invalid tag name/i, remoteRejected: /rejected because the remote contains work/i, + revertHasConflicts: /(error: could not revert .*) (hint: After resolving the conflicts)/gi, }; const GitWarnings = { @@ -173,6 +176,13 @@ const tagErrorAndReason: [RegExp, TagErrorReason][] = [ [GitErrors.remoteRejected, TagErrorReason.RemoteRejected], ]; +const revertErrorAndReason = [ + [GitErrors.badRevision, RevertErrorReason.BadRevision], + [GitErrors.invalidObjectName, RevertErrorReason.InvalidObjectName], + [GitErrors.revertHasConflicts, RevertErrorReason.Conflict], + [GitErrors.changesWouldBeOverwritten, RevertErrorReason.LocalChangesWouldBeOverwritten], +]; + export class Git { /** Map of running git commands -- avoids running duplicate overlaping commands */ private readonly pendingCommands = new Map>(); @@ -1588,6 +1598,21 @@ export class Git { return this.git({ cwd: repoPath }, 'reset', '-q', '--', ...pathspecs); } + async revert(repoPath: string, ...args: string[]) { + try { + await this.git({ cwd: repoPath }, 'revert', ...args); + } catch (ex) { + const msg: string = ex?.toString() ?? ''; + for (const [error, reason] of revertErrorAndReason) { + if (error.test(msg) || error.test(ex.stderr ?? '')) { + throw new RevertError(reason, ex); + } + } + + throw new RevertError(RevertErrorReason.Other, ex); + } + } + async rev_list( repoPath: string, ref: string, diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 20a16c579551f..2dfcc2b5c7cd4 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -31,6 +31,7 @@ import { PullError, PushError, PushErrorReason, + RevertError, StashApplyError, StashApplyErrorReason, StashPushError, @@ -6044,6 +6045,24 @@ export class LocalGitProvider implements GitProvider, Disposable { return worktrees; } + @log() + async revert(repoPath: string, ref: string, options?: { edit?: boolean }): Promise { + const args = []; + if (options.edit !== undefined) { + args.push(options.edit ? '--edit' : '--no-edit'); + } + + try { + await this.git.revert(repoPath, ...args, ref); + } catch (ex) { + if (ex instanceof RevertError) { + throw ex.WithRef(ref); + } + + throw ex; + } + } + @log() // eslint-disable-next-line @typescript-eslint/require-await async getWorktreesDefaultUri(repoPath: string): Promise { diff --git a/src/git/errors.ts b/src/git/errors.ts index e1ef081fdfb25..1117093e0bc6c 100644 --- a/src/git/errors.ts +++ b/src/git/errors.ts @@ -567,3 +567,65 @@ export class TagError extends Error { return this; } } + +export const enum RevertErrorReason { + BadRevision, + InvalidObjectName, + Conflict, + LocalChangesWouldBeOverwritten, + Other, +} + +export class RevertError extends Error { + static is(ex: unknown, reason?: RevertErrorReason): ex is RevertError { + return ex instanceof RevertError && (reason == null || ex.reason === reason); + } + + readonly original?: Error; + readonly reason: RevertErrorReason | undefined; + ref?: string; + + constructor(reason?: RevertErrorReason, original?: Error, ref?: string); + constructor(message?: string, original?: Error); + constructor(messageOrReason: string | RevertErrorReason | undefined, original?: Error, ref?: string) { + let message; + let reason: RevertErrorReason | undefined; + if (messageOrReason == null) { + message = 'Unable to revert'; + } else if (typeof messageOrReason === 'string') { + message = messageOrReason; + reason = undefined; + } else { + reason = messageOrReason; + message = RevertError.buildRevertErrorMessage(reason, ref); + } + super(message); + + this.original = original; + this.reason = reason; + this.ref = ref; + Error.captureStackTrace?.(this, RevertError); + } + + WithRef(ref: string): this { + this.ref = ref; + this.message = RevertError.buildRevertErrorMessage(this.reason, ref); + return this; + } + + private static buildRevertErrorMessage(reason?: RevertErrorReason, ref?: string): string { + const baseMessage = `Unable to revert${ref ? ` revision '${ref}'` : ''}`; + switch (reason) { + case RevertErrorReason.BadRevision: + return `${baseMessage} because it is not a valid revision.`; + case RevertErrorReason.InvalidObjectName: + return `${baseMessage} because it is not a valid object name.`; + case RevertErrorReason.Conflict: + return `${baseMessage} it has unresolved conflicts. Resolve the conflicts and try again.`; + case RevertErrorReason.LocalChangesWouldBeOverwritten: + return `${baseMessage} because local changes would be overwritten. Commit or stash your changes first.`; + default: + return `${baseMessage}.`; + } + } +} diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index 74450e2a2a829..e9cffbfed0869 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -125,6 +125,7 @@ export interface GitProviderRepository { addRemote?(repoPath: string, name: string, url: string, options?: { fetch?: boolean }): Promise; pruneRemote?(repoPath: string, name: string): Promise; removeRemote?(repoPath: string, name: string): Promise; + revert?(repoPath: string, ref: string, options?: { edit?: boolean }): Promise; applyUnreachableCommitForPatch?( repoPath: string, diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index aeb41768e11f0..aab5d3645fe12 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -1334,6 +1334,28 @@ export class GitProviderService implements Disposable { return provider.removeRemote(path, name); } + @log() + async revert(repoPath: string | Uri, ref: string, flags: string[] | undefined = []): Promise { + const { provider, path } = this.getProvider(repoPath); + if (provider.revert == null) throw new ProviderNotSupportedError(provider.descriptor.name); + + const options: { edit?: boolean } = {}; + for (const flag of flags) { + switch (flag) { + case '--edit': + options.edit = true; + break; + case '--no-edit': + options.edit = false; + break; + default: + break; + } + } + + return provider.revert(path, ref, options); + } + @log() applyChangesToWorkingFile(uri: GitUri, ref1?: string, ref2?: string): Promise { const { provider } = this.getProvider(uri); diff --git a/src/git/models/repository.ts b/src/git/models/repository.ts index a536ad7b622fe..300813d55e5e2 100644 --- a/src/git/models/repository.ts +++ b/src/git/models/repository.ts @@ -870,11 +870,6 @@ export class Repository implements Disposable { } } - @log() - revert(...args: string[]) { - void this.runTerminalCommand('revert', ...args); - } - async setRemoteAsDefault(remote: GitRemote, value: boolean = true) { await this.container.storage.storeWorkspace('remote:default', value ? remote.name : undefined); diff --git a/src/messages.ts b/src/messages.ts index d0370344c4995..81a6040c73f0e 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -230,6 +230,22 @@ export function showIntegrationRequestTimedOutWarningMessage(providerName: strin ); } +export async function showShouldCommitOrStashPrompt(): Promise { + const stash = { title: 'Stash' }; + const commit = { title: 'Commit' }; + const cancel = { title: 'Cancel', isCloseAffordance: true }; + const result = await showMessage( + 'warn', + 'You have changes in your working tree. Commit or stash them before reverting', + undefined, + null, + stash, + commit, + cancel, + ); + return result?.title; +} + export async function showWhatsNewMessage(majorVersion: string) { const confirm = { title: 'OK', isCloseAffordance: true }; const releaseNotes = { title: 'View Release Notes' };