Re: [RFC] ox-icalendar: Unscheduled tasks & repeating tasks

[Jack Kamm <jackkamm@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]

Ihor Radchenko <yantar92@posteo.net> writes:

> Side note: here, and in other places, we use "\n" as end of line. Yet,
> for example
> https://icalendar.org/iCalendar-RFC-5545/3-8-2-4-date-time-start.html
> prescribes CRLF (\r\n). Also, see
> https://orgmode.org/list/87ilgljv6i.fsf@localhost
> If you are familiar with iCalendar spec, may you look through the
> ox-icalendar code and check other places where we do not conform to the
> newline spec?

org-icalendar--vtodo is wrapped in org-icalendar-fold-string, so this
"\n" gets converted to CRLF later on.

However you are right that other parts of the iCalendar export have
inconsistent line endings. Currently, VEVENT and VTODO components have
the correct CRLF endings, but the other parts of the VCALENDAR do not
(such as the preamble).

I like your suggestion in the above thread to just wrap the whole
export in `org-icalendar-fold-string'.  Though I think it's slightly
nicer to do it in `org-icalendar--vcalendar' instead of
`org-icalendar-template'.

So, I've attached a standalone patch to do this. It also fixes an issue
with `org-icalendar-fold-string' where the last newline was missing
"\r", and adds a unit test.

Note that fixing the line endings causes a surprising compatibility
issue with org-caldav. I fixed this problem on the org-caldav side, and
made a note in ORG-NEWS.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ox-icalendar-Use-consistent-CRLF-line-endings.patch --]
[-- Type: text/x-patch, Size: 12143 bytes --]

From 712a4ef09b63b2f6bdec2a3967712be912dce0d2 Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Thu, 30 Mar 2023 22:19:09 -0700
Subject: [PATCH] ox-icalendar: Use consistent CRLF line endings

Fixes issue where the ox-icalendar export uses an inconsistent mix of
dos and unix style line endings.

* lisp/ox-icalendar.el (org-icalendar-fold-string): Don't use "\r"
during the string construction, instead replace "\n" with "\r\n" after
string has been created.  This fixes an issue where the final "\n"
added by `org-element-normalize-string' was missing "\r".
(org-icalendar--vevent): Remove call to `org-icalendar-fold-string'.
(org-icalendar--vtodo): Remove call to `org-icalendar-fold-string'.
(org-icalendar--vcalendar): Wrap in `org-icalendar-fold-string'.
* testing/lisp/test-ox-icalendar.el: New file for unit tests of
ox-icalendar.  Add an initial test for CRLF line endings.

See also:

https://list.orgmode.org/87o7oetneo.fsf@localhost/T/#m3e3eb80f9fc51ba75854b33ebfe9ecdefa2ded24

