Skip to content

Commit f572399

Browse files
eamodioeamodio
committed
Continues Git command error handling improvements
 - Converts stash commands to use standard error handling - Better messaging for many other commands
1 parent 9f93a53 commit f572399

File tree

13 files changed

+157
-162
lines changed

13 files changed

+157
-162
lines changed

‎src/commands/git/branch.ts‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
isBranchReference,
1515
isRevisionReference,
1616
} from '../../git/utils/reference.utils';
17-
import { showGenericErrorMessage, showGitErrorMessage } from '../../messages';
17+
import { showGitErrorMessage } from '../../messages';
1818
import { getIssueOwner } from '../../plus/integrations/providers/utils';
1919
import type { QuickPickItemOfT } from '../../quickpicks/items/common';
2020
import { createQuickPickSeparator } from '../../quickpicks/items/common';
@@ -495,11 +495,7 @@ export class BranchGitCommand extends QuickCommand {
495495
return;
496496
}
497497

498-
if (BranchError.is(ex)) {
499-
void showGitErrorMessage(ex);
500-
} else {
501-
void showGitErrorMessage(ex, 'Unable to create branch');
502-
}
498+
void showGitErrorMessage(ex, BranchError.is(ex) ? undefined : 'Unable to create branch');
503499
return;
504500
}
505501
}
@@ -669,15 +665,18 @@ export class BranchGitCommand extends QuickCommand {
669665
await state.repo.git.branches.deleteLocalBranch?.(name, { force: true });
670666
} catch (ex) {
671667
Logger.error(ex, context.title);
672-
void showGitErrorMessage(ex);
668+
void showGitErrorMessage(
669+
ex,
670+
BranchError.is(ex) ? undefined : 'Unable to force delete branch',
671+
);
673672
}
674673
}
675674

676675
continue;
677676
}
678677

679678
Logger.error(ex, context.title);
680-
void showGitErrorMessage(ex);
679+
void showGitErrorMessage(ex, BranchError.is(ex) ? undefined : 'Unable to delete branch');
681680
}
682681
}
683682
}
@@ -786,8 +785,7 @@ export class BranchGitCommand extends QuickCommand {
786785
await state.repo.git.branches.renameBranch?.(state.reference.ref, state.name);
787786
} catch (ex) {
788787
Logger.error(ex, context.title);
789-
// TODO likely need some better error handling here
790-
return showGenericErrorMessage('Unable to rename branch');
788+
void showGitErrorMessage(ex, BranchError.is(ex) ? undefined : 'Unable to rename branch');
791789
}
792790
}
793791
}
@@ -852,7 +850,7 @@ export class BranchGitCommand extends QuickCommand {
852850
);
853851
} catch (ex) {
854852
Logger.error(ex, context.title);
855-
void showGenericErrorMessage('Unable to manage upstream tracking');
853+
void showGitErrorMessage(ex, BranchError.is(ex) ? undefined : 'Unable to manage upstream tracking');
856854
}
857855
}
858856
}

‎src/commands/git/cherry-pick.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,7 @@ export class CherryPickGitCommand extends QuickCommand<State> {
160160
return;
161161
}
162162

163-
if (CherryPickError.is(ex)) {
164-
void showGitErrorMessage(ex);
165-
} else {
166-
void showGitErrorMessage(ex, 'Unable to cherry-pick');
167-
}
163+
void showGitErrorMessage(ex, CherryPickError.is(ex) ? undefined : 'Unable to cherry-pick');
168164
}
169165
}
170166

‎src/commands/git/merge.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,7 @@ export class MergeGitCommand extends QuickCommand<State> {
133133
return;
134134
}
135135

136-
if (MergeError.is(ex)) {
137-
void showGitErrorMessage(ex);
138-
} else {
139-
void showGitErrorMessage(ex, 'Unable to merge');
140-
}
136+
void showGitErrorMessage(ex, MergeError.is(ex) ? undefined : 'Unable to merge');
141137
}
142138
}
143139

‎src/commands/git/rebase.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,7 @@ export class RebaseGitCommand extends QuickCommand<State> {
125125
return;
126126
}
127127

128-
if (RebaseError.is(ex)) {
129-
void showGitErrorMessage(ex);
130-
} else {
131-
void showGitErrorMessage(ex, 'Unable to rebase');
132-
}
128+
void showGitErrorMessage(ex, RebaseError.is(ex) ? undefined : 'Unable to rebase');
133129
}
134130
}
135131

‎src/commands/git/revert.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,7 @@ export class RevertGitCommand extends QuickCommand<State> {
120120
return;
121121
}
122122

123-
if (RevertError.is(ex)) {
124-
void showGitErrorMessage(ex);
125-
} else {
126-
void showGitErrorMessage(ex, 'Unable to revert');
127-
}
123+
void showGitErrorMessage(ex, RevertError.is(ex) ? undefined : 'Unable to revert');
128124
}
129125
}
130126

