Package: emacs;
Reported by: Ferdinand Pieper <list_gnu <at> pie.tf>
Date: Tue, 27 Aug 2024 13:52:02 UTC
Severity: normal
Tags: fixed, patch
Fixed in version 31.1
Done: Robert Pluim <rpluim <at> gmail.com>
Bug is archived. No further changes may be made.
Message #11 received at 72831 <at> debbugs.gnu.org (full text, mbox):
From: Robert Pluim <rpluim <at> gmail.com> To: Ferdinand Pieper <list_gnu <at> pie.tf> Cc: Eli Zaretskii <eliz <at> gnu.org>, Andrew G Cohen <cohen <at> andy.bu.edu>, Alexandre Duret-Lutz <adl <at> lrde.epita.fr>, 72831 <at> debbugs.gnu.org Subject: Re: bug#72831: [PATCH] gnus-icalendar: Allow comments in event replies Date: Wed, 28 Aug 2024 15:45:10 +0200
>>>>> On Wed, 28 Aug 2024 14:02:42 +0300, Eli Zaretskii <eliz <at> gnu.org> said:
>> From: Ferdinand Pieper <list_gnu <at> pie.tf>
>> Date: Tue, 27 Aug 2024 11:58:32 +0200
>>
>> Tags: patch
>>
>> The iCalendar RFC allows comments in an event reply to e.g. indicate
>> a reason for declining. This patch implements sending an additional
>> comment if the usual functions
>> `gnus-icalendar-reply-[accept,decline,tentative]` (bound to `i a`,
>> `i d`, `i t`) are called with a prefix argument.
>>
>> Please let me know any feedback regarding the patch and commit
>> message formatting.
Now that you have the bug number, please put (Bug#72831) somewhere in
the commit message.
>>
>> Notably this only implements the comment functionality for the
>> functions mentioned above and does not include the buttons defined
>> in `gnus-icalendar-event:inline-reply-buttons`, as I am not sure how
>> the `read-string` for the user's comment should be implemented for
>> them.
You can pass prefix args to commands bound to mouse clicks as well as
keys pressed inside the button, but getting the pieces to work would
take some experimentation. I donʼt think thatʼs necessary for an
initial implementation though.
Eli> Thanks.
Eli> Robert, Andrew, and Alexandre: would you please review the proposed
Eli> changes below and comment?
I think a change this size requires copyright assigment, which I donʼt
know if youʼve done.
>> >From 8ff07723b407d56de72ddfea98b8931951bb7ad8 Mon Sep 17 00:00:00 2001
>> From: fpi <git <at> pie.tf>
>> Date: Mon, 19 Aug 2024 17:18:11 +0200
>> Subject: [PATCH] Allow comments to organizer in ical event replies
>>
>> * 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.
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.
>> ---
>> lisp/gnus/gnus-icalendar.el | 52 ++++++++++++++++++++++++++-----------
>> 1 file changed, 37 insertions(+), 15 deletions(-)
>>
>> diff --git a/lisp/gnus/gnus-icalendar.el b/lisp/gnus/gnus-icalendar.el
>> index af7284b88e8..7a295fb9ed2 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,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?
>> (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
Could you describe the COMMENT arg in the docstring?
>> @@ -396,7 +406,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 +888,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 +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."
>> (interactive nil 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."
>> +(defun gnus-icalendar-reply-tentative (&optional comment-p)
>> + "Send tentative response to invitation in the current article.
>> +
>> +With a prefix `\\[universal-argument]' prompt for a comment to include
>> +in the reply."
>> (interactive nil 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.
>> +
>> +With a prefix `\\[universal-argument]' 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.30.2
>>
Ideally youʼd add test cases for this to
"test/lisp/gnus/gnus-icalendar-tests.el". But thatʼs not mandatory.
Robert
--
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.