3529 Change tag terminal-run commands into normal commands w/ proper error handling (#3670)

* change git tag terminal-run cmds into normal cmds

* customize tag errors with action

* rebase & lint errors

Author: sergiolms

src/commands/git/tag.ts

@@ -8,9 +8,11 @@ import {
 	isTagReference,
 } from '../../git/models/reference';
 import type { Repository } from '../../git/models/repository';
+import { showGenericErrorMessage } from '../../messages';
 import type { QuickPickItemOfT } from '../../quickpicks/items/common';
 import type { FlagsQuickPickItem } from '../../quickpicks/items/flags';
 import { createFlagsQuickPickItem } from '../../quickpicks/items/flags';
+import { Logger } from '../../system/logger';
 import { pluralize } from '../../system/string';
 import type { ViewsWithRepositoryFolders } from '../../views/viewBase';
 import type {
@@ -293,12 +295,12 @@ export class TagGitCommand extends QuickCommand<State> {
 			}
 
 			endSteps(state);
-			state.repo.tag(
-				...state.flags,
-				...(state.message.length !== 0 ? [`"${state.message}"`] : []),
-				state.name,
-				state.reference.ref,
-			);
+			try {
+				await state.repo.git.createTag(state.name, state.reference.ref, state.message);
+			} catch (ex) {
+				Logger.error(ex, context.title);
+				void showGenericErrorMessage(ex);
+			}
 		}
 	}
 
@@ -356,7 +358,7 @@ export class TagGitCommand extends QuickCommand<State> {
 		return canPickStepContinue(step, state, selection) ? selection[0].item : StepResultBreak;
 	}
 
