A note's name is one rule, pinned by the shared fixture
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
/// 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
|
/// 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]
|
#[test]
|
||||||
fn fixture_lifts() {
|
fn fixture_lifts() {
|
||||||
for case in fixture()["lifts"].as_array().unwrap() {
|
for case in fixture()["lifts"].as_array().unwrap() {
|
||||||
|
|||||||
+2
-23
@@ -21,27 +21,6 @@ fn new_id() -> String {
|
|||||||
Uuid::new_v4().to_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 {
|
fn escape_like(s: &str) -> String {
|
||||||
s.replace('\\', "\\\\")
|
s.replace('\\', "\\\\")
|
||||||
.replace('%', "\\%")
|
.replace('%', "\\%")
|
||||||
@@ -183,7 +162,7 @@ fn load_note(conn: &Connection, id: &str) -> rusqlite::Result<Note> {
|
|||||||
note.items = items_of(¬e.body);
|
note.items = items_of(¬e.body);
|
||||||
note.attachments = load_attachments(conn, id)?;
|
note.attachments = load_attachments(conn, id)?;
|
||||||
note.previews = load_previews(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)
|
Ok(note)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -294,7 +273,7 @@ fn find_or_create_label(conn: &Connection, name: &str) -> rusqlite::Result<Strin
|
|||||||
/// so this overwrites what they wrote, on purpose.
|
/// so this overwrites what they wrote, on purpose.
|
||||||
///
|
///
|
||||||
/// `display_title` needs no attention here, unlike on the server: the core derives it
|
/// `display_title` needs no attention here, unlike on the server: the core derives it
|
||||||
/// on READ (see `display_title` above, called from `load_note`) rather than storing
|
/// on READ (`derive::display_title`, called from `load_note`) rather than storing
|
||||||
/// it, so there is no persisted copy to go stale.
|
/// it, so there is no persisted copy to go stale.
|
||||||
///
|
///
|
||||||
/// The two kinds of tag are handled differently, and that difference IS what `via_tag`
|
/// The two kinds of tag are handled differently, and that difference IS what `via_tag`
|
||||||
|
|||||||
Vendored
+14
-1
@@ -1,9 +1,10 @@
|
|||||||
{
|
{
|
||||||
"_about": [
|
"_about": [
|
||||||
"The cases every implementation of the note grammar is tested against.",
|
"The cases every implementation of the note grammar is tested against.",
|
||||||
"One file, read by four test suites: core (derive.rs), the server (checklist.py, tags.py), the web (markdown.ts, colors.ts) and, for the tint, Android (DerivedTint.kt, which pins the same values by hand).",
|
"One file, read by four test suites: core (derive.rs), the server (checklist.py, tags.py, helpers.py), the web (markdown.ts, colors.ts) and, for the tint, Android (DerivedTint.kt, which pins the same values by hand).",
|
||||||
"A difference between any two implementations is a note that changes shape when it syncs. Add a case here, not to one suite.",
|
"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.",
|
"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.",
|
||||||
"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."
|
"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."
|
||||||
],
|
],
|
||||||
|
|
||||||
@@ -68,6 +69,18 @@
|
|||||||
{ "body": "(#todo)\nnotes", "standalone": [], "inline": [], "lifted": "(#todo)\nnotes" }
|
{ "body": "(#todo)\nnotes", "standalone": [], "inline": [], "lifted": "(#todo)\nnotes" }
|
||||||
],
|
],
|
||||||
|
|
||||||
|
"display_titles": [
|
||||||
|
{ "body": "", "title": "" },
|
||||||
|
{ "body": "\n\n Shopping \nmilk", "title": "Shopping" },
|
||||||
|
{ "body": "- [ ] milk\n- [x] eggs", "title": "milk" },
|
||||||
|
{ "body": "- [ ]\n- [ ] bread", "title": "bread" },
|
||||||
|
{ "body": " \n\t\nlate start", "title": "late start" },
|
||||||
|
{ "body": "one\r\ntwo", "title": "one" },
|
||||||
|
{ "body": "a\rb\nc", "title": "a\rb" },
|
||||||
|
{ "body": "a\u2028b\nc", "title": "a\u2028b" },
|
||||||
|
{ "body": "ééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééé", "title": "éééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééééé" }
|
||||||
|
],
|
||||||
|
|
||||||
"tint": {
|
"tint": {
|
||||||
"keys": ["red", "orange", "yellow", "green", "teal", "blue", "purple", "pink", "gray"],
|
"keys": ["red", "orange", "yellow", "green", "teal", "blue", "purple", "pink", "gray"],
|
||||||
"hashes": [
|
"hashes": [
|
||||||
|
|||||||
@@ -43,10 +43,13 @@ def derive_display_title(body: str | None) -> str:
|
|||||||
would show someone the storage instead of the note — and skipping an EMPTY item, so
|
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.
|
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
|
Lines split on "\n" alone, as every other reading of the grammar does;
|
||||||
line, never generated.
|
`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()
|
stripped = strip_marker(line.strip()).strip()
|
||||||
if stripped:
|
if stripped:
|
||||||
return stripped[:DISPLAY_TITLE_CAP]
|
return stripped[:DISPLAY_TITLE_CAP]
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ from pathlib import Path
|
|||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from inkwell.notes.checklist import parse_items, render_item
|
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
|
from inkwell.notes.tags import parse_tags, split_body_tags
|
||||||
|
|
||||||
FIXTURE = json.loads(
|
FIXTURE = json.loads(
|
||||||
@@ -41,3 +42,8 @@ def test_tags(case):
|
|||||||
def test_lifts(case):
|
def test_lifts(case):
|
||||||
standalone, inline, lifted = split_body_tags(case["body"])
|
standalone, inline, lifted = split_body_tags(case["body"])
|
||||||
assert (standalone, inline, lifted) == (case["standalone"], case["inline"], case["lifted"])
|
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"]
|
||||||
|
|||||||
Reference in New Issue
Block a user