diff options
| author | Danilo M. <danix@danix.xyz> | 2026-10-13 10:40:51 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-10-13 10:40:51 +0200 |
| commit | dc6211b61676fcea24aabd3300648cec699175fb (patch) | |
| tree | b74b3cd3d9fa3025cf8a9800f860a7cdb438b1d2 /desktop | |
| parent | 901a88e26b91eb104aa9a837372d35db24f7b6bf (diff) | |
| download | quickshell-dc6211b61676fcea24aabd3300648cec699175fb.tar.gz quickshell-dc6211b61676fcea24aabd3300648cec699175fb.zip | |
fix(calendar): review findings in calctl and the editor
An edit now rewrites only the fields that changed. A save with nothing
changed used to rebuild the alarm, turning ACTION:AUDIO into DISPLAY
and dropping ACKNOWLEDGED, re-anchor the start to the local zone, and
strip SUMMARY's parameters; the test edits a foreign event unchanged and
checks every one survives.
get, edit and delete name the calendar the event is in. The same invite
synced into two calendars has one UID, and the panel acted on whichever
file came first, so editing one copy could overwrite or delete the
other.
A custom rule's date cannot be moved from the panel: BYDAY=MO stayed on
Monday while the start moved, and the series lost the day shown.
UNTIL is read back in local time, so a 22:30 series keeps its last
occurrence. Files are written 0644 like vdirsyncer's. Enter in the title
no longer bypasses the busy guard, which could create an event twice,
and a throwing callback no longer leaks its Process.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Diffstat (limited to 'desktop')
| -rw-r--r-- | desktop/island/calendar/Cal.js | 7 | ||||
| -rw-r--r-- | desktop/island/calendar/Calendar.qml | 11 | ||||
| -rw-r--r-- | desktop/island/calendar/CalendarPanel.qml | 6 | ||||
| -rw-r--r-- | desktop/island/calendar/EventEditor.qml | 6 | ||||
| -rwxr-xr-x | desktop/island/calendar/calctl | 96 | ||||
| -rw-r--r-- | desktop/island/calendar/test_cal.js | 2 | ||||
| -rw-r--r-- | desktop/island/calendar/test_calctl.py | 101 |
7 files changed, 170 insertions, 59 deletions
diff --git a/desktop/island/calendar/Cal.js b/desktop/island/calendar/Cal.js index dc50b04..d8dcf80 100644 --- a/desktop/island/calendar/Cal.js +++ b/desktop/island/calendar/Cal.js @@ -107,14 +107,15 @@ function stepTime(s, minutes) { // on the same day. function newDraft(day, now, calendar) { const h = Math.min(22, now.getHours() + 1); - return { uid: "", calendar: calendar, title: "", allDay: false, + return { uid: "", source: "", calendar: calendar, title: "", allDay: false, startDay: day, endDay: day, startTime: pad(h) + ":00", endTime: pad(h + 1) + ":00", repeat: "none", until: "", alarm: "1h", location: "", notes: "", override: false }; } -// calctl's event JSON as an editor draft. +// calctl's event JSON as an editor draft. `source` is the calendar the event +// lives in now; `calendar` is where the form wants it. function fromEvent(ev) { - return { uid: ev.uid, calendar: ev.calendar, title: ev.title, allDay: ev.allDay, + return { uid: ev.uid, source: ev.calendar, calendar: ev.calendar, title: ev.title, allDay: ev.allDay, startDay: ev.start.slice(0, 10), endDay: ev.end.slice(0, 10), startTime: ev.allDay ? "09:00" : ev.start.slice(11, 16), endTime: ev.allDay ? "10:00" : ev.end.slice(11, 16), diff --git a/desktop/island/calendar/Calendar.qml b/desktop/island/calendar/Calendar.qml index f215021..4a5e7d4 100644 --- a/desktop/island/calendar/Calendar.qml +++ b/desktop/island/calendar/Calendar.qml @@ -51,10 +51,11 @@ Singleton { }); } - function get(uid, cb) { root.run(["get", uid], cb); } + // `cal` is the calendar the event is in now: one UID can be in two. + function get(uid, cal, cb) { root.run(["get", uid, cal], cb); } function create(event, cb) { root.run(["new", JSON.stringify(event)], cb); } - function update(uid, event, cb) { root.run(["edit", uid, JSON.stringify(event)], cb); } - function remove(uid, cb) { root.run(["delete", uid], cb); } + function update(uid, cal, event, cb) { root.run(["edit", uid, cal, JSON.stringify(event)], cb); } + function remove(uid, cal, cb) { root.run(["delete", uid, cal], cb); } // One Process per call, so a list and a save never share one (a running // Process ignores a second start, AGENTS.md). cb gets calctl's envelope. @@ -72,8 +73,8 @@ Singleton { let r; try { r = JSON.parse(text); } catch (e) { r = { ok: false, error: "calctl: " + (text.trim() || "no output") }; } - if (proc.cb) proc.cb(r); - proc.destroy(); + try { if (proc.cb) proc.cb(r); } + finally { proc.destroy(); } } } } diff --git a/desktop/island/calendar/CalendarPanel.qml b/desktop/island/calendar/CalendarPanel.qml index 602f58a..7d42f7d 100644 --- a/desktop/island/calendar/CalendarPanel.qml +++ b/desktop/island/calendar/CalendarPanel.qml @@ -72,8 +72,8 @@ Item { panel.show(+day.slice(0, 4), +day.slice(5, 7) - 1); } - function openEvent(uid) { - Calendar.get(uid, r => { + function openEvent(uid, calendar) { + Calendar.get(uid, calendar, r => { if (!r.ok) { panel.agendaError = r.error; return; } editor.load(Cal.fromEvent(r.data)); panel.editing = true; @@ -291,7 +291,7 @@ Item { anchors.fill: parent hoverEnabled: true cursorShape: Qt.PointingHandCursor - onClicked: panel.openEvent(row.modelData.uid) + onClicked: panel.openEvent(row.modelData.uid, row.modelData.calendar) } } } diff --git a/desktop/island/calendar/EventEditor.qml b/desktop/island/calendar/EventEditor.qml index 63b29f0..7f3ad2a 100644 --- a/desktop/island/calendar/EventEditor.qml +++ b/desktop/island/calendar/EventEditor.qml @@ -65,17 +65,19 @@ Column { } function save() { + if (ed.busy) return; const r = Cal.toPayload(ed.draft); if (r.error) { ed.err = r.error; return; } ed.busy = true; - if (ed.draft.uid) Calendar.update(ed.draft.uid, r.payload, ed.finish); + if (ed.draft.uid) Calendar.update(ed.draft.uid, ed.draft.source, r.payload, ed.finish); else Calendar.create(r.payload, ed.finish); } function remove() { + if (ed.busy) return; if (!ed.confirmDelete) { ed.confirmDelete = true; return; } ed.busy = true; - Calendar.remove(ed.draft.uid, ed.finish); + Calendar.remove(ed.draft.uid, ed.draft.source, ed.finish); } function dayLabel(day) { diff --git a/desktop/island/calendar/calctl b/desktop/island/calendar/calctl index d79d8b9..e9cfb93 100755 --- a/desktop/island/calendar/calctl +++ b/desktop/island/calendar/calctl @@ -17,10 +17,10 @@ # # calctl calendars {default, calendars: [{name, color, path}]} # calctl list FROM TO occurrences in [FROM, TO), local time -# calctl get UID the series itself, for the editor +# calctl get UID CAL the series itself, for the editor # calctl new EVENT-JSON {uid} -# calctl edit UID EVENT-JSON {uid} -# calctl delete UID {} +# calctl edit UID CAL EVENT-JSON {uid}; CAL is where it is now +# calctl delete UID CAL {} # # Every command prints one object: {"ok": true, "data": ...} or # {"ok": false, "error": "..."} with exit 1. Environment, for the tests: @@ -61,7 +61,8 @@ COLORS = { "cyan": "teal", "dark cyan": "teal", "light cyan": "teal", } -USAGE = "usage: calctl calendars | list FROM TO | get UID | new JSON | edit UID JSON | delete UID" +USAGE = ("usage: calctl calendars | list FROM TO | get UID CAL | new JSON" + " | edit UID CAL JSON | delete UID CAL") class Fail(Exception): @@ -150,11 +151,13 @@ def events(): yield cal, path, ev -def find(uid): +def find(uid, cal_name): + """An event by UID within one calendar: the same invite synced into two + calendars has one UID and two files, and each row must act on its own.""" for cal, path, ev in events(): - if str(ev.get("UID", "")) == uid: + if cal["name"] == cal_name and str(ev.get("UID", "")) == uid: return cal, path - raise Fail(f"no event {uid}") + raise Fail(f"no event {uid} in {cal_name}") def as_local(value): @@ -256,7 +259,17 @@ def cmd_list(lo_s, hi_s): dur = e - s if "RRULE" in ev: # Naive local time keeps the wall-clock time across DST. - rule = rrulestr(ev["RRULE"].to_ical().decode(), dtstart=s, ignoretz=True) + # ponytail: right for local and same-rule zones (all the live + # data); a series in a zone with other DST dates shows an hour + # off in the weeks they differ. Expand in the event's own zone + # if such events arrive. + rule = ev["RRULE"] + if "UNTIL" in rule: + # UNTIL is UTC on disk; read it as local time like the + # start, or a 22:30 series loses its last occurrence. + rule = icalendar.vRecur(rule) + rule["UNTIL"] = [as_local(u) for u in rule["UNTIL"]] + rule = rrulestr(rule.to_ical().decode(), dtstart=s, ignoretz=True) starts = rule.between(lo - dur, hi, inc=True) else: starts = [s] @@ -273,8 +286,8 @@ def cmd_list(lo_s, hi_s): return out -def cmd_get(uid): - cal, path = find(uid) +def cmd_get(uid, cal_name): + cal, path = find(uid, cal_name) ev = master(icalendar.Calendar.from_ical(path.read_bytes())) start, end, all_day = span(ev) return to_json(cal, ev, start, end, all_day, overrides()) @@ -320,23 +333,26 @@ def parse_payload(text, cals): def apply(ev, p): - """Write the form's fields into a VEVENT. Everything else on it is left - alone; a `custom` repeat or alarm keeps what is there.""" - for key in ("DTSTART", "DTEND", "DURATION", "SUMMARY", "LOCATION", "DESCRIPTION", - "DTSTAMP", "LAST-MODIFIED"): - ev.pop(key, None) - if p["allDay"]: - ev.add("DTSTART", p["start"]) - ev.add("DTEND", p["end"]) - else: - ev.add("DTSTART", p["start"].replace(tzinfo=LOCAL)) - ev.add("DTEND", p["end"].replace(tzinfo=LOCAL)) - ev.add("SUMMARY", p["title"]) - if p["location"]: - ev.add("LOCATION", p["location"]) - if p["notes"]: - ev.add("DESCRIPTION", p["notes"]) - if p["repeat"] != "custom": + """Write the form's fields into a VEVENT, touching only what changed: an + unchanged time keeps its zone and DURATION, an unchanged alarm keeps its + ACTION and extras, an unchanged title keeps its parameters. Everything + else on it is left alone; a `custom` repeat or alarm keeps what is there.""" + if "DTSTART" not in ev or span(ev) != (p["start"], p["end"], p["allDay"]): + for key in ("DTSTART", "DTEND", "DURATION"): + ev.pop(key, None) + if p["allDay"]: + ev.add("DTSTART", p["start"]) + ev.add("DTEND", p["end"]) + else: + ev.add("DTSTART", p["start"].replace(tzinfo=LOCAL)) + ev.add("DTEND", p["end"].replace(tzinfo=LOCAL)) + for key, prop in (("title", "SUMMARY"), ("location", "LOCATION"), ("notes", "DESCRIPTION")): + if str(ev.get(prop, "")) != p[key]: + ev.pop(prop, None) + if p[key]: + ev.add(prop, p[key]) + until = p["until"].isoformat() if p["until"] and p["repeat"] != "none" else "" + if p["repeat"] != "custom" and repeat_of(ev) != (p["repeat"], until): ev.pop("RRULE", None) if p["repeat"] != "none": rule = {"FREQ": PRESET_REPEAT[p["repeat"]]} @@ -346,7 +362,7 @@ def apply(ev, p): datetime.combine(p["until"], time(23, 59, 59), LOCAL) .astimezone(timezone.utc)) ev.add("RRULE", rule) - if p["alarm"] != "custom": + if p["alarm"] != "custom" and alarm_of(ev) != p["alarm"]: ev.subcomponents = [c for c in ev.subcomponents if c.name != "VALARM"] if p["alarm"] != "none": alarm = icalendar.Alarm() @@ -355,6 +371,8 @@ def apply(ev, p): alarm.add("TRIGGER", -PRESET_ALARM[p["alarm"]]) ev.add_component(alarm) now = datetime.now(timezone.utc).replace(microsecond=0) + ev.pop("DTSTAMP", None) + ev.pop("LAST-MODIFIED", None) ev.add("DTSTAMP", now) ev.add("LAST-MODIFIED", now) @@ -365,6 +383,8 @@ def write(path, ics): temporary name does not end in .ics, so nothing lists it.""" fd, tmp = tempfile.mkstemp(dir=path.parent, prefix=".calctl-", suffix=".tmp") try: + # mkstemp makes 0600; vdirsyncer's own files are 0644. + os.fchmod(fd, 0o644) with os.fdopen(fd, "wb") as f: f.write(ics.to_ical()) os.replace(tmp, path) @@ -405,12 +425,18 @@ def cmd_new(text): return {"uid": uid} -def cmd_edit(uid, text): +def cmd_edit(uid, cal_name, text): cals, _ = calendars() p = parse_payload(text, cals) - _, path = find(uid) + _, path = find(uid, cal_name) ics = icalendar.Calendar.from_ical(path.read_bytes()) ev = master(ics) + if p["repeat"] == "custom": + # A rule like BYDAY=MO is tied to its day; moving the start alone + # would leave the series on the old day. + day = lambda v: v.date() if isinstance(v, datetime) else v + if day(span(ev)[0]) != day(p["start"]): + raise Fail("this event repeats by a rule the panel cannot edit, so its date cannot move here") apply(ev, p) sequence = int(ev.pop("SEQUENCE", 0)) ev.add("SEQUENCE", sequence + 1) @@ -425,8 +451,8 @@ def cmd_edit(uid, text): return {"uid": uid} -def cmd_delete(uid): - _, path = find(uid) +def cmd_delete(uid, cal_name): + _, path = find(uid, cal_name) path.unlink() sync() return {} @@ -442,13 +468,13 @@ def main(argv): for c in cals]} elif cmd == "list" and len(args) == 2: data = cmd_list(*args) - elif cmd == "get" and len(args) == 1: + elif cmd == "get" and len(args) == 2: data = cmd_get(*args) elif cmd == "new" and len(args) == 1: data = cmd_new(*args) - elif cmd == "edit" and len(args) == 2: + elif cmd == "edit" and len(args) == 3: data = cmd_edit(*args) - elif cmd == "delete" and len(args) == 1: + elif cmd == "delete" and len(args) == 2: data = cmd_delete(*args) else: raise Fail(USAGE) diff --git a/desktop/island/calendar/test_cal.js b/desktop/island/calendar/test_cal.js index d91966e..9d0d147 100644 --- a/desktop/island/calendar/test_cal.js +++ b/desktop/island/calendar/test_cal.js @@ -84,6 +84,7 @@ assert.strictEqual(d.endTime, "11:00"); assert.strictEqual(d.startDay, "2026-10-20"); assert.strictEqual(d.alarm, "1h"); assert.strictEqual(d.calendar, "home"); +assert.strictEqual(d.source, ""); assert.strictEqual(C.newDraft("2026-10-20", new Date(2026, 9, 13, 23, 10), "home").startTime, "22:00"); // From calctl's JSON. @@ -92,6 +93,7 @@ const fe = C.fromEvent({ uid: "u", calendar: "home", title: "Trip", allDay: true assert.strictEqual(fe.startDay, "2026-10-14"); assert.strictEqual(fe.endDay, "2026-10-16"); assert.strictEqual(fe.allDay, true); +assert.strictEqual(fe.source, "home"); // where it lives now, for edit and delete assert.strictEqual(C.fromEvent(Object.assign({}, timed, { title: "x" })).startTime, "14:30"); // Picking on the grid: a click moves the event and keeps its length in days; diff --git a/desktop/island/calendar/test_calctl.py b/desktop/island/calendar/test_calctl.py index 98973ee..91ad384 100644 --- a/desktop/island/calendar/test_calctl.py +++ b/desktop/island/calendar/test_calctl.py @@ -117,7 +117,7 @@ END:VALARM END:VEVENT END:VCALENDAR """) -w = run("get", "weird-1") +w = run("get", "weird-1", "home") assert w["repeat"] == "custom" and w["alarm"] == "custom" and w["title"] == "Standup", w assert w["start"] == "2026-10-05T09:00", w occ = [e["start"] for e in run("list", "2026-10-01", "2026-11-01") if e["uid"] == "weird-1"] @@ -128,7 +128,7 @@ assert len(occ) == 8 and occ[0] == "2026-10-05T09:00" and "2026-10-26T09:00" in assert run("list", "2026-10-01", "2026-11-01") # Unknown uid and bad usage: the error envelope, exit 1. -assert "no event" in run("get", "nope", ok=False) +assert "no event" in run("get", "nope", "home", ok=False) assert "usage" in run("frobnicate", ok=False) def ev(**kw): @@ -156,7 +156,7 @@ assert len(mine) == 1, mine assert mine[0]["start"] == "2026-10-15T14:30" and mine[0]["end"] == "2026-10-15T15:30" assert mine[0]["alarm"] == "1h" and mine[0]["repeat"] == "none" and mine[0]["color"] == "green" assert mine[0]["override"] is False -assert run("get", uid)["title"] == "Dentist" +assert run("get", uid, "home")["title"] == "Dentist" # all-day, multi-day: exclusive DTEND on disk, inclusive end in JSON. trip = run("new", ev(title="Trip", allDay=True, start="2026-10-30", end="2026-11-02", alarm="none"))["uid"] @@ -171,11 +171,19 @@ assert not [e for e in run("list", "2026-10-01", "2026-10-30") if e["uid"] == tr gym = run("new", ev(title="Gym", repeat="weekly", until="2026-10-29"))["uid"] occ = [e["start"] for e in run("list", "2026-10-01", "2026-12-01") if e["uid"] == gym] assert occ == ["2026-10-15T14:30", "2026-10-22T14:30", "2026-10-29T14:30"], occ -assert run("get", gym)["until"] == "2026-10-29" +assert os.stat(tmp / "a" / f"{gym}.ics").st_mode & 0o777 == 0o644 + +# A late-evening series keeps its last occurrence: UNTIL is UTC on disk and +# read back in local time. +late = run("new", ev(title="Late", start="2026-10-05T22:30", end="2026-10-05T23:00", + repeat="weekly", until="2026-10-19"))["uid"] +occ = [e["start"] for e in run("list", "2026-10-01", "2026-11-01") if e["uid"] == late] +assert occ == ["2026-10-05T22:30", "2026-10-12T22:30", "2026-10-19T22:30"], occ +assert run("get", gym, "home")["until"] == "2026-10-29" # edit keeping custom: the rich rule, both alarms and an unknown property # survive; SEQUENCE goes up. -run("edit", "weird-1", ev(title="Standup v2", start="2026-10-05T09:30", +run("edit", "weird-1", "home", ev(title="Standup v2", start="2026-10-05T09:30", end="2026-10-05T10:00", repeat="custom", alarm="custom")) t = (tmp / "a" / "weird.ics").read_text() assert "BYDAY=MO,WE" in t and t.count("BEGIN:VALARM") == 2 and "X-FOO:bar" in t, t @@ -183,24 +191,95 @@ assert "SUMMARY:Standup v2" in t and "SEQUENCE:1" in t, t no_temp() # edit to presets: the rule goes, the alarms become one. -run("edit", "weird-1", ev(title="Standup v2", start="2026-10-05T09:30", +run("edit", "weird-1", "home", ev(title="Standup v2", start="2026-10-05T09:30", end="2026-10-05T10:00", repeat="none", alarm="10m")) t = (tmp / "a" / "weird.ics").read_text() assert "RRULE" not in t and t.count("BEGIN:VALARM") == 1 and "TRIGGER:-PT10M" in t, t +# a custom rule cannot have its date moved: its BY* parts would stay on the +# old day. +put(tmp / "a" / "weird2.ics", """ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//test//EN +BEGIN:VEVENT +UID:weird-2 +DTSTAMP:20260101T000000Z +DTSTART;TZID=Europe/Rome:20261005T090000 +DTEND;TZID=Europe/Rome:20261005T100000 +RRULE:FREQ=WEEKLY;BYDAY=MO +SUMMARY:Standup +END:VEVENT +END:VCALENDAR +""") +assert "rule" in run("edit", "weird-2", "home", ev(title="Standup", start="2026-10-07T09:00", + end="2026-10-07T10:00", repeat="custom", + alarm="none"), ok=False) + +# an edit that changes nothing leaves the event's own encoding alone: zone, +# DURATION, the alarm's ACTION and extras, SUMMARY's parameter. +put(tmp / "a" / "foreign.ics", """ +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//test//EN +BEGIN:VEVENT +UID:foreign-1 +DTSTAMP:20260101T000000Z +DTSTART;TZID=America/New_York:20261005T100000 +DURATION:PT1H +SUMMARY;LANGUAGE=it:Chiamata +BEGIN:VALARM +ACTION:AUDIO +TRIGGER:-PT1H +ACKNOWLEDGED:20261001T080000Z +END:VALARM +END:VEVENT +END:VCALENDAR +""") +g = run("get", "foreign-1", "home") +same = {k: g[k] for k in ("calendar", "title", "allDay", "start", "end", "repeat", "until", + "alarm", "location", "notes")} +run("edit", "foreign-1", "home", json.dumps(same)) +t = (tmp / "a" / "foreign.ics").read_text() +for kept in ("DTSTART;TZID=America/New_York:20261005T100000", "DURATION:PT1H", "ACTION:AUDIO", + "ACKNOWLEDGED:20261001T080000Z", "SUMMARY;LANGUAGE=it:Chiamata"): + assert kept in t, (kept, t) +# Changing only the title rewrites the title, nothing else. +run("edit", "foreign-1", "home", json.dumps(dict(same, title="Call"))) +t = (tmp / "a" / "foreign.ics").read_text() +assert "SUMMARY:Call" in t and "TZID=America/New_York" in t and "ACTION:AUDIO" in t, t + +# the same UID in two calendars: each command acts on the copy it names. +for d in ("a", "b"): + put(tmp / d / "dup.ics", f""" +BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//test//EN +BEGIN:VEVENT +UID:dup-1 +DTSTAMP:20260101T000000Z +DTSTART;VALUE=DATE:20261020 +SUMMARY:copy {d} +END:VEVENT +END:VCALENDAR +""") +assert run("get", "dup-1", "birthdays")["title"] == "copy b" +run("delete", "dup-1", "home") +assert not (tmp / "a" / "dup.ics").exists() and (tmp / "b" / "dup.ics").exists() + # a calendar change moves the file. -run("edit", uid, ev(calendar="birthdays")) +run("edit", uid, "home", ev(calendar="birthdays")) assert not f.exists() and (tmp / "b" / f.name).exists() -assert run("get", uid)["calendar"] == "birthdays" +assert run("get", uid, "birthdays")["calendar"] == "birthdays" # cal-notif's per-event override is reported. (tmp / "overrides.toml").write_text(f'"{uid}" = ["10m"]\n') -assert run("get", uid)["override"] is True +assert run("get", uid, "birthdays")["override"] is True # delete. -assert run("delete", uid) == {} +assert run("delete", uid, "birthdays") == {} assert not (tmp / "b" / f.name).exists() -assert "no event" in run("get", uid, ok=False) +assert "no event" in run("get", uid, "birthdays", ok=False) # validation: the error envelope, exit 1, nothing written. before = sorted(p.name for p in tmp.rglob("*.ics")) |
