A rewrite replaces the line it names, on both sides of the wire
Editing a message left the original where it was and put the new wording underneath it, because neither half of an edit was connected. Outgoing, `sendDraft` sent the tagged PRIVMSG and then fell through to the local echo every other message takes, so the rewrite arrived as a second line. It also sent the edit unsigned, which freeq refuses from an account — so the server kept the original too, and the one thing that did change was the screen, wrongly. Incoming, `applyEdit` was written, documented and called from nowhere: a revision came in as an ordinary PRIVMSG and was appended. It is now folded into the line it names, under that line's own id, ahead of the echo check — our own edit comes back the same road. An edit whose original is older than the backlog is still shown, as a line of its own, rather than dropped; and a replayed message carrying `+freeq.at/edited` arrives marked edited, which is the only trace a replay leaves. `edits.nim` had no tests. It has them now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
31683ce parent: 20b5f50 modified
nim/src/frq/reducer.nim +44 -2 | @@ -400,9 +400,24 @@ proc sendDraft() = | ||
| 400 | 400 | if app.editing.has: |
| 401 | 401 | # An edit is a fresh PRIVMSG tagged with what it replaces; the server |
| 402 | 402 | # rewrites the original and echoes the revision back. |
| 403 | - send("@+draft/edit=" & app.editing.id & " PRIVMSG " & app.current & | |
| 404 | - " :" & line) | |
| 403 | + # | |
| 404 | + # Signed like every other change to a record already written: freeq keeps | |
| 405 | + # the original and says nothing an unsigned edit from an account asked | |
| 406 | + # for, so the line would simply never change. | |
| 407 | + let mid = app.editing.id | |
| 408 | + var tags = "+draft/edit=" & mid | |
| 409 | + for k, v in editTags(app.current, mid, line, "", | |
| 410 | + peerDid(app.currentRoom, app.formNick), nowMs()): | |
| 411 | + tags.add ";" & k & "=" & v | |
| 412 | + send("@" & tags & " PRIVMSG " & app.current & " :" & line) | |
| 413 | + # Rewritten on screen now rather than when the echo lands, and — more to | |
| 414 | + # the point — *instead* of the local echo below, which would leave the | |
| 415 | + # original sitting above a copy of itself with the new wording. | |
| 416 | + discard app.rooms.applyEdit(app.current, mid, app.formNick, line, "") | |
| 405 | 417 | app.editing = EditTarget() |
| 418 | + app.draft = "" | |
| 419 | + app.attachment = Attachment() | |
| 420 | + return | |
| 406 | 421 | elif app.replyingTo.has: |
| 407 | 422 | send("@+draft/reply=" & app.replyingTo.id & " PRIVMSG " & app.current & |
| 408 | 423 | " :" & line) |
| @@ -954,6 +969,28 @@ proc drain*() = | ||
| 954 | 969 | # A message to us rather than to a channel belongs in a buffer named |
| 955 | 970 | # for the sender: the target is our own nick and is nobody's room. |
| 956 | 971 | let room = if target.startsWith("#"): target else: who |
| 972 | + # A revision is not a new line: it replaces the one it names, under | |
| 973 | + # that line's own id — never the revision's own wire msgid, which | |
| 974 | + # nothing else refers to. Before the echo check below, because our | |
| 975 | + # own edit comes back this way too and is not a new message either. | |
| 976 | + let (editOf, isEdit) = block: | |
| 977 | + let (v, ok2) = tagValue(p.tags, "+edit") | |
| 978 | + if ok2: (v, true) else: tagValue(p.tags, "+draft/edit") | |
| 979 | + if isEdit and editOf.len > 0: | |
| 980 | + if app.rooms.applyEdit(room, editOf, who, p.params[^1], msgid) == | |
| 981 | + erAbsent: | |
| 982 | + # The original is older than the backlog we hold, so show the | |
| 983 | + # current wording rather than dropping what was said. | |
| 984 | + var m = Message(id: editOf, frm: who, text: p.params[^1], at: at, | |
| 985 | + edited: true) | |
| 986 | + if p.hasAccount: m.account = p.account | |
| 987 | + m.imageUrl = firstImageUrl(m.text) | |
| 988 | + let (rep, hasRep) = tagValue(p.tags, "+reply") | |
| 989 | + if hasRep: m.replyTo = rep | |
| 990 | + notePreviewTags(p.tags) | |
| 991 | + note(room, m) | |
| 992 | + continue | |
| 993 | + | |
| 957 | 994 | # Our own line coming back is the copy we already showed, with the |
| 958 | 995 | # msgid the server gave it — not a new message. |
| 959 | 996 | if who == app.formNick and app.rooms.hasKey(room): |
| @@ -975,6 +1012,11 @@ proc drain*() = | ||
| 975 | 1012 | if hasRep: m.replyTo = rep |
| 976 | 1013 | let (tally, hasTally) = tagValue(p.tags, "+freeq.at/reacts") |
| 977 | 1014 | if hasTally: m.reactions = parseTally(tally) |
| 1015 | + # A replay sends one row per message, carrying the current text and | |
| 1016 | + # no `+draft/edit` to say it is not the original. This tag is the | |
| 1017 | + # only trace, so a message can arrive already edited. | |
| 1018 | + let (wasEdited, hasEdited) = tagValue(p.tags, "+freeq.at/edited") | |
| 1019 | + if hasEdited and wasEdited != "0": m.edited = true | |
| 978 | 1020 | notePreviewTags(p.tags) |
| 979 | 1021 | note(room, m) |
| 980 | 1022 | |
| @@ -400,9 +400,24 @@ proc sendDraft() = | |||
| 400 | if app.editing.has: | 400 | if app.editing.has: |
| 401 | # An edit is a fresh PRIVMSG tagged with what it replaces; the server | 401 | # An edit is a fresh PRIVMSG tagged with what it replaces; the server |
| 402 | # rewrites the original and echoes the revision back. | 402 | # rewrites the original and echoes the revision back. |
| 403 | - send("@+draft/edit=" & app.editing.id & " PRIVMSG " & app.current & | 403 | + # |
| 404 | - " :" & line) | 404 | + # Signed like every other change to a record already written: freeq keeps |
| 405 | + # the original and says nothing an unsigned edit from an account asked | ||
| 406 | + # for, so the line would simply never change. | ||
| 407 | + let mid = app.editing.id | ||
| 408 | + var tags = "+draft/edit=" & mid | ||
| 409 | + for k, v in editTags(app.current, mid, line, "", | ||
| 410 | + peerDid(app.currentRoom, app.formNick), nowMs()): | ||
| 411 | + tags.add ";" & k & "=" & v | ||
| 412 | + send("@" & tags & " PRIVMSG " & app.current & " :" & line) | ||
| 413 | + # Rewritten on screen now rather than when the echo lands, and — more to | ||
| 414 | + # the point — *instead* of the local echo below, which would leave the | ||
| 415 | + # original sitting above a copy of itself with the new wording. | ||
| 416 | + discard app.rooms.applyEdit(app.current, mid, app.formNick, line, "") | ||
| 405 | app.editing = EditTarget() | 417 | app.editing = EditTarget() |
| 418 | + app.draft = "" | ||
| 419 | + app.attachment = Attachment() | ||
| 420 | + return | ||
| 406 | elif app.replyingTo.has: | 421 | elif app.replyingTo.has: |
| 407 | send("@+draft/reply=" & app.replyingTo.id & " PRIVMSG " & app.current & | 422 | send("@+draft/reply=" & app.replyingTo.id & " PRIVMSG " & app.current & |
| 408 | " :" & line) | 423 | " :" & line) |
| @@ -954,6 +969,28 @@ proc drain*() = | |||
| 954 | # A message to us rather than to a channel belongs in a buffer named | 969 | # A message to us rather than to a channel belongs in a buffer named |
| 955 | # for the sender: the target is our own nick and is nobody's room. | 970 | # for the sender: the target is our own nick and is nobody's room. |
| 956 | let room = if target.startsWith("#"): target else: who | 971 | let room = if target.startsWith("#"): target else: who |
| 972 | + # A revision is not a new line: it replaces the one it names, under | ||
| 973 | + # that line's own id — never the revision's own wire msgid, which | ||
| 974 | + # nothing else refers to. Before the echo check below, because our | ||
| 975 | + # own edit comes back this way too and is not a new message either. | ||
| 976 | + let (editOf, isEdit) = block: | ||
| 977 | + let (v, ok2) = tagValue(p.tags, "+edit") | ||
| 978 | + if ok2: (v, true) else: tagValue(p.tags, "+draft/edit") | ||
| 979 | + if isEdit and editOf.len > 0: | ||
| 980 | + if app.rooms.applyEdit(room, editOf, who, p.params[^1], msgid) == | ||
| 981 | + erAbsent: | ||
| 982 | + # The original is older than the backlog we hold, so show the | ||
| 983 | + # current wording rather than dropping what was said. | ||
| 984 | + var m = Message(id: editOf, frm: who, text: p.params[^1], at: at, | ||
| 985 | + edited: true) | ||
| 986 | + if p.hasAccount: m.account = p.account | ||
| 987 | + m.imageUrl = firstImageUrl(m.text) | ||
| 988 | + let (rep, hasRep) = tagValue(p.tags, "+reply") | ||
| 989 | + if hasRep: m.replyTo = rep | ||
| 990 | + notePreviewTags(p.tags) | ||
| 991 | + note(room, m) | ||
| 992 | + continue | ||
| 993 | + | ||
| 957 | # Our own line coming back is the copy we already showed, with the | 994 | # Our own line coming back is the copy we already showed, with the |
| 958 | # msgid the server gave it — not a new message. | 995 | # msgid the server gave it — not a new message. |
| 959 | if who == app.formNick and app.rooms.hasKey(room): | 996 | if who == app.formNick and app.rooms.hasKey(room): |
| @@ -975,6 +1012,11 @@ proc drain*() = | |||
| 975 | if hasRep: m.replyTo = rep | 1012 | if hasRep: m.replyTo = rep |
| 976 | let (tally, hasTally) = tagValue(p.tags, "+freeq.at/reacts") | 1013 | let (tally, hasTally) = tagValue(p.tags, "+freeq.at/reacts") |
| 977 | if hasTally: m.reactions = parseTally(tally) | 1014 | if hasTally: m.reactions = parseTally(tally) |
| 1015 | + # A replay sends one row per message, carrying the current text and | ||
| 1016 | + # no `+draft/edit` to say it is not the original. This tag is the | ||
| 1017 | + # only trace, so a message can arrive already edited. | ||
| 1018 | + let (wasEdited, hasEdited) = tagValue(p.tags, "+freeq.at/edited") | ||
| 1019 | + if hasEdited and wasEdited != "0": m.edited = true | ||
| 978 | notePreviewTags(p.tags) | 1020 | notePreviewTags(p.tags) |
| 979 | note(room, m) | 1021 | note(room, m) |
| 980 | 1022 | ||
added
nim/tests/tedits.nim +43 -0 | new file mode 100644 | ||
| @@ -0,0 +1,43 @@ | ||
| 1 | +## A revision folded into the buffer, and a line taken out of it. | |
| 2 | + | |
| 3 | +import std/[tables, unittest] | |
| 4 | +import frq/[model, edits] | |
| 5 | + | |
| 6 | +proc roomWith(msgs: varargs[Message]): OrderedTable[string, Room] = | |
| 7 | + var r = initRoom("#test") | |
| 8 | + for m in msgs: r.messages.add m | |
| 9 | + result = initOrderedTable[string, Room]() | |
| 10 | + result["#test"] = r | |
| 11 | + | |
| 12 | +suite "applyEdit": | |
| 13 | + test "the sender's own line is rewritten in place": | |
| 14 | + var rooms = roomWith(Message(id: "m1", frm: "ann", text: "helo")) | |
| 15 | + check rooms.applyEdit("#test", "m1", "ann", "hello", "m2") == erApplied | |
| 16 | + check rooms["#test"].messages.len == 1 | |
| 17 | + check rooms["#test"].messages[0].text == "hello" | |
| 18 | + check rooms["#test"].messages[0].edited | |
| 19 | + # The line keeps the id it was born with; the revision's own id joins it, | |
| 20 | + # so a reply naming either still finds this line. | |
| 21 | + check rooms["#test"].messages[0].id == "m1" | |
| 22 | + check "m2" in rooms["#test"].messages[0].editIds | |
| 23 | + | |
| 24 | + test "somebody else's line is not theirs to rewrite": | |
| 25 | + var rooms = roomWith(Message(id: "m1", frm: "ann", text: "helo")) | |
| 26 | + check rooms.applyEdit("#test", "m1", "bob", "words", "") == erRefused | |
| 27 | + check rooms["#test"].messages[0].text == "helo" | |
| 28 | + | |
| 29 | + test "an edit of something older than the backlog is absent": | |
| 30 | + var rooms = roomWith(Message(id: "m1", frm: "ann", text: "helo")) | |
| 31 | + check rooms.applyEdit("#test", "m0", "ann", "hello", "") == erAbsent | |
| 32 | + | |
| 33 | +suite "applyDelete": | |
| 34 | + test "the line stops being on screen": | |
| 35 | + var rooms = roomWith(Message(id: "m1", frm: "ann", text: "oops"), | |
| 36 | + Message(id: "m2", frm: "bob", text: "hi")) | |
| 37 | + check rooms.applyDelete("#test", "m1") | |
| 38 | + check rooms["#test"].messages.len == 1 | |
| 39 | + check rooms["#test"].messages[0].id == "m2" | |
| 40 | + | |
| 41 | + test "a delete for a line we never had is not an error": | |
| 42 | + var rooms = roomWith(Message(id: "m1", frm: "ann", text: "oops")) | |
| 43 | + check not rooms.applyDelete("#test", "m9") | |
| new file mode 100644 | |||
| @@ -0,0 +1,43 @@ | |||
| 1 | +## A revision folded into the buffer, and a line taken out of it. | ||
| 2 | + | ||
| 3 | +import std/[tables, unittest] | ||
| 4 | +import frq/[model, edits] | ||
| 5 | + | ||
| 6 | +proc roomWith(msgs: varargs[Message]): OrderedTable[string, Room] = | ||
| 7 | + var r = initRoom("#test") | ||
| 8 | + for m in msgs: r.messages.add m | ||
| 9 | + result = initOrderedTable[string, Room]() | ||
| 10 | + result["#test"] = r | ||
| 11 | + | ||
| 12 | +suite "applyEdit": | ||
| 13 | + test "the sender's own line is rewritten in place": | ||
| 14 | + var rooms = roomWith(Message(id: "m1", frm: "ann", text: "helo")) | ||
| 15 | + check rooms.applyEdit("#test", "m1", "ann", "hello", "m2") == erApplied | ||
| 16 | + check rooms["#test"].messages.len == 1 | ||
| 17 | + check rooms["#test"].messages[0].text == "hello" | ||
| 18 | + check rooms["#test"].messages[0].edited | ||
| 19 | + # The line keeps the id it was born with; the revision's own id joins it, | ||
| 20 | + # so a reply naming either still finds this line. | ||
| 21 | + check rooms["#test"].messages[0].id == "m1" | ||
| 22 | + check "m2" in rooms["#test"].messages[0].editIds | ||
| 23 | + | ||
| 24 | + test "somebody else's line is not theirs to rewrite": | ||
| 25 | + var rooms = roomWith(Message(id: "m1", frm: "ann", text: "helo")) | ||
| 26 | + check rooms.applyEdit("#test", "m1", "bob", "words", "") == erRefused | ||
| 27 | + check rooms["#test"].messages[0].text == "helo" | ||
| 28 | + | ||
| 29 | + test "an edit of something older than the backlog is absent": | ||
| 30 | + var rooms = roomWith(Message(id: "m1", frm: "ann", text: "helo")) | ||
| 31 | + check rooms.applyEdit("#test", "m0", "ann", "hello", "") == erAbsent | ||
| 32 | + | ||
| 33 | +suite "applyDelete": | ||
| 34 | + test "the line stops being on screen": | ||
| 35 | + var rooms = roomWith(Message(id: "m1", frm: "ann", text: "oops"), | ||
| 36 | + Message(id: "m2", frm: "bob", text: "hi")) | ||
| 37 | + check rooms.applyDelete("#test", "m1") | ||
| 38 | + check rooms["#test"].messages.len == 1 | ||
| 39 | + check rooms["#test"].messages[0].id == "m2" | ||
| 40 | + | ||
| 41 | + test "a delete for a line we never had is not an error": | ||
| 42 | + var rooms = roomWith(Message(id: "m1", frm: "ann", text: "oops")) | ||
| 43 | + check not rooms.applyDelete("#test", "m9") | ||