https://list.orgmode.org/orgmode/87ilgljv6i.fsf@localhost/
---
 etc/ORG-NEWS                      |  12 +++
 lisp/ox-icalendar.el              | 159 +++++++++++++++---------------
 testing/lisp/test-ox-icalendar.el |  46 +++++++++
 3 files changed, 138 insertions(+), 79 deletions(-)
 create mode 100644 testing/lisp/test-ox-icalendar.el

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index ac233a986..9f7d01707 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -23,6 +23,18 @@ If you still want to use python-mode with ob-python, you might
 consider [[https://gitlab.com/jackkamm/ob-python-mode-mode][ob-python-mode-mode]], where the code to support python-mode
 has been ported to.
 
+*** =ox-icalendar.el= line ending fix may affect downstream packages
+
+iCalendar export now uses dos-style CRLF ("\r\n") line endings
+throughout, as required by the iCalendar specification (RFC 5545).
+Previously, the export used an inconsistent mix of dos and unix line
+endings.
+
+This might cause errors in external packages that parse output from
+ox-icalendar.  In particular, older versions of org-caldav may
+encounter issues, and users are advised to update to the most recent
+version of org-caldav.  See [[https://github.com/dengste/org-caldav/commit/618bf4cdc9be140ca1993901d017b7f18297f1b8][this org-caldav commit]] for more information.
+
 ** New and changed options
 *** New ~org-cite-natbib-export-bibliography~ option defining fallback bibliography style
 
diff --git a/lisp/ox-icalendar.el b/lisp/ox-icalendar.el
index 81a77a770..06e90d032 100644
--- a/lisp/ox-icalendar.el
+++ b/lisp/ox-icalendar.el
@@ -526,25 +526,27 @@ (defun org-icalendar-cleanup-string (s)
 
 (defun org-icalendar-fold-string (s)
   "Fold string S according to RFC 5545."
-  (org-element-normalize-string
-   (mapconcat
-    (lambda (line)
-      ;; Limit each line to a maximum of 75 characters.  If it is
-      ;; longer, fold it by using "\r\n " as a continuation marker.
-      (let ((len (length line)))
-	(if (<= len 75) line
-	  (let ((folded-line (substring line 0 75))
-		(chunk-start 75)
-		chunk-end)
-	    ;; Since continuation marker takes up one character on the
-	    ;; line, real contents must be split at 74 chars.
-	    (while (< (setq chunk-end (+ chunk-start 74)) len)
-	      (setq folded-line
-		    (concat folded-line "\r\n "
-			    (substring line chunk-start chunk-end))
-		    chunk-start chunk-end))
-	    (concat folded-line "\r\n " (substring line chunk-start))))))
-    (org-split-string s "\n") "\r\n")))
+  (replace-regexp-in-string
+   "\n" "\r\n"
+   (org-element-normalize-string
+    (mapconcat
+     (lambda (line)
+       ;; Limit each line to a maximum of 75 characters.  If it is
+       ;; longer, fold it by using "\r\n " as a continuation marker.
+       (let ((len (length line)))
+	 (if (<= len 75) line
+	   (let ((folded-line (substring line 0 75))
+		 (chunk-start 75)
+		 chunk-end)
+	     ;; Since continuation marker takes up one character on the
+	     ;; line, real contents must be split at 74 chars.
+	     (while (< (setq chunk-end (+ chunk-start 74)) len)
+	       (setq folded-line
+		     (concat folded-line "\n "
+			     (substring line chunk-start chunk-end))
+		     chunk-start chunk-end))
+	     (concat folded-line "\n " (substring line chunk-start))))))
+     (org-split-string s "\n") "\n"))))
 
 
 \f
@@ -736,31 +738,30 @@ (\"PUBLIC\", \"CONFIDENTIAL\", and \"PRIVATE\") are predefined, others
 should be treated as \"PRIVATE\" if they are unknown to the iCalendar server.
 
 Return VEVENT component as a string."
-  (org-icalendar-fold-string
-   (if (eq (org-element-property :type timestamp) 'diary)
-       (org-icalendar-transcode-diary-sexp
-	(org-element-property :raw-value timestamp) uid summary)
-     (concat "BEGIN:VEVENT\n"
-	     (org-icalendar-dtstamp) "\n"
-	     "UID:" uid "\n"
-	     (org-icalendar-convert-timestamp timestamp "DTSTART" nil timezone) "\n"
-	     (org-icalendar-convert-timestamp timestamp "DTEND" t timezone) "\n"
-	     ;; RRULE.
-	     (when (org-element-property :repeater-type timestamp)
-	       (format "RRULE:FREQ=%s;INTERVAL=%d\n"
-		       (cl-case (org-element-property :repeater-unit timestamp)
-			 (hour "HOURLY") (day "DAILY") (week "WEEKLY")
-			 (month "MONTHLY") (year "YEARLY"))
-		       (org-element-property :repeater-value timestamp)))
-	     "SUMMARY:" summary "\n"
-	     (and (org-string-nw-p location) (format "LOCATION:%s\n" location))
-	     (and (org-string-nw-p class) (format "CLASS:%s\n" class))
-	     (and (org-string-nw-p description)
-		  (format "DESCRIPTION:%s\n" description))
-	     "CATEGORIES:" categories "\n"
-	     ;; VALARM.
-	     (org-icalendar--valarm entry timestamp summary)
-	     "END:VEVENT"))))
+  (if (eq (org-element-property :type timestamp) 'diary)
+      (org-icalendar-transcode-diary-sexp
+       (org-element-property :raw-value timestamp) uid summary)
+    (concat "BEGIN:VEVENT\n"
+	    (org-icalendar-dtstamp) "\n"
+	    "UID:" uid "\n"
+	    (org-icalendar-convert-timestamp timestamp "DTSTART" nil timezone) "\n"
+	    (org-icalendar-convert-timestamp timestamp "DTEND" t timezone) "\n"
+	    ;; RRULE.
+	    (when (org-element-property :repeater-type timestamp)
+	      (format "RRULE:FREQ=%s;INTERVAL=%d\n"
+		      (cl-case (org-element-property :repeater-unit timestamp)
+			(hour "HOURLY") (day "DAILY") (week "WEEKLY")
+			(month "MONTHLY") (year "YEARLY"))
+		      (org-element-property :repeater-value timestamp)))
+	    "SUMMARY:" summary "\n"
+	    (and (org-string-nw-p location) (format "LOCATION:%s\n" location))
+	    (and (org-string-nw-p class) (format "CLASS:%s\n" class))
+	    (and (org-string-nw-p description)
+		 (format "DESCRIPTION:%s\n" description))
+	    "CATEGORIES:" categories "\n"
+	    ;; VALARM.
+	    (org-icalendar--valarm entry timestamp summary)
+	    "END:VEVENT")))
 
 (defun org-icalendar--vtodo
     (entry uid summary location description categories timezone class)
@@ -786,34 +787,33 @@ (defun org-icalendar--vtodo
 				 :day-start (nth 3 now)
 				 :month-start (nth 4 now)
 				 :year-start (nth 5 now)))))))
-    (org-icalendar-fold-string
-     (concat "BEGIN:VTODO\n"
-	     "UID:TODO-" uid "\n"
-	     (org-icalendar-dtstamp) "\n"
-	     (org-icalendar-convert-timestamp start "DTSTART" nil timezone) "\n"
-	     (and (memq 'todo-due org-icalendar-use-deadline)
-		  (org-element-property :deadline entry)
-		  (concat (org-icalendar-convert-timestamp
-			   (org-element-property :deadline entry) "DUE" nil timezone)
-			  "\n"))
-	     "SUMMARY:" summary "\n"
-	     (and (org-string-nw-p location) (format "LOCATION:%s\n" location))
-	     (and (org-string-nw-p class) (format "CLASS:%s\n" class))
-	     (and (org-string-nw-p description)
-		  (format "DESCRIPTION:%s\n" description))
-	     "CATEGORIES:" categories "\n"
-	     "SEQUENCE:1\n"
-	     (format "PRIORITY:%d\n"
-		     (let ((pri (or (org-element-property :priority entry)
-				    org-priority-default)))
-		       (floor (- 9 (* 8. (/ (float (- org-priority-lowest pri))
-					    (- org-priority-lowest
-					       org-priority-highest)))))))
-	     (format "STATUS:%s\n"
-		     (if (eq (org-element-property :todo-type entry) 'todo)
-			 "NEEDS-ACTION"
-		       "COMPLETED"))
-	     "END:VTODO"))))
+    (concat "BEGIN:VTODO\n"
+	    "UID:TODO-" uid "\n"
+	    (org-icalendar-dtstamp) "\n"
+	    (org-icalendar-convert-timestamp start "DTSTART" nil timezone) "\n"
+	    (and (memq 'todo-due org-icalendar-use-deadline)
+		 (org-element-property :deadline entry)
+		 (concat (org-icalendar-convert-timestamp
+			  (org-element-property :deadline entry) "DUE" nil timezone)
+			 "\n"))
+	    "SUMMARY:" summary "\n"
+	    (and (org-string-nw-p location) (format "LOCATION:%s\n" location))
+	    (and (org-string-nw-p class) (format "CLASS:%s\n" class))
+	    (and (org-string-nw-p description)
+		 (format "DESCRIPTION:%s\n" description))
+	    "CATEGORIES:" categories "\n"
+	    "SEQUENCE:1\n"
+	    (format "PRIORITY:%d\n"
+		    (let ((pri (or (org-element-property :priority entry)
+				   org-priority-default)))
+		      (floor (- 9 (* 8. (/ (float (- org-priority-lowest pri))
+					   (- org-priority-lowest
+					      org-priority-highest)))))))
+	    (format "STATUS:%s\n"
+		    (if (eq (org-element-property :todo-type entry) 'todo)
+			"NEEDS-ACTION"
+		      "COMPLETED"))
+	    "END:VTODO")))
 
 (defun org-icalendar--valarm (entry timestamp summary)
   "Create a VALARM component.
@@ -879,19 +879,20 @@ (defun org-icalendar--vcalendar (name owner tz description contents)
 NAME, OWNER, TZ, DESCRIPTION and CONTENTS are all strings giving,
 respectively, the name of the calendar, its owner, the timezone
 used, a short description and the other components included."
-  (concat (format "BEGIN:VCALENDAR
+  (org-icalendar-fold-string
+   (concat (format "BEGIN:VCALENDAR
 VERSION:2.0
 X-WR-CALNAME:%s
 PRODID:-//%s//Emacs with Org mode//EN
 X-WR-TIMEZONE:%s
 X-WR-CALDESC:%s
 CALSCALE:GREGORIAN\n"
-		  (org-icalendar-cleanup-string name)
-		  (org-icalendar-cleanup-string owner)
-		  (org-icalendar-cleanup-string tz)
-		  (org-icalendar-cleanup-string description))
-	  contents
-	  "END:VCALENDAR\n"))
+		   (org-icalendar-cleanup-string name)
+		   (org-icalendar-cleanup-string owner)
+		   (org-icalendar-cleanup-string tz)
+		   (org-icalendar-cleanup-string description))
+	   contents
+	   "END:VCALENDAR\n")))
 
 
 \f
diff --git a/testing/lisp/test-ox-icalendar.el b/testing/lisp/test-ox-icalendar.el
new file mode 100644
index 000000000..539d2a0e0
--- /dev/null
+++ b/testing/lisp/test-ox-icalendar.el
@@ -0,0 +1,46 @@
+;;; test-ox-icalendar.el --- tests for ox-icalendar.el  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2023  Jack Kamm
+
+;; Author: Jack Kamm <jackkamm@gmail.com>
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests checking validity of Org iCalendar export output.
+
+;;; Code:
+
+(require 'ox-icalendar)
+
+(ert-deftest test-ox-icalendar/crfl-endings ()
+  "Test every line of iCalendar export has CRFL ending."
+  (should
+   (seq-every-p
+    (lambda (x) (equal (substring x -1) "\r"))
+    (org-split-string
+     (org-test-with-temp-text
+      "* Test event
+:PROPERTIES:
+:ID:       b17d8f92-1beb-442e-be4d-d2060fa3c7ff
+:END:
+<2023-03-30 Thu>"
+      (with-current-buffer
+          (org-export-to-buffer 'icalendar "*Test iCalendar Export*")
+        (buffer-string)))
+     "\n"))))
+
+(provide 'test-ox-icalendar)
+;;; test-ox-icalendar.el ends here
-- 
2.39.2