Skip to content

Commit 5c7fea1

Browse files
authored
Fix/wrong target type after renaming in csolution yaml file (#49)
* Implement external file change detection and refresh logic in text file management * Add activated state handling in solution load change events * Add delay to ensure external file change detection in tests * Rename sendContextData to sendContextDataFromControllerState for clarity and update references in tests * Improve active target type handling in ManageSolutionController and update related tests * Refactor ensureActiveTargetTypeNameInKnownTargets access in manage-solution-controller tests * Fix: update test to spy on sendContextDataFromControllerState instead of sendContextData
1 parent f99f961 commit 5c7fea1

7 files changed

Lines changed: 372 additions & 53 deletions

src/generic/text-file.factory.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,6 @@ export const textFileFactory = makeFactory<ITextFile, IErrorList>({
3939
clear: () => jest.fn(),
4040
exists: () => jest.fn(),
4141
unlink: () => jest.fn(),
42+
hasExternalFileChanged: () => jest.fn().mockReturnValue(false),
43+
refreshExternalFileStamp: () => jest.fn(),
4244
}, errorListFactory);

src/generic/text-file.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,4 +370,32 @@ describe('TextFile', () => {
370370

371371
testDataHandler.rmDir(unicodeDir);
372372
});
373+
374+
it('primes external stamp when checked before load/save', async () => {
375+
fsUtils.writeTextFile(TEST_FILE, initialContent);
376+
const tf = new TextFile(TEST_FILE);
377+
378+
expect(tf.hasExternalFileChanged()).toBe(false);
379+
380+
fsUtils.writeTextFile(TEST_FILE, `${changedContent}!`);
381+
await new Promise(resolve => setTimeout(resolve, 500));
382+
383+
expect(tf.hasExternalFileChanged()).toBe(true);
384+
});
385+
386+
it('detects external on-disk updates and then re-baselines', async () => {
387+
const tf = new TextFile(TEST_FILE);
388+
tf.text = initialContent;
389+
const saveResult = await tf.save();
390+
expect(saveResult).toBe(ETextFileResult.Success);
391+
392+
tf.refreshExternalFileStamp();
393+
expect(tf.hasExternalFileChanged()).toBe(false);
394+
395+
fsUtils.writeTextFile(TEST_FILE, `${changedContent}!`);
396+
await new Promise(resolve => setTimeout(resolve, 500));
397+
398+
expect(tf.hasExternalFileChanged()).toBe(true);
399+
expect(tf.hasExternalFileChanged()).toBe(false);
400+
});
373401
});

src/generic/text-file.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
import path from 'node:path';
18+
import * as fs from 'node:fs';
1819
import * as fsUtils from '../utils/fs-utils';
1920
import * as vscodeUtils from '../utils/vscode-utils';
2021
import { ITextParser } from './text-parser';
@@ -40,6 +41,12 @@ export enum ETextFileResult {
4041
NotExists = 3
4142
}
4243

44+
type ExternalFileStamp = {
45+
path: string;
46+
mtimeMs: number;
47+
size: number;
48+
};
49+
4350
/**
4451
* Represents a text file with parsing, rendering, and error handling capabilities.
4552
* Provides methods and properties for managing file content, file metadata, and associated parsers/renderers.
@@ -170,6 +177,16 @@ export interface ITextFile extends IErrorList {
170177
/** Clears file content and errors */
171178
clear(): void;
172179

