Skip to content

Commit 6458f81

Browse files
IliyaBrookclaude
andcommitted
fix: address review nits — atomic write, regex docs, strip test
- scripts/tray-icon-settings.js: _save() now writes to <path>.tmp and renameSync's on top of the real file, so a kill mid-write can't truncate the settings file. The failure path also unlinks the stray .tmp if the write itself threw. - tests/tray-icon-settings.test.js: add coverage asserting the parent directory has only the final file after a save cycle (no leftover .tmp). - scripts/patches/tray.sh: document the [^\]]*? assumption in the submenu-strip regex — relies on no radio label containing a literal ']'. All addressed from @aaddrick's PR #491 review. Co-Authored-By: Claude <[email protected]>
1 parent ea7eaaa commit 6458f81

3 files changed

Lines changed: 25 additions & 1 deletion

File tree

scripts/patches/tray.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,9 @@ patch_tray_icon_submenu() {
234234
const fs = require('fs');
235235
const p = 'app.asar.contents/.vite/build/index.js';
236236
let code = fs.readFileSync(p, 'utf8');
237+
// NB: [^\\]]*? assumes no radio label inside the submenu contains
238+
// a literal ']' — true for Auto/Black/White today; revisit the
239+
// regex (e.g. balanced-brace match) if a future label gains one.
237240
const prevRe = /\\{label:\"Icon color[^\"]*\",submenu:\\[[^\\]]*?\\]\\},/;
238241
if (prevRe.test(code)) {
239242
code = code.replace(prevRe, '');

scripts/tray-icon-settings.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,16 +79,23 @@ class TrayIconSettings extends EventEmitter {
7979
return migrated;
8080
}
8181

82+
// Atomic write via <path>.tmp + renameSync so a kill mid-write
83+
// can't truncate the settings file — _load() is soft-fail, but
84+
// renameSync on POSIX guarantees the reader sees either the old
85+
// content or the fully-written new content, never a partial one.
8286
_save() {
87+
const tmp = this._path + '.tmp';
8388
try {
8489
fs.mkdirSync(path.dirname(this._path), { recursive: true });
8590
fs.writeFileSync(
86-
this._path,
91+
tmp,
8792
JSON.stringify({ iconMode: this._mode }, null, 2) + '\n',
8893
'utf8',
8994
);
95+
fs.renameSync(tmp, this._path);
9096
} catch (e) {
9197
console.warn('[Tray Icon] Write failed:', e.message);
98+
try { fs.unlinkSync(tmp); } catch (_) { /* ignore */ }
9299
}
93100
}
94101
}

tests/tray-icon-settings.test.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,20 @@ test('set: persists value to disk', () => {
158158
} finally { cleanup(tmp); }
159159
});
160160

161+
test('set: atomic write leaves no leftover .tmp file', () => {
162+
const tmp = mkTmp();
163+
try {
164+
const p = path.join(tmp, 'atomic.json');
165+
const s = new TrayIconSettings({ path: p });
166+
s.set('black');
167+
s.set('white');
168+
s.set('auto');
169+
const entries = fs.readdirSync(tmp);
170+
// Only the final file should exist, never a trailing .tmp
171+
assert.deepEqual(entries, ['atomic.json']);
172+
} finally { cleanup(tmp); }
173+
});
174+
161175
test('set: creates parent directory if missing', () => {
162176
const tmp = mkTmp();
163177
try {

0 commit comments

Comments
 (0)