diff --git a/.claude/skills/verify/features/timestamps-uris.md b/.claude/skills/verify/features/timestamps-uris.md index 4a807646..773c2064 100644 --- a/.claude/skills/verify/features/timestamps-uris.md +++ b/.claude/skills/verify/features/timestamps-uris.md @@ -28,9 +28,9 @@ Preconditions: - `PodNotes Fixture` is saved ([feeds.md](./feeds.md)) and `Fixture Episode 1` is unplayed. - A note exists and is open: `CLI create path='Verify/Timestamps.md' content='# Timestamps\n\n' open`. -- **Set a linked format.** Open Settings > PodNotes ([feeds.md](./feeds.md)), run `UI fill '.modal.mod-settings textarea[placeholder="- {{linktime}} "]' '- {{linktime}} '`, then close Settings. `data.json` `timestamp.template` is `- {{linktime}} `. +- **Set a linked format.** Open Settings > PodNotes ([feeds.md](./feeds.md)), run `UI fill '.modal.mod-settings textarea[placeholder="- {{linktime}} "]' '* {{linktime}} - '`, then close Settings. `data.json` `timestamp.template` is `* {{linktime}} - `. - **Play.** Start `Fixture Episode 1` ([player.md](./player.md)) and wait about 5 s. -- **Capture.** Run `UI click '.workspace-leaf-content[data-type="markdown"] .cm-content .cm-line:last-child'`, `UI key Ctrl+p`, `UI type .prompt-input 'Capture Timestamp'`, `UI click .suggestion-item --text 'Capture Timestamp'`. After about 2 s, `CLI read path='Verify/Timestamps.md'` has a line like `- [00:00:05](obsidian://podnotes?episodeName=Fixture%20Episode%201&url=...&time=5.73)`. +- **Capture.** Run `UI click '.workspace-leaf-content[data-type="markdown"] .cm-content .cm-line:last-child'`, `UI key Ctrl+p`, `UI type .prompt-input 'Capture Timestamp'`, `UI click .suggestion-item --text 'Capture Timestamp'`. After about 2 s, `CLI read path='Verify/Timestamps.md'` has a line like `* [00:00:05](obsidian://podnotes?episodeName=Fixture%20Episode%201&url=...&time=5.73) - `. - **Segment.** Run the same palette steps with `Capture Last 10 Seconds`. The line holds a range such as `00:00:05-00:00:15` and its URI has `time` and `endTime`. - **Open the link.** Let playback move on, run `CLI command id=markdown:toggle-preview` (Reading view), then `UI click '.markdown-reading-view a' --text '00:00:05'`. `api.podcast.title` is `Fixture Episode 1` and `api.currentTime` restarts from about 5.7 s. The instance log `/Library/Logs/obsidian.log` shows `Received callback URL` and `Processed URI`. - **Direct URI path.** Run `UI uri 'obsidian://podnotes?episodeName=Fixture%20Episode%201&url=&time=30'`. `api.currentTime` jumps to about 30 s. @@ -39,7 +39,7 @@ Preconditions: ## Gotchas - Capture commands are editor commands. The palette lists them only while a Markdown editor is focused, so click into the note first. -- The default format `- {{time}} ` inserts plain text with no link. Set `{{linktime}}` before proving links. +- The default format is `- {{linktime}} `. A vault whose `data.json` holds a `{{time}}` format inserts plain text with no link, so check `timestamp.template` before proving links. - Obsidian writes the note about 2 s after the edit. Read after a short wait, or read the editor with `CLI eval code='app.workspace.getLeavesOfType("markdown")[0].view.editor.getValue()'`. - A clicked link leaves the app through `xdg-open` and comes back through Obsidian's single-instance handoff, which the Linux bridge sets up. Never run `xdg-open` yourself: it reaches the system Obsidian profile. - `UI uri` decodes values with `decodeURIComponent`, as Obsidian does, so `+` stays a literal plus. diff --git a/docs/docs/timestamps.md b/docs/docs/timestamps.md index ea696045..c52611e2 100644 --- a/docs/docs/timestamps.md +++ b/docs/docs/timestamps.md @@ -7,6 +7,8 @@ With the default `{{linktime}}` format, each captured timestamp becomes a clicka ![Notes with clickable timestamp links](resources/timestamps.png) +If you used PodNotes before this default, it keeps your saved format. Set `Capture timestamp format` to `- {{linktime}}` followed by a space to get clickable links. + ## Settings For timestamps, you can use the following format strings: diff --git a/obsidian-e2e.config.mjs b/obsidian-e2e.config.mjs index e77af3b3..62f22578 100644 --- a/obsidian-e2e.config.mjs +++ b/obsidian-e2e.config.mjs @@ -42,7 +42,7 @@ export default { skipBackwardLength: 15, skipForwardLength: 15, timestamp: { - template: "- {{time}} ", + template: "- {{linktime}} ", offset: 0, }, note: { diff --git a/src/constants.ts b/src/constants.ts index 5ac40069..19df3d81 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -78,7 +78,7 @@ export const DEFAULT_SETTINGS: IPodNotesSettings = { currentEpisode: undefined, timestamp: { - template: "- {{time}} ", + template: "- {{linktime}} ", offset: 0, }, diff --git a/src/main.persistence.test.ts b/src/main.persistence.test.ts index da9af010..f6b99c31 100644 --- a/src/main.persistence.test.ts +++ b/src/main.persistence.test.ts @@ -221,6 +221,22 @@ describe("PodNotes persistence integration", () => { ); }); + it("writes timestamp links as the capture default on a new install", async () => { + const saveData = vi.fn().mockResolvedValue(undefined); + const plugin = makePlugin(saveData); + Object.assign(plugin, { loadData: vi.fn().mockResolvedValue(null) }); + + await plugin.loadSettings(); + + expect(saveData).toHaveBeenCalledTimes(1); + expect(saveData).toHaveBeenCalledWith( + expect.objectContaining({ + schemaVersion: 2, + timestamp: { template: "- {{linktime}} ", offset: 0 }, + }), + ); + }); + it("does not rewrite data that is already schema v2", async () => { const saveData = vi.fn().mockResolvedValue(undefined); const plugin = makePlugin(saveData); diff --git a/src/persistence/dateCodec.test.ts b/src/persistence/dateCodec.test.ts index 7a3e195e..5f12cf95 100644 --- a/src/persistence/dateCodec.test.ts +++ b/src/persistence/dateCodec.test.ts @@ -19,8 +19,8 @@ describe("dateCodec", () => { expect(encodeDate("2024-03-01T11:05:03+01:00")).toBe(iso); }); - it.each([undefined, null, "", " ", "not-a-date", 1_709_290_000_000, {}, []])( - "rejects invalid or unsupported input %p without throwing", + it.each([[undefined], [null], [""], [" "], ["not-a-date"], [1_709_290_000_000], [{}], [[]]])( + "rejects invalid or unsupported input %o without throwing", (value) => { expect(decodeDate(value)).toBeUndefined(); expect(encodeDate(value)).toBeUndefined(); diff --git a/src/persistence/podNotesData.test.ts b/src/persistence/podNotesData.test.ts index a46a878f..f97b8cc1 100644 --- a/src/persistence/podNotesData.test.ts +++ b/src/persistence/podNotesData.test.ts @@ -294,11 +294,11 @@ describe("PodNotes data schema", () => { expect(({} as { polluted?: boolean }).polluted).toBeUndefined(); }); - it.each([[], "data", 42, true])("fails closed for malformed root %p", (value) => { + it.each([[[]], ["data"], [42], [true]])("fails closed for malformed root %o", (value) => { expect(() => decodePodNotesData(value)).toThrowError(PodNotesDataError); }); - it.each([0, -1, 1.5, "1", null])("fails closed for invalid schema version %p", (version) => { + it.each([0, -1, 1.5, "1", null])("fails closed for invalid schema version %o", (version) => { expect(() => decodePodNotesData({ schemaVersion: version })).toThrowError( /invalid schemaVersion/, ); @@ -310,3 +310,102 @@ describe("PodNotes data schema", () => { ); }); }); + +/** An existing listener's data.json, apart from the timestamp setting under test. */ +const listenerData = { + savedFeeds: { + "Test Podcast": { + title: "Test Podcast", + url: "https://example.com/feed.xml", + artworkUrl: "https://example.com/artwork.jpg", + }, + }, + podNotes: {}, + defaultPlaybackRate: 1.5, + defaultVolume: 0.8, + hidePlayedEpisodes: false, + episodeListLimit: 10, + playedEpisodes: { + "Test Podcast::Episode 1": { + title: "Episode 1", + podcastName: "Test Podcast", + time: 1830, + duration: 3600, + finished: false, + }, + }, + favorites: { + icon: "lucide-star", + name: "Favorites", + shouldEpisodeRemoveAfterPlay: false, + shouldRepeat: false, + episodes: [], + }, + queue: { + icon: "list-ordered", + name: "Queue", + shouldEpisodeRemoveAfterPlay: true, + shouldRepeat: false, + episodes: [], + }, + autoQueue: true, + playlists: {}, + skipBackwardLength: 15, + skipForwardLength: 30, + note: { path: "Podcasts/{{title}}.md", template: "# {{title}}\n\n{{description}}" }, + download: { path: "Podcasts/{{podcast}}/{{title}}" }, + downloadedEpisodes: {}, + localFiles: { + icon: "folder", + name: "Local Files", + shouldEpisodeRemoveAfterPlay: false, + shouldRepeat: false, + episodes: [], + }, +}; + +describe("timestamp template (#345)", () => { + it.each([null, undefined])("captures timestamps as links on a new install (%o)", (data) => { + const decoded = decodePodNotesData(data); + + expect(decoded.settings.timestamp.template).toBe("- {{linktime}} "); + expect(decoded.warnings).toEqual([]); + expect(decoded.changed).toBe(true); + }); + + it.each(["- {{time}} ", "* {{time:mm:ss}} - "])( + "keeps the saved template %o through load and save", + (template) => { + const decoded = decodePodNotesData({ + ...listenerData, + schemaVersion: 2, + openAISecretId: "", + deepgramSecretId: "", + timestamp: { template, offset: 0 }, + }); + const persisted = encodePodNotesData(decoded.settings, decoded.unknownFields); + + expect(decoded.warnings).toEqual([]); + expect(decoded.changed).toBe(false); + expect(decoded.settings.timestamp).toEqual({ template, offset: 0 }); + expect(persisted.timestamp).toEqual({ template, offset: 0 }); + }, + ); + + it.each([ + ["no timestamp setting", listenerData, 0, []], + ["only a timestamp offset", { ...listenerData, timestamp: { offset: 5 } }, 5, []], + [ + "a non-string template", + { ...listenerData, timestamp: { template: 42, offset: 0 } }, + 0, + ["timestamp.template: expected a string"], + ], + ])("keeps plain timestamps for older data with %s", (_case, data, offset, warnings) => { + const decoded = decodePodNotesData(data); + + expect(decoded.sourceVersion).toBe(0); + expect(decoded.settings.timestamp).toEqual({ template: "- {{time}} ", offset }); + expect(decoded.warnings).toEqual(warnings); + }); +}); diff --git a/src/persistence/podNotesData.ts b/src/persistence/podNotesData.ts index d57f7120..fa647d87 100644 --- a/src/persistence/podNotesData.ts +++ b/src/persistence/podNotesData.ts @@ -72,7 +72,8 @@ export type PersistedPodNotesDataV2 = PersistedPodNotesSettings & * does not understand. */ export function decodePodNotesData(value: unknown): DecodedPodNotesData { - if (value === null || value === undefined) value = {}; + // No data.json yet is a new install, which starts from the current defaults. + if (value === null || value === undefined) value = structuredClone(DEFAULT_SETTINGS); if (!isPlainObject(value)) { throw new PodNotesDataError( diff --git a/src/persistence/settingsCodec.ts b/src/persistence/settingsCodec.ts index 0378fd5f..33a6a0a2 100644 --- a/src/persistence/settingsCodec.ts +++ b/src/persistence/settingsCodec.ts @@ -131,17 +131,18 @@ function decodeSecretId( return ""; } +/** + * The timestamp template before `{{linktime}}` became the default (#345). + * PodNotes always writes the template, so data without one is older or + * hand-edited and keeps the template it ran with. + */ +const LEGACY_TIMESTAMP_TEMPLATE = "- {{time}} "; + function decodeTimestamp(value: unknown, warnings: Set): IPodNotesSettings["timestamp"] { const record = optionalRecord(value, warnings, "timestamp"); return { ...copySafeObject(record), - template: readString( - record, - "template", - DEFAULT_SETTINGS.timestamp.template, - warnings, - "timestamp", - ), + template: readString(record, "template", LEGACY_TIMESTAMP_TEMPLATE, warnings, "timestamp"), offset: readFiniteNumber( record, "offset", diff --git a/src/settingsMigrations.ts b/src/settingsMigrations.ts index f7182454..9fe9d370 100644 --- a/src/settingsMigrations.ts +++ b/src/settingsMigrations.ts @@ -3,14 +3,12 @@ import { DIARIZATION_PROVIDERS, type DiarizationProviderId } from "./services/di import type { IPodNotesSettings } from "./types/IPodNotesSettings"; /** - * Settings migrations applied when settings are loaded from disk. + * Settings migrations that the persistence codec + * (`src/persistence/settingsCodec.ts`) applies while decoding `data.json`. * - * These run in `PodNotes.loadSettings` after the persisted `data.json` is merged - * over `DEFAULT_SETTINGS`. Changing a default alone is NOT enough for existing - * users: the plugin persists the whole settings object, so anyone who has used - * PodNotes already has the old value written to `data.json`, which overrides the - * new default on load. A migration rewrites that stored value in memory so the - * change actually reaches existing users (it is re-persisted on the next save). + * PodNotes persists the whole settings object, so a changed default reaches only + * new installs. Existing users keep their stored value unless a migration + * rewrites it, which we do only when the old stored value is broken. * * This module is pure (no Obsidian/UI dependencies) so the risky rewrite logic is * unit-testable. @@ -108,9 +106,8 @@ type StoredTranscript = { /** * Backfill the transcript settings with the diarization defaults (issue #168). * - * `loadSettings` replaces the whole persisted `transcript` object, so an existing - * user who has `{ path, template }` saved would otherwise get an `undefined` - * `transcript.diarization` and crash where the service reads it. This deep-merges + * An existing user who has only `{ path, template }` saved would otherwise get an + * `undefined` `transcript.diarization` and crash where the service reads it. This deep-merges * the new nested default while preserving any path/template the user configured, * and clamps a malformed/unknown provider back to the default so a hand-edited or * corrupted `data.json` can't select a provider that does not exist. @@ -151,8 +148,7 @@ type StoredFeedNote = { path?: string | null; template?: string | null }; /** * Backfill the feed-note settings so a partially-persisted `feedNote` is repaired - * on load (issue ST-08). `loadSettings` shallow-merges the persisted object over - * the default, so an old `data.json` holding only `{ path }` (or a hand-edited one + * on load (issue ST-08). An old `data.json` holding only `{ path }` (or a hand-edited one * with a `null` field) would leave `template` undefined and crash where * createFeedNote calls `template.replace(...)`. This coalesces any missing/null/ * non-string field back to the default while preserving a deliberately empty diff --git a/src/utility/formatDate.test.ts b/src/utility/formatDate.test.ts index bd6ccbba..853c4bd7 100644 --- a/src/utility/formatDate.test.ts +++ b/src/utility/formatDate.test.ts @@ -21,7 +21,7 @@ describe("formatDate", () => { }); it.each([undefined, null, "", "not-a-date", new Date("invalid")])( - "returns an empty value for invalid input %p", + "returns an empty value for invalid input %o", (value) => { expect(formatDate(value, "YYYY-MM-DD")).toBe(""); }, diff --git a/src/utility/prepareTimestampInsertion.test.ts b/src/utility/prepareTimestampInsertion.test.ts index 808f03ee..bd189983 100644 --- a/src/utility/prepareTimestampInsertion.test.ts +++ b/src/utility/prepareTimestampInsertion.test.ts @@ -127,7 +127,7 @@ describe("prepareTimestampForInsertion", () => { expect(result).toBe("[1:23](x) \\| note more"); }); - it("leaves the default '- {{time}} ' style capture unchanged in a table", () => { + it("leaves a plain '- {{time}} ' capture unchanged in a table", () => { const { getLine, lineCount } = fromLines(TABLE); const result = prepareTimestampForInsertion("- 0:01:23 ", { getLine,