180+
/**
181+
* Refreshes the stored external file stamp baseline.
182+
*/
183+
refreshExternalFileStamp(): void;
184+
185+
/**
186+
* Checks whether the file changed externally since the last stamp refresh/check.
187+
*/
188+
hasExternalFileChanged(): boolean;
189+
173190
/** Copy text and parse it to the object, does not change filename
174191
* @param src source file
175192
*/
@@ -194,6 +211,7 @@ export class TextFile extends ErrorList implements ITextFile {
194211
protected textRenderer?: ITextRenderer;
195212

196213
private _readOnly = false;
214+
private externalFileStamp?: ExternalFileStamp;
197215

198216
/**
199217
* Constructs a TextFile instance
@@ -243,6 +261,7 @@ export class TextFile extends ErrorList implements ITextFile {
243261
this._dirty = false;
244262
this.contentString = '';
245263
this.contentObject = undefined;
264+
this.externalFileStamp = undefined;
246265
this.clearErrors();
247266
}
248267

@@ -285,6 +304,7 @@ export class TextFile extends ErrorList implements ITextFile {
285304
if (value !== this._fileName) {
286305
this._fileName = value;
287306
this._fileDir = path.dirname(value);
307+
this.externalFileStamp = undefined;
288308
}
289309
}
290310

@@ -360,6 +380,7 @@ export class TextFile extends ErrorList implements ITextFile {
360380
this.fileName = fileName;
361381
}
362382
const result = this.doLoad();
383+
this.refreshExternalFileStamp();
363384
this.showErrorMessage(result);
364385
return result;
365386
}
@@ -402,10 +423,47 @@ export class TextFile extends ErrorList implements ITextFile {
402423
this._dirty = true; // force saving
403424
}
404425
const result = this.doSave();
426+
this.refreshExternalFileStamp();
405427
this.showErrorMessage(result);
406428
return result;
407429
}
408430

431+
private getCurrentFileStamp(): ExternalFileStamp | undefined {
432+
if (!this.fileName || !fsUtils.fileExists(this.fileName)) {
433+
return undefined;
434+
}
435+
try {
436+
const stat = fs.statSync(this.fileName);
437+
return {
438+
path: this.fileName,
439+
mtimeMs: stat.mtimeMs,
440+
size: stat.size,
441+
};
442+
} catch {
443+
return undefined;
444+
}
445+
}
446+
447+
public refreshExternalFileStamp(): void {
448+
this.externalFileStamp = this.getCurrentFileStamp();
449+
}
450+
451+
public hasExternalFileChanged(): boolean {
452+
const currentStamp = this.getCurrentFileStamp();
453+
if (!this.externalFileStamp) {
454+
this.externalFileStamp = currentStamp;
455+
return false;
456+
}
457+
458+
const changed = !currentStamp
459+
|| currentStamp.path !== this.externalFileStamp.path
460+
|| currentStamp.mtimeMs !== this.externalFileStamp.mtimeMs
461+
|| currentStamp.size !== this.externalFileStamp.size;
462+
463+
this.externalFileStamp = currentStamp;
464+
return changed;
465+
}
466+
409467
/**
410468
* Saves file content to disk
411469
* @returns Save result

src/views/manage-solution/manage-solution-controller.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,28 @@ describe('manage-solution-controller', () => {
145145
expect(controller.activeTargetTypeName).toBe(initialName);
146146
});
147147

148+
it('should recover from persisted empty active target type using first known target', async () => {
149+
const controller = new ManageSolutionController();
150+
await controller.loadSolution('test-resources/solutions/solution-with-debuggers.csolution');
151+
152+
controller.activeTargetTypeName = '';
153+
const updated = await controller.ensureActiveTargetTypeName();
154+
155+
expect(updated).toBe(true);
156+
expect(controller.activeTargetTypeName).toBe(controller.solutionData.targets[0].name);
157+
});
158+
159+
it('should clear persisted invalid active target type when no target types are available', async () => {
160+
const controller = new ManageSolutionController();
161+
await controller.loadSolution('test-resources/solutions/solution-with-debuggers.csolution');
162+
163+
controller.activeTargetTypeName = 'invalid-target';
164+
const updated = await controller['ensureActiveTargetTypeNameInKnownTargets']([]);
165+
166+
expect(updated).toBe(true);
167+
expect(controller.cmsisJsonFile.activeTargetTypeName).toBeUndefined();
168+
});
169+
148170
it('should get active target set name', async () => {
149171
const controller = new ManageSolutionController();
150172
await controller.loadSolution('test-resources/solutions/solution-with-debuggers.csolution');

0 commit comments

Comments
 (0)