‎src/commands/git/stash.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ export class StashGitCommand extends QuickCommand<State> {
367367
'Unable to apply stash. Your local changes would be overwritten. Please commit or stash your changes before trying again.',
368368
);
369369
} else {
370-
void showGitErrorMessage(ex);
370+
void showGitErrorMessage(ex, StashApplyError.is(ex) ? undefined : 'Unable to apply stash');
371371
}
372372
}
373373
}
@@ -594,7 +594,7 @@ export class StashGitCommand extends QuickCommand<State> {
594594
return;
595595
}
596596

597-
void showGitErrorMessage(ex, 'Unable to stash changes');
597+
void showGitErrorMessage(ex, StashPushError.is(ex) ? undefined : 'Unable to stash changes');
598598

599599
return;
600600
}
@@ -821,7 +821,7 @@ export class StashGitCommand extends QuickCommand<State> {
821821
);
822822
} catch (ex) {
823823
Logger.error(ex, context.title);
824-
void showGitErrorMessage(ex);
824+
void showGitErrorMessage(ex, 'Unable to rename stash');
825825
}
826826
}
827827
}

‎src/commands/git/switch.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,13 @@ export class SwitchGitCommand extends QuickCommand<State> {
117117
}
118118

119119
Logger.error(ex, this.title);
120-
void showGitErrorMessage(ex);
120+
void showGitErrorMessage(
121+
ex,
122+
`Unable to fast-forward ${getReferenceLabel(state.reference, {
123+
icon: false,
124+
label: true,
125+
})}`,
126+
);
121127
}
122128
}
123129
}
@@ -232,7 +238,13 @@ export class SwitchGitCommand extends QuickCommand<State> {
232238
Logger.log(ex.message, this.title);
233239
} else {
234240
Logger.error(ex, this.title);
235-
void showGitErrorMessage(ex);
241+
void showGitErrorMessage(
242+
ex,
243+
`Unable to fast-forward ${getReferenceLabel(state.reference, {
244+
icon: false,
245+
label: true,
246+
})}`,
247+
);
236248
}
237249
}
238250
}

‎src/commands/git/tag.ts‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { QuickInputButtons } from 'vscode';
1+
import { QuickInputButtons, window } from 'vscode';
22
import type { Container } from '../../container';
3+
import { TagError } from '../../git/errors';
34
import type { GitReference, GitTagReference } from '../../git/models/reference';
45
import type { Repository } from '../../git/models/repository';
56
import {
@@ -299,7 +300,20 @@ export class TagGitCommand extends QuickCommand<State> {
299300
await state.repo.git.tags.createTag?.(state.name, state.reference.ref, state.message);
300301
} catch (ex) {
301302
Logger.error(ex, context.title);
302-
void showGitErrorMessage(ex);
303+
304+
if (TagError.is(ex, 'alreadyExists')) {
305+
void window.showWarningMessage(
306+
`Unable to create tag '${state.name}'. A tag with that name already exists.`,
307+
);
308+
return;
309+
}
310+
311+
if (TagError.is(ex, 'invalidName')) {
312+
void window.showWarningMessage(`Unable to create tag '${state.name}'. The tag name is invalid.`);
313+
return;
314+
}
315+
316+
void showGitErrorMessage(ex, TagError.is(ex) ? undefined : 'Unable to create tag');
303317
}
304318
}
305319
}
@@ -389,7 +403,7 @@ export class TagGitCommand extends QuickCommand<State> {
389403
await state.repo.git.tags.deleteTag?.(ref);
390404
} catch (ex) {
391405
Logger.error(ex, context.title);
392-
void showGitErrorMessage(ex);
406+
void showGitErrorMessage(ex, TagError.is(ex) ? undefined : 'Unable to delete tag');
393407
}
394408
}
395409
}

‎src/env/node/git/git.ts‎

Lines changed: 22 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,19 @@ import type {
2525
RebaseErrorReason,
2626
ResetErrorReason,
2727
RevertErrorReason,
28+
StashApplyErrorReason,
29+
StashPushErrorReason,
2830
TagErrorReason,
2931
} from '../../../git/errors';
3032
import {
3133
BlameIgnoreRevsFileBadRevisionError,
3234
BlameIgnoreRevsFileError,
33-
BranchError,
3435
CheckoutError,
3536
FetchError,
3637
PullError,
3738
PushError,
3839
ResetError,
3940
StashPushError,
40-
TagError,
4141
WorkspaceUntrustedError,
4242
} from '../../../git/errors';
4343
import type { GitDir } from '../../../git/gitProvider';
@@ -123,6 +123,9 @@ export const GitErrors = {
123123
remoteAhead: /rejected because the remote contains work/i,
124124
remoteConnectionFailed: /Could not read from remote repository/i,
125125
remoteRejected: /rejected because the remote contains work/i,
126+
stashConflictingStagedAndUnstagedLines: /Cannot remove worktree changes/i,
127+
stashNothingToSave: /No local changes to save/i,
128+
stashSavedWorkingDirAndIndexState: /Saved working directory and index state/i,
126129
tagAlreadyExists: /tag .* already exists/i,
127130
tagConflict: /! \[rejected\].*\(would clobber existing tag\)/m,
128131
tagNotFound: /tag .* not found/i,
@@ -751,21 +754,6 @@ export class Git implements Disposable {
751754
}
752755
}
753756

