Skip to content

Commit c9e7711

Browse files
Copilotrebornix
Copilot
and
rebornix
committed
Implement fix for spurious error when checking out PR with untracked files
Co-authored-by: rebornix <876920+rebornix@users.noreply.github.com>
1 parent 2e9b609 commit c9e7711

2 files changed

Lines changed: 162 additions & 9 deletions

File tree

‎src/commands.ts‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import * as pathLib from 'path';
88
import * as vscode from 'vscode';
99
import { Repository } from './api/api';
10-
import { GitErrorCodes } from './api/api1';
10+
import { GitErrorCodes, Status } from './api/api1';
1111
import { CommentReply, findActiveHandler, resolveCommentHandler } from './commentHandlerResolver';
1212
import { COPILOT_LOGINS } from './common/copilot';
1313
import { commands } from './common/executeCommands';
@@ -57,11 +57,13 @@ const DISCARD_CHANGES = vscode.l10n.t('Discard changes');
5757
* @returns Promise<boolean> true if user chose to proceed (after staging/discarding), false if cancelled
5858
*/
5959
async function handleUncommittedChanges(repository: Repository): Promise<boolean> {
60-
const hasWorkingTreeChanges = repository.state.workingTreeChanges.length > 0;
60+
// Filter out untracked files as they typically don't conflict with PR checkout
61+
const trackedWorkingTreeChanges = repository.state.workingTreeChanges.filter(change => change.status !== Status.UNTRACKED);
62+
const hasTrackedWorkingTreeChanges = trackedWorkingTreeChanges.length > 0;
6163
const hasIndexChanges = repository.state.indexChanges.length > 0;
6264

63-
if (!hasWorkingTreeChanges && !hasIndexChanges) {
64-
return true; // No uncommitted changes, proceed
65+
if (!hasTrackedWorkingTreeChanges && !hasIndexChanges) {
66+
return true; // No tracked uncommitted changes, proceed
6567
}
6668

6769
const modalResult = await vscode.window.showInformationMessage(
@@ -82,18 +84,18 @@ async function handleUncommittedChanges(repository: Repository): Promise<boolean
8284
if (modalResult === STASH_CHANGES) {
8385
// Stash all changes (working tree changes + any unstaged changes)
8486
const allChangedFiles = [
85-
...repository.state.workingTreeChanges.map(change => change.uri.fsPath),
87+
...trackedWorkingTreeChanges.map(change => change.uri.fsPath),
8688
...repository.state.indexChanges.map(change => change.uri.fsPath),
8789
];
8890
if (allChangedFiles.length > 0) {
8991
await repository.add(allChangedFiles);
9092
await vscode.commands.executeCommand('git.stash', repository);
9193
}
9294
} else if (modalResult === DISCARD_CHANGES) {
93-
// Discard all working tree changes
94-
const workingTreeFiles = repository.state.workingTreeChanges.map(change => change.uri.fsPath);
95-
if (workingTreeFiles.length > 0) {
96-
await repository.clean(workingTreeFiles);
95+
// Discard all tracked working tree changes
96+
const trackedWorkingTreeFiles = trackedWorkingTreeChanges.map(change => change.uri.fsPath);
97+
if (trackedWorkingTreeFiles.length > 0) {
98+
await repository.clean(trackedWorkingTreeFiles);
9799
}
98100
}
99101
return true; // Successfully handled changes, proceed with checkout

‎src/test/commands/handleUncommittedChanges.test.ts‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import { default as assert } from 'assert';
7+
import { URI } from 'vscode-uri';
8+
import { MockRepository } from '../mocks/mockRepository';
9+
import { Status } from '../../api/api1';
10+
11+
// Since handleUncommittedChanges is not exported, we'll test it indirectly through the PR checkout command
12+
// For now, let's create a test to ensure the behavior works as expected
13+
describe('handleUncommittedChanges', function () {
14+
let repository: MockRepository;
15+
16+
beforeEach(function () {
17+
repository = new MockRepository();
18+
});
19+
20+
it('should return true when there are no changes', async function () {
21+
// Setup: no changes in the repository
22+
(repository as any)._state.workingTreeChanges = [];
23+
(repository as any)._state.indexChanges = [];
24+
25+
// The function should return true (proceed with checkout)
26+
// Since we can't test the function directly, we'll validate the logic
27+
const hasTrackedWorkingTreeChanges = repository.state.workingTreeChanges.filter(change => change.status !== Status.UNTRACKED).length > 0;
28+
const hasIndexChanges = repository.state.indexChanges.length > 0;
29+
30+
assert.strictEqual(hasTrackedWorkingTreeChanges, false);
31+
assert.strictEqual(hasIndexChanges, false);
32+
});
33+
34+
it('should return true when there are only untracked files', async function () {
35+
// Setup: only untracked files
36+
(repository as any)._state.workingTreeChanges = [
37+
{
38+
uri: URI.file('/root/untracked1.txt'),
39+
originalUri: URI.file('/root/untracked1.txt'),
40+
renameUri: undefined,
41+
status: Status.UNTRACKED,
42+
},
43+
{
44+
uri: URI.file('/root/untracked2.txt'),
45+
originalUri: URI.file('/root/untracked2.txt'),
46+
renameUri: undefined,
47+
status: Status.UNTRACKED,
48+
},
49+
];
50+
(repository as any)._state.indexChanges = [];
51+
52+
// The function should return true (proceed with checkout) for untracked files
53+
const hasTrackedWorkingTreeChanges = repository.state.workingTreeChanges.filter(change => change.status !== Status.UNTRACKED).length > 0;
54+
const hasIndexChanges = repository.state.indexChanges.length > 0;
55+
56+
assert.strictEqual(hasTrackedWorkingTreeChanges, false);
57+
assert.strictEqual(hasIndexChanges, false);
58+
});
59+
60+
it('should show warning when there are tracked modified files', async function () {
61+
// Setup: tracked modified files
62+
(repository as any)._state.workingTreeChanges = [
63+
{
64+
uri: URI.file('/root/modified.txt'),
65+
originalUri: URI.file('/root/modified.txt'),
66+
renameUri: undefined,
67+
status: Status.MODIFIED,
68+
},
69+
];
70+
(repository as any)._state.indexChanges = [];
71+
72+
// The function should show a warning dialog for tracked modified files
73+
const hasTrackedWorkingTreeChanges = repository.state.workingTreeChanges.filter(change => change.status !== Status.UNTRACKED).length > 0;
74+
const hasIndexChanges = repository.state.indexChanges.length > 0;
75+
76+
assert.strictEqual(hasTrackedWorkingTreeChanges, true);
77+
assert.strictEqual(hasIndexChanges, false);
78+
});
79+
80+
it('should show warning when there are index changes', async function () {
81+
// Setup: index changes
82+
(repository as any)._state.workingTreeChanges = [];
83+
(repository as any)._state.indexChanges = [
84+
{
85+
uri: URI.file('/root/staged.txt'),
86+
originalUri: URI.file('/root/staged.txt'),
87+
renameUri: undefined,
88+
status: Status.INDEX_MODIFIED,
89+
},
90+
];
91+
92+
// The function should show a warning dialog for index changes
93+
const hasTrackedWorkingTreeChanges = repository.state.workingTreeChanges.filter(change => change.status !== Status.UNTRACKED).length > 0;
94+
const hasIndexChanges = repository.state.indexChanges.length > 0;
95+
96+
assert.strictEqual(hasTrackedWorkingTreeChanges, false);
97+
assert.strictEqual(hasIndexChanges, true);
98+
});
99+
100+
it('should show warning when there are mixed tracked and untracked files', async function () {
101+
// Setup: mixed tracked and untracked files
102+
(repository as any)._state.workingTreeChanges = [
103+
{
104+
uri: URI.file('/root/untracked.txt'),
105+
originalUri: URI.file('/root/untracked.txt'),
106+
renameUri: undefined,
107+
status: Status.UNTRACKED,
108+
},
109+
{
110+
uri: URI.file('/root/modified.txt'),
111+
originalUri: URI.file('/root/modified.txt'),
112+
renameUri: undefined,
113+
status: Status.MODIFIED,
114+
},
115+
];
116+
(repository as any)._state.indexChanges = [];
117+
118+
// The function should show a warning dialog because there are tracked changes
119+
const hasTrackedWorkingTreeChanges = repository.state.workingTreeChanges.filter(change => change.status !== Status.UNTRACKED).length > 0;
120+
const hasIndexChanges = repository.state.indexChanges.length > 0;
121+
122+
assert.strictEqual(hasTrackedWorkingTreeChanges, true);
123+
assert.strictEqual(hasIndexChanges, false);
124+
});
125+
126+
it('should only stash/discard tracked files', async function () {
127+
// Setup: mixed tracked and untracked files
128+
(repository as any)._state.workingTreeChanges = [
129+
{
130+
uri: URI.file('/root/untracked.txt'),
131+
originalUri: URI.file('/root/untracked.txt'),
132+
renameUri: undefined,
133+
status: Status.UNTRACKED,
134+
},
135+
{
136+
uri: URI.file('/root/modified.txt'),
137+
originalUri: URI.file('/root/modified.txt'),
138+
renameUri: undefined,
139+
status: Status.MODIFIED,
140+
},
141+
];
142+
(repository as any)._state.indexChanges = [];
143+
144+
// Verify that only tracked files are considered for stashing/discarding
145+
const trackedWorkingTreeChanges = repository.state.workingTreeChanges.filter(change => change.status !== Status.UNTRACKED);
146+
const trackedWorkingTreeFiles = trackedWorkingTreeChanges.map(change => change.uri.fsPath);
147+
148+
assert.strictEqual(trackedWorkingTreeChanges.length, 1);
149+
assert.strictEqual(trackedWorkingTreeFiles[0], '/root/modified.txt');
150+
});
151+
});

0 commit comments

Comments
 (0)