-	private *deleteCommandSteps(state: DeleteStepState, context: Context): StepGenerator {
+	private async *deleteCommandSteps(state: DeleteStepState, context: Context): StepGenerator {
 		while (this.canStepsContinue(state)) {
 			if (state.references != null && !Array.isArray(state.references)) {
 				state.references = [state.references];
@@ -381,7 +383,14 @@ export class TagGitCommand extends QuickCommand<State> {
 			if (result === StepResultBreak) continue;
 
 			endSteps(state);
-			state.repo.tagDelete(state.references);
+			for (const { ref } of state.references) {
+				try {
+					await state.repo.git.deleteTag(ref);
+				} catch (ex) {
+					Logger.error(ex, context.title);
+					void showGenericErrorMessage(ex);
+				}
+			}
 		}
 	}
 

src/env/node/git/git.ts

@@ -22,6 +22,8 @@ import {
 	PushErrorReason,
 	StashPushError,
 	StashPushErrorReason,
+	TagError,
+	TagErrorReason,
 	WorkspaceUntrustedError,
 } from '../../../git/errors';
 import type { GitDir } from '../../../git/gitProvider';
@@ -32,7 +34,6 @@ import type { GitUser } from '../../../git/models/user';
 import { parseGitBranchesDefaultFormat } from '../../../git/parsers/branchParser';
 import { parseGitLogAllFormat, parseGitLogDefaultFormat } from '../../../git/parsers/logParser';
 import { parseGitRemoteUrl } from '../../../git/parsers/remoteParser';
-import { parseGitTagsDefaultFormat } from '../../../git/parsers/tagParser';
 import { splitAt } from '../../../system/array';
 import { log } from '../../../system/decorators/log';
 import { join } from '../../../system/iterable';
@@ -100,6 +101,10 @@ export const GitErrors = {
 	tagConflict: /! \[rejected\].*\(would clobber existing tag\)/m,
 	unmergedFiles: /is not possible because you have unmerged files/i,
 	unstagedChanges: /You have unstaged changes/i,
+	tagAlreadyExists: /tag .* already exists/i,
+	tagNotFound: /tag .* not found/i,
+	invalidTagName: /invalid tag name/i,
+	remoteRejected: /rejected because the remote contains work/i,
 };
 
 const GitWarnings = {
@@ -160,6 +165,14 @@ function getStdinUniqueKey(): number {
 type ExitCodeOnlyGitCommandOptions = GitCommandOptions & { exitCodeOnly: true };
 export type PushForceOptions = { withLease: true; ifIncludes?: boolean } | { withLease: false; ifIncludes?: never };
 
+const tagErrorAndReason: [RegExp, TagErrorReason][] = [
+	[GitErrors.tagAlreadyExists, TagErrorReason.TagAlreadyExists],
+	[GitErrors.tagNotFound, TagErrorReason.TagNotFound],
+	[GitErrors.invalidTagName, TagErrorReason.InvalidTagName],
+	[GitErrors.permissionDenied, TagErrorReason.PermissionDenied],
+	[GitErrors.remoteRejected, TagErrorReason.RemoteRejected],
+];
+
 export class Git {
 	/** Map of running git commands -- avoids running duplicate overlaping commands */
 	private readonly pendingCommands = new Map<string, Promise<string | Buffer>>();
@@ -2078,8 +2091,19 @@ export class Git {
 		return this.git<string>({ cwd: repoPath }, 'symbolic-ref', '--short', ref);
 	}
 
-	tag(repoPath: string) {
-		return this.git<string>({ cwd: repoPath }, 'tag', '-l', `--format=${parseGitTagsDefaultFormat}`);
+	async tag(repoPath: string, ...args: string[]) {
+		try {
+			const output = await this.git<string>({ cwd: repoPath }, 'tag', ...args);
+			return output;
+		} catch (ex) {
+			const msg: string = ex?.toString() ?? '';
+			for (const [error, reason] of tagErrorAndReason) {
+				if (error.test(msg) || error.test(ex.stderr ?? '')) {
+					throw new TagError(reason, ex);
+				}
+			}
+			throw new TagError(TagErrorReason.Other, ex);
+		}
 	}
 
 	worktree__add(

src/env/node/git/localGitProvider.ts

@@ -34,6 +34,7 @@ import {
 	StashApplyError,
 	StashApplyErrorReason,
 	StashPushError,
+	TagError,
 	WorktreeCreateError,
 	WorktreeCreateErrorReason,
 	WorktreeDeleteError,
@@ -161,7 +162,7 @@ import {
 import { parseGitRefLog, parseGitRefLogDefaultFormat } from '../../../git/parsers/reflogParser';
 import { parseGitRemotes } from '../../../git/parsers/remoteParser';
 import { parseGitStatus } from '../../../git/parsers/statusParser';
-import { parseGitTags } from '../../../git/parsers/tagParser';
+import { parseGitTags, parseGitTagsDefaultFormat } from '../../../git/parsers/tagParser';
 import { parseGitLsFiles, parseGitTree } from '../../../git/parsers/treeParser';
 import { parseGitWorktrees } from '../../../git/parsers/worktreeParser';
 import { getRemoteProviderMatcher, loadRemoteProviders } from '../../../git/remotes/remoteProviders';
@@ -1263,6 +1264,32 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		await this.git.branch(repoPath, '-m', oldName, newName);
 	}
 
+	@log()
+	async createTag(repoPath: string, name: string, ref: string, message?: string): Promise<void> {
+		try {
+			await this.git.tag(repoPath, name, ref, ...(message != null && message.length > 0 ? ['-m', message] : []));
+		} catch (ex) {
+			if (ex instanceof TagError) {
+				throw ex.WithTag(name).WithAction('create');
+			}
+
+			throw ex;
+		}
+	}
+
+	@log()
+	async deleteTag(repoPath: string, name: string): Promise<void> {
+		try {
+			await this.git.tag(repoPath, '-d', name);
+		} catch (ex) {
+			if (ex instanceof TagError) {
+				throw ex.WithTag(name).WithAction('delete');
+			}
+
+			throw ex;
+		}
+	}
+
 	@log()
 	async checkout(
 		repoPath: string,
@@ -5117,7 +5144,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		if (resultsPromise == null) {
 			async function load(this: LocalGitProvider): Promise<PagedResult<GitTag>> {
 				try {
-					const data = await this.git.tag(repoPath!);
+					const data = await this.git.tag(repoPath!, '-l', `--format=${parseGitTagsDefaultFormat}`);
 					return { values: parseGitTags(data, repoPath!) };
 				} catch (_ex) {
 					this._tagsCache.delete(repoPath!);

src/git/errors.ts

@@ -489,3 +489,81 @@ export class WorktreeDeleteError extends Error {
 		Error.captureStackTrace?.(this, WorktreeDeleteError);
 	}
 }
+
+export const enum TagErrorReason {
+	TagAlreadyExists,
+	TagNotFound,
+	InvalidTagName,
+	PermissionDenied,
+	RemoteRejected,
+	Other,
+}
+
+export class TagError extends Error {
+	static is(ex: unknown, reason?: TagErrorReason): ex is TagError {
+		return ex instanceof TagError && (reason == null || ex.reason === reason);
+	}
+
+	readonly original?: Error;
+	readonly reason: TagErrorReason | undefined;
+	action?: string;
+	tag?: string;
+
+	private static buildTagErrorMessage(reason?: TagErrorReason, tag?: string, action?: string): string {
+		let baseMessage: string;
+		if (action != null) {
+			baseMessage = `Unable to ${action} tag ${tag ? `'${tag}'` : ''}`;
+		} else {
+			baseMessage = `Unable to perform action${tag ? ` with tag '${tag}'` : 'on tag'}`;
+		}
+
+		switch (reason) {
+			case TagErrorReason.TagAlreadyExists:
+				return `${baseMessage} because it already exists`;
+			case TagErrorReason.TagNotFound:
+				return `${baseMessage} because it does not exist`;
+			case TagErrorReason.InvalidTagName:
+				return `${baseMessage} because the tag name is invalid`;
+			case TagErrorReason.PermissionDenied:
+				return `${baseMessage} because you don't have permission to push to this remote repository.`;
+			case TagErrorReason.RemoteRejected:
+				return `${baseMessage} because the remote repository rejected the push.`;
+			default:
+				return baseMessage;
+		}
+	}
+
+	constructor(reason?: TagErrorReason, original?: Error, tag?: string, action?: string);
+	constructor(message?: string, original?: Error);
+	constructor(messageOrReason: string | TagErrorReason | undefined, original?: Error, tag?: string, action?: string) {
+		let reason: TagErrorReason | undefined;
+		if (typeof messageOrReason !== 'string') {
+			reason = messageOrReason as TagErrorReason;
+		} else {
+			super(messageOrReason);
+		}
+		const message =
+			typeof messageOrReason === 'string'
+				? messageOrReason
+				: TagError.buildTagErrorMessage(messageOrReason as TagErrorReason, tag, action);
+		super(message);
+
+		this.original = original;
+		this.reason = reason;
+		this.tag = tag;
+		this.action = action;
+		Error.captureStackTrace?.(this, TagError);
+	}
+
+	WithTag(tag: string) {
+		this.tag = tag;
+		this.message = TagError.buildTagErrorMessage(this.reason, tag, this.action);
+		return this;
+	}
+
+	WithAction(action: string) {
+		this.action = action;
+		this.message = TagError.buildTagErrorMessage(this.reason, this.tag, action);
+		return this;
+	}
+}

src/git/gitProvider.ts

@@ -120,7 +120,8 @@ export interface BranchContributorOverview {
 export interface GitProviderRepository {
 	createBranch?(repoPath: string, name: string, ref: string): Promise<void>;
 	renameBranch?(repoPath: string, oldName: string, newName: string): Promise<void>;
-
+	createTag?(repoPath: string, name: string, ref: string, message?: string): Promise<void>;
+	deleteTag?(repoPath: string, name: string): Promise<void>;
 	addRemote?(repoPath: string, name: string, url: string, options?: { fetch?: boolean }): Promise<void>;
 	pruneRemote?(repoPath: string, name: string): Promise<void>;
 	removeRemote?(repoPath: string, name: string): Promise<void>;

src/git/gitProviderService.ts

@@ -1377,6 +1377,22 @@ export class GitProviderService implements Disposable {
 		return provider.renameBranch(path, oldName, newName);
 	}
 
+	@log()
+	createTag(repoPath: string | Uri, name: string, ref: string, message?: string): Promise<void> {
+		const { provider, path } = this.getProvider(repoPath);
+		if (provider.createTag == null) throw new ProviderNotSupportedError(provider.descriptor.name);
+
+		return provider.createTag(path, name, ref, message);
+	}
+
+	@log()
+	deleteTag(repoPath: string | Uri, name: string): Promise<void> {
+		const { provider, path } = this.getProvider(repoPath);
+		if (provider.deleteTag == null) throw new ProviderNotSupportedError(provider.descriptor.name);
+
+		return provider.deleteTag(path, name);
+	}
+
 	@log()
 	checkout(
 		repoPath: string | Uri,

src/git/models/repository.ts

@@ -27,7 +27,7 @@ import type { GitProviderDescriptor, GitProviderRepository } from '../gitProvide
 import type { GitProviderService } from '../gitProviderService';
 import type { GitBranch } from './branch';
 import { getBranchNameWithoutRemote, getRemoteNameFromBranchName } from './branch';
-import type { GitBranchReference, GitReference, GitTagReference } from './reference';
+import type { GitBranchReference, GitReference } from './reference';
 import { getNameWithoutRemote, isBranchReference } from './reference';
 import type { GitRemote } from './remote';
 import type { GitWorktree } from './worktree';
@@ -992,21 +992,6 @@ export class Repository implements Disposable {
 		this._suspended = true;
 	}
 
-	@log()
-	tag(...args: string[]) {
-		void this.runTerminalCommand('tag', ...args);
-	}
-
-	@log()
-	tagDelete(tags: GitTagReference | GitTagReference[]) {
-		if (!Array.isArray(tags)) {
-			tags = [tags];
-		}
-
-		const args = ['--delete'];
-		void this.runTerminalCommand('tag', ...args, ...tags.map(t => t.ref));
-	}
-
 	private _fsWatcherDisposable: Disposable | undefined;
 	private _fsWatchers = new Map<string, number>();
 	private _fsChangeDelay: number = defaultFileSystemChangeDelay;