Hello,
Michaël Cadilhac <mich...@cadilhac.name> writes:
> Well, certainly. I may not have had the best discipline in writing
> these, so turning them into patches is a bit painful. Let me know if
> I can make things better. (I believe my FSF paperwork is still
> alright, if need be.)
Thank you! Comments follow.
> +(defcustom org-icalendar-bump-todos nil
> + "Non-nil means that pending TODO VEVENTs are bumped to today.
> +In addition, if non-nil, the number of late days is indicated in the
> summary."
> + :group 'org-export-icalendar
> + :type 'boolean)
This variable is missing :package-version '(Org . "9.3") and :safe
keywords. This is also true for other defcustom introduced throughout
your patches.
> +(defun org-icalendar-today-timestamp ()
> + "Return a TIMESTAMP object for today, at 00:00."
> + (let ((dt (decode-time)))
> + (list 'timestamp
> + (nconc (list :year-start (nth 5 dt)
> + :year-end (nth 5 dt)
> + :month-start (nth 4 dt)
> + :month-end (nth 4 dt)
> + :day-start (nth 3 dt)
> + :day-end (nth 3 dt))))))
This function already exists: `org-timestamp-from-time'.
> +(defun org-icalendar-days-until-timestamp (timestamp)
> + "Return the number of days until TIMESTAMP.
> +
> +If TIMESTAMP occurs today, return 0.
> +If TIMESTAMP occurs yesterday, return -1."
> + (floor
> + (/ (float-time
> + (time-subtract (org-timestamp--to-internal-time timestamp)
> + (apply 'encode-time
> + (append '(0 0 0) (nthcdr 3 (decode-time))))))
(time-substract (org-timestamp-to-time timestamp)
(current-time))
> + (* 60 60 24))))
>
> - (concat "DL: " summary) loc desc cat tz class)))
> + (concat "DL: " summary) loc desc cat tz class))
> +
Spurious blank line.
> + ;; Case 2: pending TODO overdue and should bump.
> + ((and org-icalendar-bump-todos
> + (< days-until-deadline 0))
> + (org-icalendar--vevent
> + entry (org-icalendar-today-timestamp) (concat "DL-" uid)
> + (concat "DL (" (number-to-string (- days-until-deadline))
> + "x): " summary)
Nitpick:
(format "DL (%d)x): %s" (- days-until-deadline) summary
is more readable.
> + loc desc cat tz class))
> + ;; Case 3: in the future and should warn.
> + (t
> + (concat
> + ;; If in the warning zone.
> + (when (<= days-until-deadline org-deadline-warning-days)
> + (org-icalendar--vevent
> + entry (org-icalendar-today-timestamp) (concat "DL-" uid)
> + (concat "DL (in " (number-to-string days-until-deadline)
> + "d.): " summary)
See above.
> + ;; Overdue and should bump.
> + (t
> + (org-icalendar--vevent
> + entry (org-icalendar-today-timestamp) (concat "SC-" uid)
> + (concat "S (" (number-to-string (- days-until-scheduled))
> "x): "
> + summary)
Ditto.
> Subject: [PATCH 2/6] Include tags in SUMMARY if so desired.
I'm ignoring this patch since it is superseded by the fifth. Could you
merge them?
> #+vindex: org-icalendar-categories
> #+vindex: org-icalendar-alarm-time
> +#+vindex: org-icalendar-summary-uses-tags
> For tags on the headline, the iCalendar export back-end makes them
> into iCalendar categories. To tweak the inheritance of tags and TODO
> states, configure the variable ~org-icalendar-categories~. To assign
> clock alarms based on time, configure the ~org-icalendar-alarm-time~
> -variable.
> +variable. Additionally, the variable
> +~org-icalendar-summary-uses-tags~ can be configured to have tags
> +appear in the title of the event, when SUMMARY is not provided.
I think this should go near the paragraph about
`org-calendar-include-body'.
> (defcustom org-icalendar-summary-uses-tags nil
> - "Non-nil means that TITLE+TAGS is used when SUMMARY is not provided.
> -If nil, then TITLE is used alone."
> + "Whether tags should be added to the title to create the summary.
Nitpick.
"Non-nil means" > "Whether" because it is more unambiguously informative
When non-nil, tags are added to ...
or
Non-nil means tags are added to ...
> (let ((tags (apply 'concat
> - (mapcar (lambda (x) (concat ":" x))
> - (org-export-get-tags entry info nil t)))))
> + (mapcar
> + (lambda (x) (concat ":" x))
> + (org-export-get-tags
> + entry info nil
> + (eq org-icalendar-summary-uses-tags
> 'all-tags))))))
(apply 'concat (lambda ...) ...) => (mapconcat (lambda ...) ...)
Could you send an updated patch series?
Thank you again.
Regards,
--
Nicolas Goaziou
