Skip to content

feat: support duplicate snippet shortcuts#311

Merged
bartekplus merged 2 commits intomasterfrom
codex/duplicate-snippet-shortcuts
Mar 14, 2026
Merged

feat: support duplicate snippet shortcuts#311
bartekplus merged 2 commits intomasterfrom
codex/duplicate-snippet-shortcuts

Conversation

@bartekplus
Copy link
Owner

Summary

  • allow duplicate snippet shortcuts to be saved and imported without collapsing them in the options UI
  • refresh live Presage engines when text expansion data changes so updated duplicate shortcuts appear in the typing popup
  • add regression coverage for duplicate snippet persistence, live Presage refresh, and popup selection

Test Plan

  • bun test tests/TextAssetsPanel.test.ts
  • bun test tests/PresageHandler.live.test.ts
  • bun scripts/run-e2e.ts --suite=smoke -- --test-name-pattern="text expansion popup shows duplicate shortcut entries"
  • bun run lint
  • bun run format:check

@bartekplus
Copy link
Owner Author

Code review

Found 1 issue:

  1. CSV import is no longer idempotent — re-importing the same file accumulates duplicate entries

mergeExpansions() was changed from a Map-based merge (which deduplicates by shortcut key) to a flatMap concatenation that emits every entry unconditionally. Before this PR, importing a CSV whose shortcuts already existed in the persisted list was safe to repeat: the Map ensured one entry per shortcut. Now, every import appends all rows unconditionally, so importing the same CSV twice doubles every entry, importing it three times triples them, and so on. All duplicated entries are written to storage and then to /textExpansions.txt via TextExpansionManager.setupTextExpansions.

this.registry[KEY_USER_DICTIONARY_LIST].set(this.dictionary);
}
private mergeExpansions(
existing: TextExpansionEntry[],
imported: TextExpansionEntry[],
): TextExpansionEntry[] {
return [...existing, ...imported].flatMap(([shortcut, text]) => {
const normalizedShortcut = shortcut.trim();
return normalizedShortcut ? ([[normalizedShortcut, text]] as TextExpansionEntry[]) : [];
});
}

The call site at line 191 also swapped argument order (from mergeExpansions(imported, existing) to mergeExpansions(existing, imported)), which is correct for the new signature but means any merge logic that previously gave imported entries priority over existing ones (last-write-wins via Map) now gives existing entries first position in the output array.

const imported = parsed
.filter((row) => row.length === 2)
.map((row) => [toTextValue(row[0]), toTextValue(row[1])] as TextExpansionEntry);
this.syncPersistedRows(this.mergeExpansions(this.getPersistedExpansions(), imported));
this.setSnippetStatus(i18n.get("settings_status_saved"));
this.persistSnippetRows();

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@bartekplus
Copy link
Owner Author

Addressed the CSV import regression in 57586c2. Import now deduplicates only exact (shortcut, text) pairs, so re-importing the same file is idempotent while same-shortcut snippets with different bodies are still preserved. Added a regression test that imports the same CSV twice and verifies only distinct pairs remain.

@bartekplus bartekplus merged commit 81f706c into master Mar 14, 2026
8 checks passed
@bartekplus bartekplus deleted the codex/duplicate-snippet-shortcuts branch March 14, 2026 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant