From f71802184a02b9e16531e5d8bf936800d3ec857b Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Tue, 16 Jun 2026 16:38:23 +0200 Subject: [PATCH] fix(timestamps): capture into the cursor's table cell without breaking the row (#165) Capture Timestamp derived a position with getCursor(), wrote it with replaceRange, then re-placed the caret with a hand-computed setCursor. Inside an Obsidian Live Preview table cell that pattern fights the table-editing widget, so the timestamp landed in the wrong cell and the cursor scattered. Raw pipes or newlines in the capture also broke the row. Insert with editor.replaceSelection so CodeMirror owns caret placement, and, when the cursor is inside a table (including one nested in a callout or blockquote), escape pipes and collapse newlines via prepareTimestampForInsertion so the row stays intact. Outside a table the capture is inserted verbatim. Closes #165 --- docs/docs/timestamps.md | 2 + src/main.ts | 18 ++- src/utility/prepareTimestampInsertion.test.ts | 138 ++++++++++++++++++ src/utility/prepareTimestampInsertion.ts | 104 +++++++++++++ 4 files changed, 259 insertions(+), 3 deletions(-) create mode 100644 src/utility/prepareTimestampInsertion.test.ts create mode 100644 src/utility/prepareTimestampInsertion.ts diff --git a/docs/docs/timestamps.md b/docs/docs/timestamps.md index 8ee62c87..fd8e4cbc 100644 --- a/docs/docs/timestamps.md +++ b/docs/docs/timestamps.md @@ -16,6 +16,8 @@ For example, you might use `{{time:H\h mm\m ss\s}}` to get the time in the forma ## Capturing timestamps You can use the `Capture Timestamp` command by using the `PodNotes: Capture Timestamp` command in the command palette. +The timestamp is inserted at your cursor. When the cursor is inside a markdown table cell, the captured text stays on that row: any pipes are escaped and newlines are collapsed to spaces so the table is not broken. + **On desktop**, it is possible to bind this command to a hotkey, which makes it faster to use while writing. You can bind hotkeys in the `Hotkeys` tab of the Obsidian settings. diff --git a/src/main.ts b/src/main.ts index 711f6f9f..04a7ed0c 100644 --- a/src/main.ts +++ b/src/main.ts @@ -39,6 +39,7 @@ import type { Episode } from "./types/Episode"; import CurrentEpisodeController from "./store_controllers/CurrentEpisodeController"; import { HidePlayedEpisodesController } from "./store_controllers/HidePlayedEpisodesController"; import { TimestampTemplateEngine } from "./TemplateEngine"; +import { prepareTimestampForInsertion } from "./utility/prepareTimestampInsertion"; import createPodcastNote from "./createPodcastNote"; import createFeedNote from "./createFeedNote"; import { FeedSuggestModal, orderFeedsByCurrent } from "./ui/FeedSuggestModal"; @@ -273,13 +274,24 @@ export default class PodNotes extends Plugin implements IPodNotes { return !!this.api.podcast && !!this.settings.timestamp.template; } - const cursorPos = editor.getCursor(); const capture = TimestampTemplateEngine( this.settings.timestamp.template, ); - editor.replaceRange(capture, cursorPos); - editor.setCursor(cursorPos.line, cursorPos.ch + capture.length); + // Insert with replaceSelection (not getCursor + replaceRange + + // setCursor): it drops the text at the live cursor and lets the + // editor place the caret after it, which is reliable inside Live + // Preview table cells where hand-computed positions land in the + // wrong cell. Inside a table the capture is escaped so pipes and + // newlines don't break the row. See issue #165. + const cursor = editor.getCursor("from"); + const textToInsert = prepareTimestampForInsertion(capture, { + getLine: (line) => editor.getLine(line), + lineCount: editor.lineCount(), + cursorLine: cursor.line, + }); + + editor.replaceSelection(textToInsert); }, }); diff --git a/src/utility/prepareTimestampInsertion.test.ts b/src/utility/prepareTimestampInsertion.test.ts new file mode 100644 index 00000000..40a8441e --- /dev/null +++ b/src/utility/prepareTimestampInsertion.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, it } from "vitest"; +import { + escapeForTableCell, + isInsideTable, + isTableDelimiterRow, + prepareTimestampForInsertion, +} from "./prepareTimestampInsertion"; + +// Build a line-accessor pair (getLine, lineCount) over an array of lines, the +// shape the editor exposes, so detection can be exercised without a real editor. +function fromLines(lines: string[]): { + getLine: (line: number) => string; + lineCount: number; +} { + return { getLine: (line: number) => lines[line] ?? "", lineCount: lines.length }; +} + +const TABLE = [ + "| Time | Note |", + "| ---- | ---- |", + "| 0:00 | intro |", + "| 1:23 | topic |", +]; + +describe("isTableDelimiterRow", () => { + it("recognises plain, aligned, and tight delimiter rows", () => { + expect(isTableDelimiterRow("| --- | --- |")).toBe(true); + expect(isTableDelimiterRow("| :--- | ---: | :--: |")).toBe(true); + expect(isTableDelimiterRow("|---|---|")).toBe(true); + expect(isTableDelimiterRow(" | ---- | ---- | ")).toBe(true); + }); + + it("rejects content rows and pipe-free lines", () => { + expect(isTableDelimiterRow("| Time | Note |")).toBe(false); + expect(isTableDelimiterRow("| 0:00 | intro |")).toBe(false); + // A bare thematic break / setext underline has no pipe and must not count. + expect(isTableDelimiterRow("---")).toBe(false); + expect(isTableDelimiterRow("")).toBe(false); + }); + + it("recognises a delimiter row nested in a blockquote or callout", () => { + expect(isTableDelimiterRow("> | ---- | ---- |")).toBe(true); + expect(isTableDelimiterRow(">| ---- | ---- |")).toBe(true); + expect(isTableDelimiterRow("> > | --- | --- |")).toBe(true); + }); +}); + +describe("isInsideTable", () => { + it("is true on the header, delimiter, and body rows", () => { + const { getLine, lineCount } = fromLines(TABLE); + expect(isInsideTable(getLine, lineCount, 0)).toBe(true); + expect(isInsideTable(getLine, lineCount, 1)).toBe(true); + expect(isInsideTable(getLine, lineCount, 2)).toBe(true); + expect(isInsideTable(getLine, lineCount, 3)).toBe(true); + }); + + it("is false in ordinary prose, even when the line contains a pipe", () => { + const lines = ["Some prose here.", "a | b is not a table", "More prose."]; + const { getLine, lineCount } = fromLines(lines); + expect(isInsideTable(getLine, lineCount, 0)).toBe(false); + expect(isInsideTable(getLine, lineCount, 1)).toBe(false); + expect(isInsideTable(getLine, lineCount, 2)).toBe(false); + }); + + it("is false on a blank line separating a table from following text", () => { + const lines = [...TABLE, "", "After the table."]; + const { getLine, lineCount } = fromLines(lines); + expect(isInsideTable(getLine, lineCount, 4)).toBe(false); + expect(isInsideTable(getLine, lineCount, 5)).toBe(false); + }); + + it("detects a table nested inside a callout/blockquote", () => { + const lines = [ + "> [!note] Timestamps", + "> | Time | Note |", + "> | ---- | ----- |", + "> | 0:00 | intro |", + ]; + const { getLine, lineCount } = fromLines(lines); + expect(isInsideTable(getLine, lineCount, 1)).toBe(true); + expect(isInsideTable(getLine, lineCount, 3)).toBe(true); + }); +}); + +describe("escapeForTableCell", () => { + it("escapes unescaped pipes so they stay textual in a cell", () => { + expect(escapeForTableCell("a | b")).toBe("a \\| b"); + }); + + it("does not double-escape an already-escaped pipe", () => { + expect(escapeForTableCell("a \\| b")).toBe("a \\| b"); + }); + + it("collapses newlines (LF, CRLF, CR) to single spaces", () => { + expect(escapeForTableCell("line1\nline2")).toBe("line1 line2"); + expect(escapeForTableCell("line1\r\nline2")).toBe("line1 line2"); + expect(escapeForTableCell("line1\rline2")).toBe("line1 line2"); + }); + + it("leaves a plain timestamp link untouched", () => { + const link = "[1:23](obsidian://podnotes?episodeName=Show&time=83)"; + expect(escapeForTableCell(link)).toBe(link); + }); +}); + +describe("prepareTimestampForInsertion", () => { + it("escapes a pipe/newline capture when the cursor is in a table cell", () => { + const { getLine, lineCount } = fromLines(TABLE); + const result = prepareTimestampForInsertion("[1:23](x) | note\nmore", { + getLine, + lineCount, + cursorLine: 2, + }); + expect(result).toBe("[1:23](x) \\| note more"); + }); + + it("leaves the default '- {{time}} ' style capture unchanged in a table", () => { + const { getLine, lineCount } = fromLines(TABLE); + const result = prepareTimestampForInsertion("- 0:01:23 ", { + getLine, + lineCount, + cursorLine: 2, + }); + expect(result).toBe("- 0:01:23 "); + }); + + it("never escapes outside a table", () => { + const lines = ["Notes:", ""]; + const { getLine, lineCount } = fromLines(lines); + const capture = "[1:23](x) | note\nmore"; + const result = prepareTimestampForInsertion(capture, { + getLine, + lineCount, + cursorLine: 0, + }); + expect(result).toBe(capture); + }); +}); diff --git a/src/utility/prepareTimestampInsertion.ts b/src/utility/prepareTimestampInsertion.ts new file mode 100644 index 00000000..e15b5437 --- /dev/null +++ b/src/utility/prepareTimestampInsertion.ts @@ -0,0 +1,104 @@ +// Helpers for inserting a captured timestamp at the editor cursor without +// breaking the markdown around it. See issue #165: capturing a timestamp into a +// markdown table cell would land the text in the wrong cell, scatter the cursor, +// or break the row. The root causes were (1) deriving a position with +// `getCursor()` and feeding it to `replaceRange` + a hand-computed `setCursor`, +// which fights Obsidian's Live Preview table-editing widget, and (2) inserting +// raw `|`/newline characters that are structural inside a table. +// +// The command now inserts with `editor.replaceSelection` (which lets CodeMirror +// own cursor placement) and, when the cursor sits inside a table, runs the +// capture through `escapeForTableCell` so pipes and newlines stay textual. + +/** + * Strip a leading blockquote/callout marker chain (`>`, `> >`, ...) so a table + * nested inside a callout or blockquote is detected the same as a top-level one. + * Obsidian renders `> | a | b |` as a table inside the callout, so for cell + * detection the `>` prefix is not part of the row. + */ +function stripBlockquotePrefix(line: string): string { + return line.replace(/^\s*(?:>\s?)+/, ""); +} + +/** Does the line contain a table cell pipe, ignoring any blockquote prefix? */ +function lineHasCellPipe(line: string): boolean { + return stripBlockquotePrefix(line).includes("|"); +} + +/** + * Is `line` a GFM table delimiter row (e.g. `| --- | :--: |`)? A delimiter row + * is what distinguishes a real table from an ordinary line that merely contains + * pipes, so it is the signal we key table detection on. Requires at least one + * pipe so a bare `---` thematic break / setext underline is not misread. A + * leading blockquote/callout marker is ignored so nested tables still match. + */ +export function isTableDelimiterRow(line: string): boolean { + const trimmed = stripBlockquotePrefix(line).trim(); + if (!trimmed.includes("|")) return false; + + const cells = trimmed + .replace(/^\|/, "") + .replace(/\|$/, "") + .split("|"); + + return cells.length > 0 && cells.every((cell) => /^\s*:?-+:?\s*$/.test(cell)); +} + +/** + * Is the cursor inside a markdown table? True when the cursor line contains a + * pipe and the contiguous block of pipe-bearing lines around it includes a + * delimiter row. Scanning the block (rather than just an adjacent line) keeps + * detection correct whether the cursor is on the header, the delimiter, or any + * body row, while the "must contain a pipe" gate avoids escaping ordinary prose. + * Blockquote/callout markers are ignored so tables nested in a callout match. + */ +export function isInsideTable( + getLine: (line: number) => string, + lineCount: number, + cursorLine: number, +): boolean { + const current = getLine(cursorLine); + if (!current || !lineHasCellPipe(current)) return false; + + for (let i = cursorLine; i >= 0; i--) { + const line = getLine(i); + if (!line || !lineHasCellPipe(line)) break; + if (isTableDelimiterRow(line)) return true; + } + + for (let i = cursorLine; i < lineCount; i++) { + const line = getLine(i); + if (!line || !lineHasCellPipe(line)) break; + if (isTableDelimiterRow(line)) return true; + } + + return false; +} + +/** + * Make `text` safe to drop into a single table cell: collapse newlines to a + * space (a raw newline would end the row) and escape any unescaped pipe (a raw + * pipe would open a new column). Already-escaped pipes (`\|`) are left alone so + * the cell is never double-escaped. + */ +export function escapeForTableCell(text: string): string { + return text.replace(/\r\n?|\n/g, " ").replace(/(? string; + lineCount: number; + cursorLine: number; + }, +): string { + return isInsideTable(context.getLine, context.lineCount, context.cursorLine) + ? escapeForTableCell(capture) + : capture; +}