Skip to content

Commit d29d6f6

Browse files
committed
fix: address PR review feedback
- Preserve JSON formatting by using editor's indent/newline symbols - Handle deleted config keys explicitly (editor.update only merges) - Capture config snapshot at write time, not schedule time - Fix load method create parameter to actually create empty state
1 parent 58863e3 commit d29d6f6

File tree

2 files changed

+46
-17
lines changed

2 files changed

+46
-17
lines changed

src/utils/config.mts

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -350,10 +350,11 @@ export function updateConfigValue<Key extends keyof LocalConfig>(
350350

351351
if (!_pendingSave) {
352352
_pendingSave = true
353-
// Capture the current config state to save.
354-
const configToSave = { ...localConfig }
355353
process.nextTick(() => {
356354
_pendingSave = false
355+
// Capture the config state at write time, not at schedule time.
356+
// This ensures all updates in the same tick are included.
357+
const configToSave = { ...localConfig }
357358
const { socketAppDataPath } = constants
358359
if (socketAppDataPath) {
359360
mkdirSync(socketAppDataPath, { recursive: true })
@@ -377,17 +378,45 @@ export function updateConfigValue<Key extends keyof LocalConfig>(
377378
editor.create(configFilePath)
378379
}
379380
// Update with the captured config state.
380-
editor.update(configToSave)
381-
// Get content with formatting symbols stripped.
382-
const contentToSave = Object.fromEntries(
383-
Object.entries(editor.content).filter(
384-
([key]) => typeof key === 'string',
385-
),
381+
// Note: We need to handle deletions explicitly since editor.update() only merges.
382+
// First, get all keys from the existing content.
383+
const existingKeys = new Set(
384+
Object.keys(editor.content).filter(k => typeof k === 'string')
386385
)
387-
const jsonContent = JSON.stringify(contentToSave)
386+
const newKeys = new Set(Object.keys(configToSave))
387+
388+
// Delete keys that are in existing but not in new config.
389+
for (const key of existingKeys) {
390+
if (!newKeys.has(key)) {
391+
delete (editor.content as any)[key]
392+
}
393+
}
394+
395+
// Now update with new values.
396+
editor.update(configToSave)
397+
// Use the editor's internal stringify which preserves formatting.
398+
// We need to extract the content without symbols and then stringify
399+
// with the formatting metadata.
400+
const { getEditableJsonClass: _, ...contentWithoutImport } = editor.content as any
401+
const INDENT_SYMBOL = Symbol.for('indent')
402+
const NEWLINE_SYMBOL = Symbol.for('newline')
403+
const indent = (editor.content as any)[INDENT_SYMBOL] ?? 2
404+
const newline = (editor.content as any)[NEWLINE_SYMBOL] ?? '\n'
405+
406+
// Strip formatting symbols from content.
407+
const contentToSave: Record<string, unknown> = {}
408+
for (const [key, val] of Object.entries(editor.content)) {
409+
if (typeof key === 'string') {
410+
contentToSave[key] = val
411+
}
412+
}
413+
414+
// Stringify with formatting preserved.
415+
const jsonContent = JSON.stringify(contentToSave, undefined, indent)
416+
.replace(/\n/g, newline)
388417
writeFileSync(
389418
configFilePath,
390-
Buffer.from(jsonContent).toString('base64'),
419+
Buffer.from(jsonContent + newline).toString('base64'),
391420
)
392421
}
393422
})

src/utils/editable-json.mts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -300,20 +300,20 @@ export class EditableJson<T = Record<string, unknown>> {
300300
*/
301301
async load(path: string, create?: boolean): Promise<this> {
302302
this._path = path
303-
let parseErr: Error | undefined
304303
try {
305304
this._readFileContent = await readFile(this.filename)
305+
this.fromJSON(this._readFileContent)
306+
this._readFileJson = parseJson(this._readFileContent)
306307
} catch (err) {
307308
if (!create) {
308309
throw err
309310
}
310-
parseErr = err as Error
311+
// File doesn't exist and create is true - initialize empty.
312+
this._content = {}
313+
this._readFileContent = ''
314+
this._readFileJson = undefined
315+
this._canSave = true
311316
}
312-
if (parseErr) {
313-
throw parseErr
314-
}
315-
this.fromJSON(this._readFileContent)
316-
this._readFileJson = parseJson(this._readFileContent)
317317
return this
318318
}
319319

0 commit comments

Comments
 (0)