From db9e9a2dfbd3b537ef7d503174e7d1f403a94e06 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 14:27:12 -0400 Subject: [PATCH] A note's name is one rule, pinned by the shared fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server and the core each derived display_title and disagreed twice: the server cut it at 200 characters and the core didn't, and the server split lines with splitlines(), which also breaks on a lone \r or a U+2028, where the core and every other reading of the grammar split on \n alone. - grammar.json gains a display_titles section: blank lines, markers, an empty item, \r\n, a lone \r, U+2028, and a 201-character line of 'é' (the cut is characters, not bytes). - derive::display_title and DISPLAY_TITLE_CAP are the core's half, moved next to strip_marker. The server splits on "\n". Both suites run the cases. Behaviour: a device now names a note with a first line over 200 characters the way the web always has, and the server names a note containing a lone \r or a U+2028 the way devices always have. Fixes #5398. DRY pass #2, batch 3 (#5372). Co-Authored-By: Claude Opus 5.5 --- core/src/local/derive.rs | 35 +++++++++++++++++++++++++++++++++++ core/src/local/store.rs | 25 ++----------------------- core/testdata/grammar.json | 15 ++++++++++++++- src/inkwell/notes/helpers.py | 9 ++++++--- tests/test_grammar_fixture.py | 6 ++++++ 5 files changed, 63 insertions(+), 27 deletions(-) diff --git a/core/src/local/derive.rs b/core/src/local/derive.rs index 70ce99f..549f975 100644 --- a/core/src/local/derive.rs +++ b/core/src/local/derive.rs @@ -341,6 +341,32 @@ fn render_task_line(indent: &str, bullet: char, checked: bool, text: &str) -> St } } +/// The longest a note's name runs, in characters (not bytes): the server's +/// `DISPLAY_TITLE_CAP`. +pub const DISPLAY_TITLE_CAP: usize = 200; + +/// The note's NAME: the first line of its body that says anything, cut to +/// [`DISPLAY_TITLE_CAP`] characters. +/// +/// Mirrors `derive_display_title` in the server's notes/helpers.py — one rule written +/// twice, and they have to agree or a synced note is called different things on either +/// side of the wire. Both run the `display_titles` cases in testdata/grammar.json. +/// +/// It no longer needs the items, because the items ARE lines of the body now (M304). +/// What it needs instead is to strip the task marker off: a list-only note is still +/// named by its first item, and calling that note "- [ ] milk" would be showing +/// someone the storage rather than the note. An empty item is skipped rather than +/// naming the note "", which is what a half-typed list would otherwise do. +pub fn display_title(body: &str) -> String { + for line in body.split('\n') { + let text = strip_marker(line.trim()).trim(); + if !text.is_empty() { + return text.chars().take(DISPLAY_TITLE_CAP).collect(); + } + } + String::new() +} + /// The text of a line with its task marker removed, or the line as it was. /// /// For naming a note: a list-only note is named by its first item, and calling one @@ -759,6 +785,15 @@ mod tests { } } + #[test] + fn fixture_display_titles() { + for case in fixture()["display_titles"].as_array().unwrap() { + let body = case["body"].as_str().unwrap(); + let want = case["title"].as_str().unwrap(); + assert_eq!(display_title(body), want, "body {body:?}"); + } + } + #[test] fn fixture_lifts() { for case in fixture()["lifts"].as_array().unwrap() { diff --git a/core/src/local/store.rs b/core/src/local/store.rs index b937387..d425d0f 100644 --- a/core/src/local/store.rs +++ b/core/src/local/store.rs @@ -21,27 +21,6 @@ fn new_id() -> String { Uuid::new_v4().to_string() } -/// The note's NAME: the first line of its body that says anything. -/// -/// Mirrors `derive_display_title` in the server's notes/helpers.py — one rule written -/// twice, and they have to agree or a synced note is called different things on either -/// side of the wire. -/// -/// It no longer needs the items, because the items ARE lines of the body now (M304). -/// What it needs instead is to strip the task marker off: a list-only note is still -/// named by its first item, and calling that note "- [ ] milk" would be showing -/// someone the storage rather than the note. An empty item is skipped rather than -/// naming the note "", which is what a half-typed list would otherwise do. -fn display_title(body: &str) -> String { - for line in body.lines() { - let text = derive::strip_marker(line.trim()).trim(); - if !text.is_empty() { - return text.to_string(); - } - } - String::new() -} - fn escape_like(s: &str) -> String { s.replace('\\', "\\\\") .replace('%', "\\%") @@ -183,7 +162,7 @@ fn load_note(conn: &Connection, id: &str) -> rusqlite::Result { note.items = items_of(¬e.body); note.attachments = load_attachments(conn, id)?; note.previews = load_previews(conn, id)?; - note.display_title = display_title(¬e.body); + note.display_title = derive::display_title(¬e.body); Ok(note) } @@ -294,7 +273,7 @@ fn find_or_create_label(conn: &Connection, name: &str) -> rusqlite::Result str: would show someone the storage instead of the note — and skipping an EMPTY item, so a half-typed list does not leave a note with no name. - Mirrors `display_title` in core/src/local/store.rs. Deterministic — a literal first - line, never generated. + Lines split on "\n" alone, as every other reading of the grammar does; + `splitlines` also broke on a lone "\r" or a U+2028, so a phone and the server named + the same note differently. Mirrors `derive::display_title` in the core, and both + run the `display_titles` cases in core/testdata/grammar.json. Deterministic — a + literal first line, never generated. """ - for line in (body or "").splitlines(): + for line in (body or "").split("\n"): stripped = strip_marker(line.strip()).strip() if stripped: return stripped[:DISPLAY_TITLE_CAP] diff --git a/tests/test_grammar_fixture.py b/tests/test_grammar_fixture.py index a4ee6e7..dd3c23a 100644 --- a/tests/test_grammar_fixture.py +++ b/tests/test_grammar_fixture.py @@ -13,6 +13,7 @@ from pathlib import Path import pytest from inkwell.notes.checklist import parse_items, render_item +from inkwell.notes.helpers import derive_display_title from inkwell.notes.tags import parse_tags, split_body_tags FIXTURE = json.loads( @@ -41,3 +42,8 @@ def test_tags(case): def test_lifts(case): standalone, inline, lifted = split_body_tags(case["body"]) assert (standalone, inline, lifted) == (case["standalone"], case["inline"], case["lifted"]) + + +@pytest.mark.parametrize("case", FIXTURE["display_titles"], ids=lambda c: repr(c["body"][:40])) +def test_display_titles(case): + assert derive_display_title(case["body"]) == case["title"]