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

[Jack Kamm <jackkamm@gmail.com>]

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

Ihor Radchenko <yantar92@posteo.net> writes:

> Looks reasonable, but I have one comment on the code.
> We should not use user-defined hooks for things that must be executed.
> Instead, we should better explicitly call the necessary functions.

Thanks, I updated the patch to explicitly call the function.

Also, I tweaked the coding-system-for-write to be a bit safer, in case
of edge cases where utf-8 doesn't work -- I think RFC 5545 just says
it's the default charset.

Attached is the (I think) final version of the patch. I'll install it
soon, unless I hear otherwise.

PS I haven't forgotten your feedback on the original VTODO-related
patches (thanks for that review). I'll work on that next, but it might
take me a bit longer.


[-- 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: 6850 bytes --]

From aa59625cd08dcee767f42ad8d45d8902aa8d38bd Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Sat, 1 Apr 2023 16:53:35 -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): No longer converts
to CRLF, instead delegating that to `org-icalendar--post-process-file'.
(org-icalendar--post-process-file): New function to handle exported
file post-processing.  Converts EOL to CRLF, and then runs
`org-icalendar-after-save-hook'.
(org-icalendar-export-to-ics, org-icalendar-export-current-agenda,
org-icalendar--combine-files): Call `org-icalendar--post-process-file'
instead of running `org-icalendar-after-save-hook' directly.
* 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              | 27 ++++++++++++-------
 testing/lisp/test-ox-icalendar.el | 44 +++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 9 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..ccc237721 100644
--- a/lisp/ox-icalendar.el
+++ b/lisp/ox-icalendar.el
@@ -540,12 +540,23 @@ (defun org-icalendar-fold-string (s)
 	    ;; 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 "
+		    (concat folded-line "\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")))
-
+	    (concat folded-line "\n " (substring line chunk-start))))))
+    (org-split-string s "\n") "\n")))
+
+(defun org-icalendar--post-process-file (file)
+  "Post-process the exported iCalendar FILE.
+Converts line endings to dos-style CRLF as per RFC 5545, then
+runs `org-icalendar-after-save-hook'."
+  (with-temp-buffer
+    (insert-file-contents file)
+    (let ((coding-system-for-write (coding-system-change-eol-conversion
+                                    last-coding-system-used 'dos)))
+      (write-region nil nil file)))
+  (run-hook-with-args 'org-icalendar-after-save-hook file)
+  nil)
 
 \f
 ;;; Filters
@@ -932,8 +943,7 @@ (defun org-icalendar-export-to-ics
     (org-export-to-file 'icalendar outfile
       async subtreep visible-only body-only
       '(:ascii-charset utf-8 :ascii-links-to-notes nil)
-      '(lambda (file)
-	 (run-hook-with-args 'org-icalendar-after-save-hook file) nil))))
+      #'org-icalendar--post-process-file)))
 
 ;;;###autoload
 (defun org-icalendar-export-agenda-files (&optional async)
@@ -1019,7 +1029,7 @@ (defun org-icalendar-export-current-agenda (file)
 	(or (org-string-nw-p org-icalendar-timezone) (format-time-string "%Z"))
 	org-icalendar-combined-description
 	contents)))
-    (run-hook-with-args 'org-icalendar-after-save-hook file)))
+    (org-icalendar--post-process-file file)))
 
 (defun org-icalendar--combine-files (&rest files)
   "Combine entries from multiple files into an iCalendar file.
@@ -1061,8 +1071,7 @@ (defun org-icalendar--combine-files (&rest files)
 	       (when (and org-icalendar-include-bbdb-anniversaries
 			  (require 'ol-bbdb nil t))
 		 (with-output-to-string (org-bbdb-anniv-export-ical)))))))
-	  (run-hook-with-args 'org-icalendar-after-save-hook
-			      org-icalendar-combined-agenda-file))
+	  (org-icalendar--post-process-file org-icalendar-combined-agenda-file))
       (org-release-buffers org-agenda-new-buffers))))
 
 
diff --git a/testing/lisp/test-ox-icalendar.el b/testing/lisp/test-ox-icalendar.el
new file mode 100644
index 000000000..bfc756d51
--- /dev/null
+++ b/testing/lisp/test-ox-icalendar.el
@@ -0,0 +1,44 @@
+;;; 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/crlf-endings ()
+  "Test every line of iCalendar export has CRLF ending."
+  (let ((tmp-ics (org-test-with-temp-text-in-file
+                  "* Test event
+:PROPERTIES:
+:ID:       b17d8f92-1beb-442e-be4d-d2060fa3c7ff
+:END:
+<2023-03-30 Thu>"
+                  (expand-file-name (org-icalendar-export-to-ics)))))
+    (unwind-protect
+        (with-temp-buffer
+          (insert-file-contents tmp-ics)
+          (should (eql 1 (coding-system-eol-type last-coding-system-used))))
+      (when (file-exists-p tmp-ics) (delete-file tmp-ics)))))
+
+(provide 'test-ox-icalendar)
+;;; test-ox-icalendar.el ends here
-- 
2.39.2