From 5f949a92ca35f963f97e0498c0d7713d0cdd645f Mon Sep 17 00:00:00 2001 From: Keith Daulton Date: Wed, 5 Apr 2023 16:20:44 -0400 Subject: [PATCH] Fixes stashing when staged is not supported Co-authored-by: Eric Amodio --- CHANGELOG.md | 1 + src/commands/git/stash.ts | 31 ++++++++++++++++-- src/commands/stashSave.ts | 62 ++++++++++++++++++++++++------------ src/env/node/git/git.ts | 20 ++++++++++-- src/env/node/git/localGitProvider.ts | 4 +++ src/features.ts | 1 + src/git/actions/stash.ts | 2 ++ src/git/errors.ts | 40 +++++++++++++++++++++++ src/plus/github/githubGitProvider.ts | 1 + 9 files changed, 137 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7fc2ef..b987ce4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p - Fixes [#2582](https://github.com/gitkraken/vscode-gitlens/issues/2582) - _Visual File History_ background color when in a panel - Fixes [#2609](https://github.com/gitkraken/vscode-gitlens/issues/2609) - If you check out a branch that is hidden, GitLens should show the branch still - Fixes tooltips sometimes failing to show in _Commit Graph_ rows when the Date column is hidden +- Fixes [#2595](https://github.com/gitkraken/vscode-gitlens/issues/2595) - Error when stashing changes ## [13.4.0] - 2023-03-16 diff --git a/src/commands/git/stash.ts b/src/commands/git/stash.ts index 96d5f7f..fd56e62 100644 --- a/src/commands/git/stash.ts +++ b/src/commands/git/stash.ts @@ -4,7 +4,7 @@ import { GlyphChars } from '../../constants'; import type { Container } from '../../container'; import { getContext } from '../../context'; import { reveal, showDetailsView } from '../../git/actions/stash'; -import { StashApplyError, StashApplyErrorReason } from '../../git/errors'; +import { StashApplyError, StashApplyErrorReason, StashPushError, StashPushErrorReason } from '../../git/errors'; import type { GitStashCommit } from '../../git/models/commit'; import type { GitStashReference } from '../../git/models/reference'; import { getReferenceLabel } from '../../git/models/reference'; @@ -82,6 +82,7 @@ interface PushState { repo: string | Repository; message?: string; uris?: Uri[]; + onlyStagedUris?: Uri[]; flags: PushFlags[]; } @@ -514,16 +515,42 @@ export class StashGitCommand extends QuickCommand { state.flags = result; } - endSteps(state); try { await state.repo.stashSave(state.message, state.uris, { includeUntracked: state.flags.includes('--include-untracked'), keepIndex: state.flags.includes('--keep-index'), onlyStaged: state.flags.includes('--staged'), }); + + endSteps(state); } catch (ex) { Logger.error(ex, context.title); + if ( + ex instanceof StashPushError && + ex.reason === StashPushErrorReason.ConflictingStagedAndUnstagedLines && + state.flags.includes('--staged') + ) { + const confirm = { title: 'Yes' }; + const cancel = { title: 'No', isCloseAffordance: true }; + const result = await window.showErrorMessage( + ex.message, + { + modal: true, + }, + confirm, + cancel, + ); + + if (result === confirm) { + state.uris = state.onlyStagedUris; + state.flags.splice(state.flags.indexOf('--staged'), 1); + continue; + } + + return; + } + const msg: string = ex?.message ?? ex?.toString() ?? ''; if (msg.includes('newer version of Git')) { void window.showErrorMessage(`Unable to stash changes. ${msg}`); diff --git a/src/commands/stashSave.ts b/src/commands/stashSave.ts index 2736662..e7d556c 100644 --- a/src/commands/stashSave.ts +++ b/src/commands/stashSave.ts @@ -3,6 +3,7 @@ import type { ScmResource } from '../@types/vscode.git.resources'; import { ScmResourceGroupType } from '../@types/vscode.git.resources.enums'; import { Commands } from '../constants'; import type { Container } from '../container'; +import { Features } from '../features'; import { push } from '../git/actions/stash'; import { GitUri } from '../git/gitUri'; import { command } from '../system/command'; @@ -20,6 +21,7 @@ export interface StashSaveCommandArgs { uris?: Uri[]; keepStaged?: boolean; onlyStaged?: boolean; + onlyStagedUris?: Uri[]; } @command() @@ -44,33 +46,44 @@ export class StashSaveCommand extends Command { args.uris = context.scmResourceStates.map(s => s.resourceUri); args.repoPath = (await this.container.git.getOrOpenRepository(args.uris[0]))?.path; - const status = await this.container.git.getStatusForRepo(args.repoPath); - if (status?.computeWorkingTreeStatus().staged) { - if ( - !context.scmResourceStates.some( - s => (s as ScmResource).resourceGroupType === ScmResourceGroupType.Index, - ) - ) { - args.keepStaged = true; - } + if ( + !context.scmResourceStates.some( + s => (s as ScmResource).resourceGroupType === ScmResourceGroupType.Index, + ) + ) { + args.keepStaged = true; } } else if (context.type === 'scm-groups') { args = { ...args }; - if (context.scmResourceGroups.every(g => g.id === 'index')) { + let isStagedOnly = true; + let hasStaged = false; + const uris = context.scmResourceGroups.reduce((a, b) => { + const isStaged = b.id === 'index'; + if (isStagedOnly && !isStaged) { + isStagedOnly = false; + } + if (isStaged) { + hasStaged = true; + } + return a.concat(b.resourceStates.map(s => s.resourceUri)); + }, []); + + const repo = await this.container.git.getOrOpenRepository(uris[0]); + let canUseStagedOnly = false; + if (isStagedOnly && repo != null) { + canUseStagedOnly = await repo.supports(Features.StashOnlyStaged); + } + + if (canUseStagedOnly) { args.onlyStaged = true; + args.onlyStagedUris = uris; } else { - args.uris = context.scmResourceGroups.reduce( - (a, b) => a.concat(b.resourceStates.map(s => s.resourceUri)), - [], - ); - args.repoPath = (await this.container.git.getOrOpenRepository(args.uris[0]))?.path; + args.uris = uris; + args.repoPath = repo?.path; - const status = await this.container.git.getStatusForRepo(args.repoPath); - if (status?.computeWorkingTreeStatus().staged) { - if (!context.scmResourceGroups.some(g => g.id === 'index')) { - args.keepStaged = true; - } + if (!hasStaged) { + args.keepStaged = true; } } } @@ -79,6 +92,13 @@ export class StashSaveCommand extends Command { } execute(args?: StashSaveCommandArgs) { - return push(args?.repoPath, args?.uris, args?.message, args?.keepStaged, args?.onlyStaged); + return push( + args?.repoPath, + args?.uris, + args?.message, + args?.keepStaged, + args?.onlyStaged, + args?.onlyStagedUris, + ); } } diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index cfbe34f..76a258d 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -8,6 +8,7 @@ import { hrtime } from '@env/hrtime'; import { GlyphChars } from '../../../constants'; import type { GitCommandOptions, GitSpawnOptions } from '../../../git/commandOptions'; import { GitErrorHandling } from '../../../git/commandOptions'; +import { StashPushError, StashPushErrorReason } from '../../../git/errors'; import type { GitDiffFilter } from '../../../git/models/diff'; import { isUncommitted, isUncommittedStaged, shortenRevision } from '../../../git/models/reference'; import type { GitUser } from '../../../git/models/user'; @@ -1859,7 +1860,11 @@ export class Git { } if (onlyStaged) { - params.push('--staged'); + if (await this.isAtLeastVersion('2.35')) { + params.push('--staged'); + } else { + throw new Error('Git version 2.35 or higher is required for --staged'); + } } if (message) { @@ -1882,7 +1887,18 @@ export class Git { params.push(...pathspecs); } - void (await this.git({ cwd: repoPath }, ...params)); + try { + void (await this.git({ cwd: repoPath }, ...params)); + } catch (ex) { + if ( + ex instanceof RunError && + ex.stdout.includes('Saved working directory and index state') && + ex.stderr.includes('Cannot remove worktree changes') + ) { + throw new StashPushError(StashPushErrorReason.ConflictingStagedAndUnstagedLines); + } + throw ex; + } } async status( diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 164b27f..6b389d3 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -502,6 +502,10 @@ export class LocalGitProvider implements GitProvider, Disposable { supported = await this.git.isAtLeastVersion('2.17.0'); this._supportedFeatures.set(feature, supported); return supported; + case Features.StashOnlyStaged: + supported = await this.git.isAtLeastVersion('2.35.0'); + this._supportedFeatures.set(feature, supported); + return supported; default: return true; } diff --git a/src/features.ts b/src/features.ts index ac01f0f..e75721c 100644 --- a/src/features.ts +++ b/src/features.ts @@ -5,6 +5,7 @@ export const enum Features { Stashes = 'stashes', Timeline = 'timeline', Worktrees = 'worktrees', + StashOnlyStaged = 'stashOnlyStaged', } export type FeatureAccess = diff --git a/src/git/actions/stash.ts b/src/git/actions/stash.ts index cd1f351..c11ffc6 100644 --- a/src/git/actions/stash.ts +++ b/src/git/actions/stash.ts @@ -33,6 +33,7 @@ export function push( message?: string, keepStaged: boolean = false, onlyStaged: boolean = false, + onlyStagedUris?: Uri[], ) { return executeGitCommand({ command: 'stash', @@ -40,6 +41,7 @@ export function push( subcommand: 'push', repo: repo, uris: uris, + onlyStagedUris: onlyStagedUris, message: message, flags: [...(keepStaged ? ['--keep-index'] : []), ...(onlyStaged ? ['--staged'] : [])] as PushFlags[], }, diff --git a/src/git/errors.ts b/src/git/errors.ts index ec4919f..a3b8149 100644 --- a/src/git/errors.ts +++ b/src/git/errors.ts @@ -41,6 +41,46 @@ export class StashApplyError extends Error { } } +export const enum StashPushErrorReason { + ConflictingStagedAndUnstagedLines = 1, +} + +export class StashPushError extends Error { + static is(ex: any, reason?: StashPushErrorReason): ex is StashPushError { + return ex instanceof StashPushError && (reason == null || ex.reason === reason); + } + + readonly original?: Error; + readonly reason: StashPushErrorReason | undefined; + + constructor(reason?: StashPushErrorReason, original?: Error); + constructor(message?: string, original?: Error); + constructor(messageOrReason: string | StashPushErrorReason | undefined, original?: Error) { + let message; + let reason: StashPushErrorReason | undefined; + if (messageOrReason == null) { + message = 'Unable to stash'; + } else if (typeof messageOrReason === 'string') { + message = messageOrReason; + reason = undefined; + } else { + reason = messageOrReason; + switch (reason) { + case StashPushErrorReason.ConflictingStagedAndUnstagedLines: + message = + 'Stash was created, but the working tree cannot be updated because at least one file has staged and unstaged changes on the same line(s).\n\nDo you want to try again by stashing both your staged and unstaged changes?'; + break; + default: + message = 'Unable to stash'; + } + } + super(message); + + this.original = original; + this.reason = reason; + Error.captureStackTrace?.(this, StashApplyError); + } +} export const enum WorktreeCreateErrorReason { AlreadyCheckedOut = 1, AlreadyExists = 2, diff --git a/src/plus/github/githubGitProvider.ts b/src/plus/github/githubGitProvider.ts index 9512a53..b545eda 100644 --- a/src/plus/github/githubGitProvider.ts +++ b/src/plus/github/githubGitProvider.ts @@ -226,6 +226,7 @@ export class GitHubGitProvider implements GitProvider, Disposable { switch (feature) { case Features.Stashes: case Features.Worktrees: + case Features.StashOnlyStaged: return false; default: return true;