754-
async branch(repoPath: string, ...args: string[]): Promise<GitResult<string>> {
755-
const params = ['branch', ...args];
756-
try {
757-
const result = await this.exec({ cwd: repoPath }, ...params);
758-
return result;
759-
} catch (ex) {
760-
throw getGitCommandError(
761-
'branch',
762-
ex,
763-
reason =>
764-
new BranchError(reason ?? 'other', ex, undefined, undefined, { repoPath: repoPath, args: params }),
765-
);
766-
}
767-
}
768-
769757
async branchOrTag__containsOrPointsAt(
770758
repoPath: string,
771759
refs: string[],
@@ -1535,25 +1523,11 @@ export class Git implements Disposable {
15351523
}
15361524

15371525
try {
1538-
const result = await this.exec({ cwd: repoPath, stdin: stdin }, ...params);
1539-
if (result.stdout.includes('No local changes to save')) {
1540-
throw new StashPushError('nothingToSave', undefined, {
1541-
repoPath: repoPath,
1542-
args: params,
1543-
});
1544-
}
1526+
void (await this.exec({ cwd: repoPath, stdin: stdin }, ...params));
15451527
} catch (ex) {
1546-
if (
1547-
ex instanceof GitError &&
1548-
ex.stdout?.includes('Saved working directory and index state') &&
1549-
ex.stderr?.includes('Cannot remove worktree changes')
1550-
) {
1551-
throw new StashPushError('conflictingStagedAndUnstagedLines', undefined, {
1552-
repoPath: repoPath,
1553-
args: params,
1554-
});
1555-
}
1556-
throw ex;
1528+
throw getGitCommandError('stash-push', ex, reason => {
1529+
return new StashPushError(reason ?? 'other', ex, { repoPath: repoPath, args: params });
1530+
});
15571531
}
15581532
}
15591533

@@ -1590,21 +1564,6 @@ export class Git implements Disposable {
15901564
return result;
15911565
}
15921566

1593-
async tag(repoPath: string, ...args: string[]): Promise<GitResult<string>> {
1594-
const params = ['tag', ...args];
1595-
try {
1596-
const result = await this.exec({ cwd: repoPath }, ...params);
1597-
return result;
1598-
} catch (ex) {
1599-
throw getGitCommandError(
1600-
'tag',
1601-
ex,
1602-
reason =>
1603-
new TagError(reason ?? 'other', ex, undefined, undefined, { repoPath: repoPath, args: params }),
1604-
);
1605-
}
1606-
}
1607-
16081567
async readDotGitFile(
16091568
gitDir: GitDir,
16101569
pathParts: string[],
@@ -1704,6 +1663,8 @@ type GitCommand =
17041663
| 'rebase'
17051664
| 'reset'
17061665
| 'revert'
1666+
| 'stash-apply'
1667+
| 'stash-push'
17071668
| 'tag';
17081669
type GitCommandToReasonMap = {
17091670
branch: BranchErrorReason;
@@ -1716,6 +1677,8 @@ type GitCommandToReasonMap = {
17161677
rebase: RebaseErrorReason;
17171678
reset: ResetErrorReason;
17181679
revert: RevertErrorReason;
1680+
'stash-apply': StashApplyErrorReason;
1681+
'stash-push': StashPushErrorReason;
17191682
tag: TagErrorReason;
17201683
};
17211684

@@ -1825,6 +1788,15 @@ const errorToReasonMap = new Map<GitCommand, [RegExp, GitCommandToReasonMap[GitC
18251788
[GitErrors.changesWouldBeOverwritten, 'wouldOverwriteChanges'],
18261789
],
18271790
],
1791+
['stash-apply', [[GitErrors.changesWouldBeOverwritten, 'uncommittedChanges']]],
1792+
[
1793+
'stash-push',
1794+
[
1795+
[GitErrors.stashConflictingStagedAndUnstagedLines, 'conflictingStagedAndUnstagedLines'],
1796+
[GitErrors.stashNothingToSave, 'nothingToSave'],
1797+
[GitErrors.stashSavedWorkingDirAndIndexState, 'conflictingStagedAndUnstagedLines'],
1798+
],
1799+
],
18281800
[
18291801
'tag',
18301802
[

0 commit comments

Comments
 (0)