diff --git a/android/app/src/main/java/com/fabledsword/inkwell/ui/LinkPreviewRow.kt b/android/app/src/main/java/com/fabledsword/inkwell/ui/LinkPreviewRow.kt index 2ab833d..c639804 100644 --- a/android/app/src/main/java/com/fabledsword/inkwell/ui/LinkPreviewRow.kt +++ b/android/app/src/main/java/com/fabledsword/inkwell/ui/LinkPreviewRow.kt @@ -35,13 +35,26 @@ private val COMPACT_IMAGE_WIDTH = 48.dp /** * A URL that is the WHOLE body, whitespace either side allowed. * - * Mirrors `LONE_URL_RE` in `NoteCard.vue` deliberately — the two surfaces have to - * agree on what counts as "this note is a link", or the same note reads as a card - * on one and a paragraph on the other. Someone pasting a link rarely trims it, + * Mirrors `LONE_URL_RE` in the web's `notes/links.ts` deliberately — the two surfaces + * have to agree on what counts as "this note is a link", or the same note reads as a + * card on one and a paragraph on the other. Someone pasting a link rarely trims it, * which is why the surrounding whitespace is tolerated rather than rejected. */ private val LONE_URL = Regex("""^\s*(https?://[^\s<>"'\]\)]+)\s*$""") +/** The sentence's punctuation, not the link's: the server trims it before unfurling. */ +private val TRAILING_PUNCTUATION = charArrayOf('.', ',', ';', ':', '!', '?') + +/** + * The link a note that is nothing but a link files its preview under, or null. + * + * Trimmed the way the server's `unfurl_queue.detect_urls` trims it, since that is the + * key the preview arrives under (#5399). The web's `loneUrl` is the same rule, and + * both run the `urls` cases in `core/testdata/grammar.json` (here, by hand). + */ +fun loneUrl(body: String): String? = + LONE_URL.matchEntire(body)?.groupValues?.get(1)?.trimEnd(*TRAILING_PUNCTUATION)?.ifEmpty { null } + /** * The preview for a note that is nothing but a URL, or null. * @@ -56,13 +69,12 @@ private val LONE_URL = Regex("""^\s*(https?://[^\s<>"'\]\)]+)\s*$""") * the URL does. */ fun loneUrlPreview(note: Note): LinkPreview? { - if (!LONE_URL.matches(note.body)) return null - val url = note.body.trim() + val url = loneUrl(note.body) ?: return null return note.previews.firstOrNull { it.url == url } } /** True when the body is a lone URL, whether or not a preview has arrived for it. */ -fun isLoneUrl(note: Note): Boolean = LONE_URL.matches(note.body) +fun isLoneUrl(note: Note): Boolean = loneUrl(note.body) != null /** * A fetched link preview, in one of two sizes. diff --git a/android/app/src/test/java/com/fabledsword/inkwell/ui/LoneUrlTest.kt b/android/app/src/test/java/com/fabledsword/inkwell/ui/LoneUrlTest.kt new file mode 100644 index 0000000..7881628 --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/inkwell/ui/LoneUrlTest.kt @@ -0,0 +1,33 @@ +package com.fabledsword.inkwell.ui + +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * Pins [loneUrl] against the `urls` cases in `core/testdata/grammar.json`, written out + * by hand as [DerivedTintTest] does the tint: these JVM tests do not read the fixture. + * The web runs the same cases in `notes/grammar.test.ts`, the server in + * `tests/test_grammar_fixture.py`, so a change to the rule changes the fixture and + * this file together, or a suite goes red. + */ +class LoneUrlTest { + @Test + fun `lone links match the fixture shared with the web and the server`() { + val cases = + listOf( + "https://example.com/a" to "https://example.com/a", + " https://example.com/a \n" to "https://example.com/a", + "https://example.com/a." to "https://example.com/a", + "https://example.com/a?q=1!" to "https://example.com/a?q=1", + "see https://example.com/a, then https://example.com/b." to null, + "" to null, + "https://example.com/a)" to null, + "https://example.com/a https://example.com/a" to null, + "ftp://example.com/a" to null, + "" to null, + ) + for ((body, lone) in cases) { + assertEquals("body [$body]", lone, loneUrl(body)) + } + } +} diff --git a/core/testdata/grammar.json b/core/testdata/grammar.json index c0173ee..37679db 100644 --- a/core/testdata/grammar.json +++ b/core/testdata/grammar.json @@ -5,6 +5,7 @@ "A difference between any two implementations is a note that changes shape when it syncs. Add a case here, not to one suite.", "Lives under core/ because core is the definition, and because core/ is in the desktop, Android and server file sets, so changing a case reruns every suite that reads it.", "display_titles: a note's name is its first line that says anything once a task marker is stripped, cut to 200 characters (not bytes). Lines split on \\n alone, as the grammar does everywhere.", + "urls: the links in a body the server unfurls (found), and the one a note that is nothing but a link files its preview under (lone). Trailing .,;:!? is the sentence's, not the link's, on every surface.", "recurrences: the repeat rules a reminder can carry, as stored. The server, the core and the web's editor each hold the list; Android's picker pins it by hand, as it does the tint.", "Tag cases are plain text on purpose. The web's renderer lets `code` and **bold** claim a `#` first, which the derivers do not; that is a rendering choice, not grammar, and is tested where it lives." ], @@ -82,6 +83,19 @@ { "body": "ééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééé", "title": "éééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééé" } ], + "urls": [ + { "body": "https://example.com/a", "found": ["https://example.com/a"], "lone": "https://example.com/a" }, + { "body": " https://example.com/a \n", "found": ["https://example.com/a"], "lone": "https://example.com/a" }, + { "body": "https://example.com/a.", "found": ["https://example.com/a"], "lone": "https://example.com/a" }, + { "body": "https://example.com/a?q=1!", "found": ["https://example.com/a?q=1"], "lone": "https://example.com/a?q=1" }, + { "body": "see https://example.com/a, then https://example.com/b.", "found": ["https://example.com/a", "https://example.com/b"], "lone": null }, + { "body": "", "found": ["https://example.com/a"], "lone": null }, + { "body": "https://example.com/a)", "found": ["https://example.com/a"], "lone": null }, + { "body": "https://example.com/a https://example.com/a", "found": ["https://example.com/a"], "lone": null }, + { "body": "ftp://example.com/a", "found": [], "lone": null }, + { "body": "", "found": [], "lone": null } + ], + "recurrences": ["daily", "weekly", "monthly", "yearly"], "tint": { diff --git a/frontend/src/components/NoteCard.vue b/frontend/src/components/NoteCard.vue index 630e85c..a396c0d 100644 --- a/frontend/src/components/NoteCard.vue +++ b/frontend/src/components/NoteCard.vue @@ -3,6 +3,7 @@ import { computed, ref, watch } from "vue"; import { useNotesStore } from "../stores/notes"; import { NOTE_CARD_SURFACE, labelChipClasses } from "../notes/colors"; import { rendersInline } from "../notes/attachments"; +import { loneUrl } from "../notes/links"; import type { Note } from "../stores/notes"; import Icon from "./Icon.vue"; import LinkPreview from "./LinkPreview.vue"; @@ -84,18 +85,14 @@ const PREVIEW_LINES = 8; // A note that is NOTHING but a URL is a link, and its preview is the whole card — // showing the raw URL underneath a card that already says where it goes is saying the // same thing twice, badly. A URL mentioned *inside* a note is a footnote to it, and -// gets a compact strip at the bottom instead. -// -// Whitespace either side still counts as lone: someone pasting a link rarely trims it. -const LONE_URL_RE = /^\s*(https?:\/\/[^\s<>"'\])]+)\s*$/; - -const isLoneUrl = computed(() => LONE_URL_RE.test(props.note.body) && !props.note.items.length); +// gets a compact strip at the bottom instead. What counts as lone is notes/links.ts. +const isLoneUrl = computed(() => loneUrl(props.note.body) !== null && !props.note.items.length); /** The preview for a lone-URL note — null while it is still being fetched, or if it * could never be fetched at all. */ const loneUrlPreview = computed(() => { if (!isLoneUrl.value) return null; - const url = props.note.body.trim(); + const url = loneUrl(props.note.body); return props.note.previews.find((p) => p.url === url) ?? null; }); diff --git a/frontend/src/notes/grammar.test.ts b/frontend/src/notes/grammar.test.ts index 0cbe36b..a4baa90 100644 --- a/frontend/src/notes/grammar.test.ts +++ b/frontend/src/notes/grammar.test.ts @@ -8,6 +8,7 @@ import { describe, expect, it } from "vitest"; import fixture from "../../../core/testdata/grammar.json"; import { DERIVED_TINT_KEYS, derivedTint, resolveLabelColor, tintHash } from "./colors"; import { parseInline, parseTaskLine, renderTaskLine } from "./markdown"; +import { loneUrl } from "./links"; import { RECURRENCES } from "./recurrence"; describe("checklist lines", () => { @@ -59,3 +60,9 @@ describe("recurrences", () => { expect(RECURRENCES.map((r) => r.value)).toEqual(fixture.recurrences); }); }); + +describe("links", () => { + it.each(fixture.urls)("lone link in $body", ({ body, lone }) => { + expect(loneUrl(body)).toBe(lone); + }); +}); diff --git a/frontend/src/notes/links.ts b/frontend/src/notes/links.ts new file mode 100644 index 0000000..f55b43d --- /dev/null +++ b/frontend/src/notes/links.ts @@ -0,0 +1,17 @@ +// A note that is nothing but a link renders as a link card. +// +// The characters a link runs over, and the punctuation trimmed off its end, are the +// server's (unfurl_queue.detect_urls): the server files the preview under the trimmed +// link, so the card has to look it up under the same one (#5399). Android's +// LinkPreviewRow holds the same rule, and all three run the `urls` cases in +// core/testdata/grammar.json. +// +// Whitespace either side still counts as lone: someone pasting a link rarely trims it. +const LONE_URL_RE = /^\s*(https?:\/\/[^\s<>"'\])]+)\s*$/; + +/** The link a lone-link note's preview is filed under, or null for any other body. */ +export function loneUrl(body: string): string | null { + const match = LONE_URL_RE.exec(body); + if (!match) return null; + return match[1].replace(/[.,;:!?]+$/, "") || null; +} diff --git a/tests/test_grammar_fixture.py b/tests/test_grammar_fixture.py index 89091ef..69b3d48 100644 --- a/tests/test_grammar_fixture.py +++ b/tests/test_grammar_fixture.py @@ -17,6 +17,7 @@ from inkwell.notes.checklist import parse_items, render_item from inkwell.notes.helpers import derive_display_title from inkwell.notes.recurrence import REMINDER_RECURRENCES from inkwell.notes.tags import parse_tags, split_body_tags +from inkwell.unfurl_queue import detect_urls FIXTURE = json.loads( (Path(__file__).resolve().parents[1] / "core" / "testdata" / "grammar.json").read_text(encoding="utf-8") @@ -59,3 +60,8 @@ def test_palette_keys(): """The palette the server accepts is the fixture's hues plus `default`, the key unrecognised input lands on. The web checks the same hues (DERIVED_TINT_KEYS).""" assert NOTE_COLORS == {"default", *FIXTURE["tint"]["keys"]} + + +@pytest.mark.parametrize("case", FIXTURE["urls"], ids=lambda c: repr(c["body"])) +def test_urls(case): + assert detect_urls(case["body"]) == case["found"]