Skip to content

Commit 67b0c9f

Browse files
committed
Add a messege to inform user Merge view can be closed after merge
1 parent 49b4bdd commit 67b0c9f

5 files changed

Lines changed: 124 additions & 38 deletions

File tree

src/desktop/extension.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ export const activate = async (context: ExtensionContext): Promise<CsolutionExte
222222
const treeViewProviderImpl = new TreeViewProviderImpl(SolutionOutlineView.treeViewId);
223223
const treeViewFileDecorationProvider = new TreeViewFileDecorationProvider(fileDecorationProviderManager, themeProvider);
224224
const mergeSessionCoordinator = new MergeSessionCoordinatorImpl(commandsProvider);
225-
const mergeCommand = new MergeCommand(commandsProvider, mergeSessionCoordinator);
225+
const mergeCommand = new MergeCommand(commandsProvider, mergeSessionCoordinator, messageProvider);
226226
const buildCommand = new BuildCommand(buildTaskProvider, commandsProvider, buildTaskDefinitionBuilder);
227227
const runGeneratorCommand = new GeneratorCommand(commandsProvider, solutionManager, outputChannelProvider, cmsisToolboxManager);
228228
const armclangDefineGetter = new ArmclangDefineGetter(processManager, workspaceFsProvider);

src/views/solution-outline/commands/merge-command.test.ts

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
jest.mock('vscode', () => ({
1818
window: {
1919
showErrorMessage: jest.fn(),
20+
showInformationMessage: jest.fn(),
2021
showWarningMessage: jest.fn(),
2122
}
2223
}));
@@ -32,7 +33,6 @@ jest.mock('path', () => {
3233
});
3334

3435
import { TestDataHandler } from '../../../__test__/test-data';
35-
import * as vscode from 'vscode';
3636
import { extensionContextFactory } from '../../../vscode-api/extension-context.factories';
3737
import { commandsProviderFactory, MockCommandsProvider } from '../../../vscode-api/commands-provider.factories';
3838
import { MergeCommand } from './merge-command';
@@ -42,6 +42,7 @@ import * as os from 'os';
4242
import * as path from 'path';
4343
import * as fsUtils from '../../../utils/fs-utils';
4444
import { MergeSessionCoordinator } from './merge-session-coordinator';
45+
import { messageProviderFactory, MockMessageProvider } from '../../../vscode-api/message-provider.factories';
4546

