bug#72831: [PATCH] gnus-icalendar: Allow comments in event replies

[Ferdinand Pieper <list_gnu@pie.tf>]

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

Thanks for the review.

Robert Pluim <rpluim@gmail.com> writes:

> Now that you have the bug number, please put (Bug#72831) somewhere in
> the commit message.

Updated in the patch attached. 

> I think a change this size requires copyright assigment, which I donʼt
> know if youʼve done.

Copyright assignment is done. I wrote some prior contributions to org-mode.

> I think that if youʼre making the same comment for the three functions
> you can put all three in one set of (), separated by commas.

Updated.

>     >> @@ -319,6 +319,10 @@ gnus-icalendar-event--build-reply-event-body
>     >> (if (string-match "^[^:]+:" line)
>     >> (replace-match (format "\\&%s: " summary-status) t nil line)
>     >> line))
>     >> +         (update-comment
>     >> +           (line)
>     >> +           (if comment (format "COMMENT:%s" comment)
>     >> +             line))
>     >> (update-dtstamp ()
>     >> (format-time-string "DTSTAMP:%Y%m%dT%H%M%SZ" nil t))
>     >> (attendee-matches-identity
>     >> @@ -341,6 +345,7 @@ gnus-icalendar-event--build-reply-event-body
>     >> (cond
>     >> ((string= key "ATTENDEE") (update-attendee-status line))
>     >> ((string= key "SUMMARY") (update-summary line))
>     >> +		     ((string= key "COMMENT") (update-comment line))
>     >> ((string= key "DTSTAMP") (update-dtstamp))
>     >> ((member key '("ORGANIZER" "DTSTART" "DTEND"
>     >> "LOCATION" "DURATION" "SEQUENCE"
>     >> @@ -363,12 +368,17 @@ gnus-icalendar-event--build-reply-event-body
>     >> attendee-status user-full-name user-mail-address)
>     >> reply-event-lines))
>     >> 
>     >> +      ;; add comment line if not existing
>     >> + (when (and comment (not (gnus-icalendar-find-if (lambda (x)
>     >> (string-match "^COMMENT" x))
>     >> + reply-event-lines)))
>     >> +        (push (format "COMMENT:%s" comment) reply-event-lines))
>     >> +
>
> So if the event was sent with a COMMENT the receiver canʼt add their
> own? That doesnʼt match my conception of 'reply with comment'. Iʼm
> hazy on whatʼs exactly allowed in ical, can you have more than one
> COMMENT line? Or we could combine the comments?

If it already exists it is replaced by the prior
>    >> ((string= key "COMMENT") (update-comment line))
Just if it does not exist the `(string= key "COMMENT")` never matches and we have to add the field.

>     >> -(defun gnus-icalendar-event-reply-from-buffer (buf status identities)
>     >> +(defun gnus-icalendar-event-reply-from-buffer (buf status
>     >> identities &optional comment)
>     >> "Build a calendar event reply for request contained in BUF.
>     >> The reply will have STATUS (`accepted', `tentative' or  `declined').
>     >> The reply will be composed for attendees matching any entry
>
> Could you describe the COMMENT arg in the docstring?

Updated.

>     >> @@ -1009,25 +1019,37 @@ gnus-icalendar-save-event
>     >> (when data
>     >> (gnus-icalendar-save-part data))))
>     >> 
>     >> -(defun gnus-icalendar-reply-accept ()
>     >> -  "Accept invitation in the current article."
>     >> +(defun gnus-icalendar-reply-accept (&optional comment-p)
>     >> +  "Accept invitation in the current article.
>     >> +
>     >> +With a prefix `\\[universal-argument]' prompt for a comment to include
>     >> +in the reply."
>
> I think we tend to word this as
>
> "Optional argument COMMENT-P (interactively the prefix argument) means
> prompt for a comment to include in the reply."

Updated using `\\[universal-argument]'. Or does that not matter here and just "prefix argument" would be fine?

> Ideally youʼd add test cases for this to
> "test/lisp/gnus/gnus-icalendar-tests.el". But thatʼs not mandatory.

I looked into it and will add a few tests for accepting/declining events with and without comments. I will followup on this in a couple days.
It might also make sense to extend the parsing of events to recognize comments (and potentially display them inside emacs.


I also missed to update the interactive argument for `gnus-icalendar-reply-accept` and `gnus-icalendar-reply-tentative`, which is now fixed in the updated patch.


Best,



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-comments-to-organizer-in-event-replies-bug-728.patch --]
[-- Type: text/x-patch, Size: 7137 bytes --]

From 35dab0bb7131cf64873298cbe9a418f82d2a2508 Mon Sep 17 00:00:00 2001
From: fpi <git@pie.tf>
Date: Wed, 28 Aug 2024 18:33:20 +0200
Subject: [PATCH] Allow comments to organizer in event replies (bug#72831)

* lisp/gnus/gnus-icalendar.el
(gnus-icalendar-event--build-reply-event-body): Add optional COMMENT
argument to be inserted into the reply.
(gnus-icalendar-event-reply-from-buffer): Add COMMENT argument to be
passed through to gnus-icalendar-event--build-reply-event-body
(gnus-icalendar-reply-accept, gnus-icalendar-reply-tentative,
gnus-icalendar-reply-decline): If interactively called with a prefix
argument ask user for a COMMENT to add to the reply.
---
 lisp/gnus/gnus-icalendar.el | 61 ++++++++++++++++++++++++++-----------
 1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/lisp/gnus/gnus-icalendar.el b/lisp/gnus/gnus-icalendar.el
index af7284b88e8..85f1aa09a56 100644
--- a/lisp/gnus/gnus-icalendar.el
+++ b/lisp/gnus/gnus-icalendar.el
@@ -309,7 +309,7 @@ gnus-icalendar-event-from-buffer
 ;;; gnus-icalendar-event-reply
 ;;;
 
-(defun gnus-icalendar-event--build-reply-event-body (ical-request status identities)
+(defun gnus-icalendar-event--build-reply-event-body (ical-request status identities &optional comment)
   (let ((summary-status (capitalize (symbol-name status)))
         (attendee-status (upcase (symbol-name status)))
         reply-event-lines)
@@ -319,6 +319,10 @@ gnus-icalendar-event--build-reply-event-body
 	  (if (string-match "^[^:]+:" line)
 	      (replace-match (format "\\&%s: " summary-status) t nil line)
 	    line))
+         (update-comment
+           (line)
+           (if comment (format "COMMENT:%s" comment)
+             line))
 	 (update-dtstamp ()
 			 (format-time-string "DTSTAMP:%Y%m%dT%H%M%SZ" nil t))
 	 (attendee-matches-identity
@@ -341,6 +345,7 @@ gnus-icalendar-event--build-reply-event-body
 		    (cond
 		     ((string= key "ATTENDEE") (update-attendee-status line))
 		     ((string= key "SUMMARY") (update-summary line))
+		     ((string= key "COMMENT") (update-comment line))
 		     ((string= key "DTSTAMP") (update-dtstamp))
 		     ((member key '("ORGANIZER" "DTSTART" "DTEND"
 				    "LOCATION" "DURATION" "SEQUENCE"
@@ -363,16 +368,24 @@ gnus-icalendar-event--build-reply-event-body
                       attendee-status user-full-name user-mail-address)
               reply-event-lines))
 
+      ;; add comment line if not existing
+      (when (and comment (not (gnus-icalendar-find-if (lambda (x) (string-match "^COMMENT" x))
+                                                      reply-event-lines)))
+        (push (format "COMMENT:%s" comment) reply-event-lines))
+
       (mapconcat #'identity `("BEGIN:VEVENT"
                               ,@(nreverse reply-event-lines)
                               "END:VEVENT")
                  "\n"))))
 
-(defun gnus-icalendar-event-reply-from-buffer (buf status identities)
+(defun gnus-icalendar-event-reply-from-buffer (buf status identities &optional comment)
   "Build a calendar event reply for request contained in BUF.
 The reply will have STATUS (`accepted', `tentative' or  `declined').
 The reply will be composed for attendees matching any entry
-on the IDENTITIES list."
+on the IDENTITIES list.
+Optional argument COMMENT will be placed in the comment field of the
+reply.
+"
   (cl-labels
       ((extract-block
 	(blockname)
@@ -396,7 +409,7 @@ gnus-icalendar-event-reply-from-buffer
                               "PRODID:Gnus"
                               "VERSION:2.0"
                               zone
-                              (gnus-icalendar-event--build-reply-event-body event status identities)
+                              (gnus-icalendar-event--build-reply-event-body event status identities comment)
                               "END:VCALENDAR")))
 
           (mapconcat #'identity (delq nil contents) "\n"))))))
@@ -878,13 +891,13 @@ gnus-icalendar-send-buffer-by-mail
       (insert "Subject: " subject)
       (message-send-and-exit))))
 
-(defun gnus-icalendar-reply (data)
+(defun gnus-icalendar-reply (data &optional comment)
   (let* ((handle (car data))
          (status (cadr data))
          (event (caddr data))
          (reply (gnus-icalendar-with-decoded-handle handle
                   (gnus-icalendar-event-reply-from-buffer
-                   (current-buffer) status (gnus-icalendar-identities))))
+                   (current-buffer) status (gnus-icalendar-identities) comment)))
          (organizer (gnus-icalendar-event:organizer event)))
 
     (when reply
@@ -1009,25 +1022,37 @@ gnus-icalendar-save-event
     (when data
       (gnus-icalendar-save-part data))))
 
-(defun gnus-icalendar-reply-accept ()
-  "Accept invitation in the current article."
-  (interactive nil gnus-article-mode gnus-summary-mode)
+(defun gnus-icalendar-reply-accept (&optional comment-p)
+  "Accept invitation in the current article.
+
+Optional argument COMMENT-P (interactively the `\\[universal-argument]')
+means prompt for a comment to include in the reply."
+  (interactive "P" gnus-article-mode gnus-summary-mode)
   (with-current-buffer gnus-article-buffer
-    (gnus-icalendar-reply (list gnus-icalendar-handle 'accepted gnus-icalendar-event))
+    (gnus-icalendar-reply (list gnus-icalendar-handle 'accepted gnus-icalendar-event)
+                          (when comment-p (read-string "Comment: ")))
     (setq-local gnus-icalendar-reply-status 'accepted)))
 
-(defun gnus-icalendar-reply-tentative ()
-  "Send tentative response to invitation in the current article."
-  (interactive nil gnus-article-mode gnus-summary-mode)
+(defun gnus-icalendar-reply-tentative (&optional comment-p)
+  "Send tentative response to invitation in the current article.
+
+Optional argument COMMENT-P (interactively the `\\[universal-argument]')
+means prompt for a comment to include in the reply."
+  (interactive "P" gnus-article-mode gnus-summary-mode)
   (with-current-buffer gnus-article-buffer
-    (gnus-icalendar-reply (list gnus-icalendar-handle 'tentative gnus-icalendar-event))
+    (gnus-icalendar-reply (list gnus-icalendar-handle 'tentative gnus-icalendar-event)
+                          (when comment-p (read-string "Comment: ")))
     (setq-local gnus-icalendar-reply-status 'tentative)))
 
-(defun gnus-icalendar-reply-decline ()
-  "Decline invitation in the current article."
-  (interactive nil gnus-article-mode gnus-summary-mode)
+(defun gnus-icalendar-reply-decline (&optional comment-p)
+  "Decline invitation in the current article.
+
+Optional argument COMMENT-P (interactively the `\\[universal-argument]')
+means prompt for a comment to include in the reply."
+  (interactive "P" gnus-article-mode gnus-summary-mode)
   (with-current-buffer gnus-article-buffer
-    (gnus-icalendar-reply (list gnus-icalendar-handle 'declined gnus-icalendar-event))
+    (gnus-icalendar-reply (list gnus-icalendar-handle 'declined gnus-icalendar-event)
+                          (when comment-p (read-string "Comment: ")))
     (setq-local gnus-icalendar-reply-status 'declined)))
 
 (defun gnus-icalendar-event-export ()
-- 
2.44.1