fix: drop orphaned overrides when a series stops, plus review minors

5 files changed, 38 insertions, 2 deletions

diff --git a/src/calendarstore.cpp b/src/calendarstore.cpp
index ef67eba..619ffb9 100644
--- a/src/calendarstore.cpp
+++ b/src/calendarstore.cpp
@@ -363,8 +363,25 @@ void writeFields(icalcomponent *root, icalcomponent *c, const EventEdit &edit, b
     if (!master || edit.repeat.custom)
         return;  // a custom RRULE is never rewritten (spec, repeat control)
     removeAll(c, ICAL_RRULE_PROPERTY);
-    if (edit.repeat.freq == RepeatRule::Freq::None)
+    if (edit.repeat.freq == RepeatRule::Freq::None) {
+        // Stopping a series orphans its RECURRENCE-ID overrides: occurrences()
+        // skips the override loop for a non-repeating event, so they would
+        // vanish from every view while staying in the synced file, ready to
+        // spring back if a repeat is re-enabled. EXDATEs are inert once the
+        // series does not repeat, and removing them would drop data, so they
+        // stay. Collect first: removing a component mid-walk invalidates it.
+        QList<icalcomponent *> doomed;
+        for (icalcomponent *v = icalcomponent_get_first_component(root, ICAL_VEVENT_COMPONENT);
+             v; v = icalcomponent_get_next_component(root, ICAL_VEVENT_COMPONENT)) {
+            if (v != c && icalcomponent_get_first_property(v, ICAL_RECURRENCEID_PROPERTY))
+                doomed.append(v);
+        }
+        for (icalcomponent *v : doomed) {
+            icalcomponent_remove_component(root, v);
+            icalcomponent_free(v);
+        }
         return;
+    }
     // UNTIL follows DTSTART's type: a DATE for all-day, else UTC (RFC 5545).
     QString until;
     if (edit.repeat.end == RepeatRule::End::Until) {
@@ -667,8 +684,13 @@ bool sameMeaning(const QByteArray &a, const QByteArray &b)
             || p.end != q.end || p.summary != q.summary || p.cancelled != q.cancelled)
             return false;
     }
+    // EXDATE properties are a SET: a server may reorder them, so compare
+    // sorted copies rather than the lists as read.
+    QList<QDateTime> xd = x.exdates, yd = y.exdates;
+    std::sort(xd.begin(), xd.end());
+    std::sort(yd.begin(), yd.end());
     return x.summary == y.summary && x.start == y.start && x.end == y.end
-        && x.allDay == y.allDay && x.repeat == y.repeat && x.exdates == y.exdates;
+        && x.allDay == y.allDay && x.repeat == y.repeat && xd == yd;
 }
 
 }  // namespace CalendarStore
diff --git a/src/calendarsync.cpp b/src/calendarsync.cpp
index 38d0c07..fa85c1c 100644
--- a/src/calendarsync.cpp
+++ b/src/calendarsync.cpp
@@ -29,6 +29,7 @@ CalendarSync::CalendarSync(const QString &command, int delayMs, QObject *parent)
             [this]() { m_output += m_process.readAll(); });
     connect(&m_process, &QProcess::finished, this,
             [this](int code, QProcess::ExitStatus status) {
+        m_output += m_process.readAll();  // drain the last chunk, delivered with finished
         emit finished(status == QProcess::NormalExit && code == 0,
                       QString::fromLocal8Bit(m_output));
         if (m_again) {
diff --git a/src/calendarwindow.cpp b/src/calendarwindow.cpp
index 7079c62..d9c1da2 100644
--- a/src/calendarwindow.cpp
+++ b/src/calendarwindow.cpp
@@ -145,6 +145,8 @@ CalendarWindow::CalendarWindow(const Config &config, const QStringList &ownAddre
     if (index >= 0)
         m_collectionBox->setCurrentIndex(index);
     (agenda ? m_agendaButton : m_monthButton)->click();
+    if (agenda)
+        m_agendaView->scrollToDay(QDate::currentDate());
 }
 
 CalendarWindow::~CalendarWindow() = default;
@@ -679,6 +681,9 @@ bool CalendarWindow::applyChanges(const QList<Change> &changes, bool reverse)
             for (int j = done.size() - 1; j >= 0; --j) {
                 QString ignored;
                 CalendarWriter::replace(done[j].path, done[j].after, done[j].before, &ignored);
+                // The write was reverted, so it must not be reported as a
+                // difference at the next sync.
+                m_written.remove(done[j].path);
             }
             status(r == CalendarWriter::Result::Stale
                        ? tr("The event changed on disk, probably from a sync. Check it and try again.")
diff --git a/src/calendarwriter.cpp b/src/calendarwriter.cpp
index 4c81a37..b39dced 100644
--- a/src/calendarwriter.cpp
+++ b/src/calendarwriter.cpp
@@ -49,6 +49,11 @@ Result replace(const QString &path, const std::optional<QByteArray> &expected,
         return Result::Ok;
     }
 
+    // ponytail: the stale read above and QSaveFile::commit() are not one
+    // atomic operation; a cron sync landing between them is lost. The writer
+    // holds no lock, because the sync command owns the flock. Upgrade path:
+    // take the same lock here if that window ever costs real data.
+    //
     // QSaveFile is the platform's atomic write: a temporary in the same
     // directory, renamed on commit(). Its temporary is named "<name>.XXXXXX",
     // which does not end in .ics, so vdirsyncer never lists it.
diff --git a/src/config.cpp b/src/config.cpp
index 23c08f7..14d7775 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -303,6 +303,9 @@ void Config::load(const QString &path)
         const int value = calDelay.toString().toInt(&ok);
         if (ok && value >= 0)
             m_calendarSyncDelayMs = value;
+        else if (ok)
+            addProblem(tr("Calendar sync delay %1 is out of range; using the default.")
+                           .arg(calDelay.toString()));
         else
             addProblem(tr("Calendar sync delay '%1' is not a number; using the default.")
                            .arg(calDelay.toString()));