4647
jest.mock('child_process');
4748
jest.mock('os');
@@ -50,6 +51,7 @@ describe('MergeCommand', () => {
5051
let commandsProvider: MockCommandsProvider;
5152
let command: MergeCommand;
5253
let mergeSessionCoordinator: jest.Mocked<MergeSessionCoordinator>;
54+
let messageProvider: MockMessageProvider;
5355
const testDataHandler = new TestDataHandler();
5456
let tmpDir: string;
5557

@@ -82,11 +84,13 @@ describe('MergeCommand', () => {
8284
commandsProvider = commandsProviderFactory();
8385
mergeSessionCoordinator = {
8486
activate: jest.fn().mockResolvedValue(),
87+
onMergeApplied: jest.fn().mockReturnValue({ dispose: jest.fn() }),
8588
startSession: jest.fn(),
8689
cancelSession: jest.fn(),
87-
onMergeProcessExit: jest.fn().mockResolvedValue(),
90+
onMergeProcessExit: jest.fn().mockResolvedValue({ applied: false }),
8891
};
89-
command = new MergeCommand(commandsProvider, mergeSessionCoordinator);
92+
messageProvider = messageProviderFactory();
93+
command = new MergeCommand(commandsProvider, mergeSessionCoordinator, messageProvider);
9094

9195
componentNode = new COutlineItem('component');
9296
componentNode.setTag('component');
@@ -106,10 +110,23 @@ describe('MergeCommand', () => {
106110
await command.activate(extensionContextFactory());
107111

108112
expect(mergeSessionCoordinator.activate).toHaveBeenCalledTimes(1);
113+
expect(mergeSessionCoordinator.onMergeApplied).toHaveBeenCalledTimes(1);
109114
expect(commandsProvider.registerCommand).toHaveBeenCalledTimes(1);
110115
expect(commandsProvider.registerCommand).toHaveBeenCalledWith(MergeCommand.mergeFile, expect.any(Function), expect.anything());
111116
});
112117

118+
it('shows success info when coordinator emits merge-applied event', async () => {
119+
await command.activate(extensionContextFactory());
120+
121+
const listener = mergeSessionCoordinator.onMergeApplied.mock.calls[0]?.[0] as (() => void) | undefined;
122+
expect(listener).toBeDefined();
123+
listener?.();
124+
125+
expect(messageProvider.showInformationMessage).toHaveBeenCalledWith(
126+
'Merge applied successfully. Merge View can be closed.',
127+
);
128+
});
129+
113130
it('forwards string command argument to the merge handler', async () => {
114131
const runVSCodeMergeFromPathSpy = jest.spyOn(
115132
command as unknown as { runVSCodeMergeFromPath: (localPath: string) => Promise<void> },
@@ -136,22 +153,18 @@ describe('MergeCommand', () => {
136153
});
137154

138155
it('shows error for unsupported command argument type', async () => {
139-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
140-
141156
await command.activate(extensionContextFactory());
142157
await commandsProvider.mockRunRegistered(MergeCommand.mergeFile, undefined);
143158

144-
expect(showErrorMessageSpy).toHaveBeenCalledWith('Cannot open merge view: unsupported command argument.');
159+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('Cannot open merge view: unsupported command argument.');
145160
});
146161
});
147162

148163
describe('merge file discovery', () => {
149164
it('shows error if merge path is missing when invoked from path', async () => {
150-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
151-
152165
await command['runVSCodeMergeFromPath']('');
153166

154-
expect(showErrorMessageSpy).toHaveBeenCalledWith('Cannot open merge view: merge file path is missing.');
167+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('Cannot open merge view: merge file path is missing.');
155168
});
156169

157170
it('normalizes the merge path before opening merge view from path', async () => {
@@ -167,17 +180,15 @@ describe('MergeCommand', () => {
167180
});
168181

169182
it('shows error if node is not passed', async () => {
170-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
171183
// @ts-expect-error - testing behavior when `runVSCodeMerge` receives null
172184
await command['runVSCodeMerge'](null);
173-
expect(showErrorMessageSpy).toHaveBeenCalledWith('File data is not available for merge operation.');
185+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('File data is not available for merge operation.');
174186
});
175187

176188
it('shows error if required local file is missing', async () => {
177-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
178189
const node = new COutlineItem('file');
179190
await command['runVSCodeMerge'](node);
180-
expect(showErrorMessageSpy).toHaveBeenCalledWith('Required local file is missing to perform merge.');
191+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('Required local file is missing to perform merge.');
181192
});
182193

183194
it('finds update and base files using the highest matching semver sibling', () => {
@@ -338,9 +349,8 @@ describe('MergeCommand', () => {
338349
throw new Error('not found');
339350
});
340351

341-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
342352
await command['runVSCodeMerge'](fileNode);
343-
expect(showErrorMessageSpy).toHaveBeenCalledWith('Visual Studio Code executable not found. Please ensure it is installed and available in your PATH.');
353+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('Visual Studio Code executable not found. Please ensure it is installed and available in your PATH.');
344354
});
345355
});
346356

@@ -375,10 +385,9 @@ describe('MergeCommand', () => {
375385
mockedExec.mockImplementation((_cmd, _cb) => { throw new Error('unexpected'); });
376386

377387
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => { });
378-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
379388
await command['runVSCodeMerge'](fileNode);
380389
expect(errorSpy).toHaveBeenCalledWith('Merge operations failed:', expect.any(Error));
381-
expect(showErrorMessageSpy).toHaveBeenCalledWith('Merge operation failed: unexpected');
390+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('Merge operation failed: unexpected');
382391
expect(mergeSessionCoordinator.cancelSession).toHaveBeenCalledTimes(1);
383392
});
384393

@@ -400,11 +409,9 @@ describe('MergeCommand', () => {
400409
jest.spyOn(fsUtils, 'copyFile').mockImplementation(() => { });
401410
jest.spyOn(fsUtils, 'getFileModificationTime').mockReturnValue(1000);
402411

403-
const showErrorMessageSpy = jest.spyOn(vscode.window, 'showErrorMessage');
404-
405412
await command['runVSCodeMerge'](node);
406413

407-
expect(showErrorMessageSpy).toHaveBeenCalledWith('Merge operation failed: Invalid update file: contains unsupported shell-sensitive characters.');
414+
expect(messageProvider.showErrorMessage).toHaveBeenCalledWith('Merge operation failed: Invalid update file: contains unsupported shell-sensitive characters.');
408415
expect(mergeSessionCoordinator.cancelSession).toHaveBeenCalledTimes(1);
409416
});
410417

@@ -441,5 +448,6 @@ describe('MergeCommand', () => {
441448
expect(mergeSessionCoordinator.onMergeProcessExit).toHaveBeenCalledWith(0);
442449
expect(mergeSessionCoordinator.cancelSession).not.toHaveBeenCalled();
443450
});
451+
444452
});
445453
});

src/views/solution-outline/commands/merge-command.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,25 @@ import semver from 'semver';
2525
import { extractVersion } from '../../../utils/string-utils';
2626
import * as fsUtils from '../../../utils/fs-utils';
2727
import { MergeSessionCoordinator } from './merge-session-coordinator';
28+
import type { MessageProvider } from '../../../vscode-api/message-provider';
2829

2930
export class MergeCommand {
3031
public static readonly mergeFile = `${PACKAGE_NAME}.mergeFile`;
32+
private static readonly mergeAppliedSuccessMessage = 'Merge applied successfully. Merge View can be closed.';
3133
private static readonly disallowedCmdChars = /[\r\n&|<>^%"']/;
3234

3335
constructor(
3436
private readonly commandsProvider: CommandsProvider,
3537
private readonly mergeSessionCoordinator: MergeSessionCoordinator,
38+
private readonly messageProvider: Pick<MessageProvider, 'showErrorMessage' | 'showInformationMessage' | 'showWarningMessage'>,
3639
) { }
3740

3841
public async activate(context: Pick<vscode.ExtensionContext, 'subscriptions'>) {
3942
await this.mergeSessionCoordinator.activate(context);
4043
context.subscriptions.push(
44+
this.mergeSessionCoordinator.onMergeApplied(() => {
45+
this.messageProvider.showInformationMessage(MergeCommand.mergeAppliedSuccessMessage);
46+
}),
4147
this.commandsProvider.registerCommand(MergeCommand.mergeFile, this.handleMergeCommand, this),
4248
);
4349
}
@@ -53,12 +59,12 @@ export class MergeCommand {
5359
return;
5460
}
5561

56-
vscode.window.showErrorMessage('Cannot open merge view: unsupported command argument.');
62+
this.messageProvider.showErrorMessage('Cannot open merge view: unsupported command argument.');
5763
}
5864

5965
private async runVSCodeMergeFromPath(localPath: string): Promise<void> {
6066
if (!localPath) {
61-
vscode.window.showErrorMessage('Cannot open merge view: merge file path is missing.');
67+
this.messageProvider.showErrorMessage('Cannot open merge view: merge file path is missing.');
6268
return;
6369
}
6470

@@ -67,13 +73,13 @@ export class MergeCommand {
6773

6874
private async runVSCodeMerge(node: COutlineItem): Promise<void> {
6975
if (!node) {
70-
vscode.window.showErrorMessage('File data is not available for merge operation.');
76+
this.messageProvider.showErrorMessage('File data is not available for merge operation.');
7177
return;
7278
}
7379

7480
const localPath = node.getResourcePath();
7581
if (!localPath) {
76-
vscode.window.showErrorMessage('Required local file is missing to perform merge.');
82+
this.messageProvider.showErrorMessage('Required local file is missing to perform merge.');
7783
return;
7884
}
7985

@@ -90,7 +96,7 @@ export class MergeCommand {
9096

9197
const codePath = this.getVSCodeExecutablePath();
9298
if (!codePath) {
93-
vscode.window.showErrorMessage('Visual Studio Code executable not found. Please ensure it is installed and available in your PATH.');
99+
this.messageProvider.showErrorMessage('Visual Studio Code executable not found. Please ensure it is installed and available in your PATH.');
94100
return;
95101
}
96102

@@ -130,7 +136,7 @@ export class MergeCommand {
130136
} catch (err) {
131137
console.error('Merge operations failed:', err);
132138
const details = err instanceof Error ? err.message : String(err);
133-
vscode.window.showErrorMessage(`Merge operation failed: ${details}`);
139+
this.messageProvider.showErrorMessage(`Merge operation failed: ${details}`);
134140
} finally {
135141
if (!exitHandled) {
136142
this.mergeSessionCoordinator.cancelSession();
@@ -143,13 +149,13 @@ export class MergeCommand {
143149
const discovered = this.findNewestMergeFiles(local);
144150
const update = discovered.update;
145151
if (!update) {
146-
vscode.window.showErrorMessage('Required update file is missing to perform merge.');
152+
this.messageProvider.showErrorMessage('Required update file is missing to perform merge.');
147153
return undefined;
148154
}
149155

150156
const base = discovered.base;
151157
if (!base) {
152-
vscode.window.showErrorMessage('Required base file is missing to perform merge.');
158+
this.messageProvider.showErrorMessage('Required base file is missing to perform merge.');
153159
return undefined;
154160
}
155161

@@ -193,7 +199,7 @@ export class MergeCommand {
193199
const resolved = execSync('which code', { stdio: ['pipe', 'pipe', 'ignore'] }).toString().trim();
194200
if (resolved && fsUtils.fileExists(resolved)) return resolved;
195201
} catch (err) {
196-
vscode.window.showWarningMessage(`Could not resolve 'code' binary via 'which': ${err instanceof Error ? err.message : String(err)}`);
202+
this.messageProvider.showWarningMessage(`Could not resolve 'code' binary via 'which': ${err instanceof Error ? err.message : String(err)}`);
197203
return undefined;
198204
}
199205
}

src/views/solution-outline/commands/merge-session-coordinator.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,29 @@ describe('MergeSessionCoordinator', () => {
8181
expect(commandsProvider.executeCommand).toHaveBeenCalledWith(REFRESH_COMMAND_ID);
8282
});
8383

84+
it('emits merge-applied event on merged file save', async () => {
85+
const local = path.join(tmpDir, 'component.c');
86+
const update = path.join(tmpDir, 'component.c.update@1.0.0');
87+
const base = path.join(tmpDir, 'component.c.base@1.0.0');
88+
const merged = `${local}.merged`;
89+
90+
fsUtils.writeTextFile(local, '// local\n');
91+
fsUtils.writeTextFile(update, '// update\n');
92+
fsUtils.writeTextFile(base, '// base\n');
93+
fsUtils.writeTextFile(merged, '// merged\n');
94+
95+
const listener = jest.fn();
96+
const disposable = coordinator.onMergeApplied(listener);
97+
coordinator.startSession({ local, update, base, merged, mergedMTimeBefore: 0 });
98+
99+
await (coordinator as unknown as {
100+
handleDidSaveTextDocument: (document: vscode.TextDocument) => Promise<void>
101+
}).handleDidSaveTextDocument({ uri: { fsPath: merged } } as vscode.TextDocument);
102+
103+
expect(listener).toHaveBeenCalledTimes(1);
104+
disposable.dispose();
105+
});
106+
84107
it('ignores save events for unrelated files', async () => {
85108
const local = path.join(tmpDir, 'component.c');
86109
const update = path.join(tmpDir, 'component.c.update@1.0.0');
@@ -143,4 +166,22 @@ describe('MergeSessionCoordinator', () => {
143166
expect(commandsProvider.executeCommand).toHaveBeenCalledTimes(1);
144167
expect(commandsProvider.executeCommand).toHaveBeenCalledWith(REFRESH_COMMAND_ID);
145168
});
169+
170+
it('finalizes on merge process exit when update file is already absent but base exists', async () => {
171+
const local = path.join(tmpDir, 'component.c');
172+
const update = path.join(tmpDir, 'component.c.update@1.0.0');
173+
const base = path.join(tmpDir, 'component.c.base@1.0.0');
174+
const merged = `${local}.merged`;
175+
176+
fsUtils.writeTextFile(local, '// local\n');
177+
fsUtils.writeTextFile(base, '// base\n');
178+
fsUtils.writeTextFile(merged, '// merged\n');
179+
180+
coordinator.startSession({ local, update, base, merged, mergedMTimeBefore: 0 });
181+
await expect(coordinator.onMergeProcessExit(0)).resolves.toEqual({ applied: true });
182+
183+
expect(fsUtils.fileExists(local)).toBeTruthy();
184+
expect(fsUtils.readTextFile(local)).toContain('// merged');
185+
expect(fsUtils.fileExists(base)).toBeTruthy();
186+
});
146187
});

0 commit comments

Comments
 (0)