emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Nick Dokos <ndokos@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: [PATCH] Allow early-warning anniversaries in agends [was: Re: org-bbdb-birthday reminder]
Date: Mon, 17 Aug 2015 11:16:35 +0200	[thread overview]
Message-ID: <87lhda6zrg.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <87pp2n6t7k.fsf_-_@pierrot.dokosmarshall.org> (Nick Dokos's message of "Sun, 16 Aug 2015 13:25:51 -0400")

Hello,

Nick Dokos <ndokos@gmail.com> writes:

> Here's a patch to add a new function to org-bbdb.el that provides early
> warning for upcoming anniversaries; it also adds some documentation to
> org.texi.

Thank you. Some comments follow.

> +(defun org-bbdb-date-list (date n)
> +  "Return a list of dates in (m d y) format from the given 'date' to n-1 days hence."
> +  (let ((abs (calendar-absolute-from-gregorian date))
> +        ret)
> +    (reverse (dotimes (i n ret)
> +	       (setq ret (cons (calendar-gregorian-from-absolute (+ abs i)) ret))))))

  (dotimes (i n (nreverse ret))
    (push (calendar-gregorian-from-absolute (+ abs i)) ret))

> +
> +;;;###autoload
> +(defun org-bbdb-anniversaries-future (&optional n)
> +  "Return list of anniversaries for today and the next n-1 days (default n=7)."
> +  (if (not n) (setq n 7))

  (let ((n (or n 7)))
    (when (<= n 0) (error "..."))
    (let* (...)))
 
> +  (if (<= n 0) nil
> +    (let* (
> +	   ;; list of relevant dates.

You need to capitalize comments.

> +	   (dates (org-bbdb-date-list date n))
> +	   ;; Function to annotate text of each element of l with the anniversary date d.
> +	   (annotate-descriptions
> +	    (lambda (d l)
> +	      (let ((modify-description
> +		     (lambda (x)
> +		       ;; The assumption here is that x is a bbdb link of the form
> +		       ;; [[bbdb:name][description]]
> +		       ;; This function rather arbitrarily modifies the description
> +		       ;; by adding the date to it in a fixed format.

Missing full stop in first sentence.

> +		       (string-match "]]" x)
> +		       (replace-match (format " -- %d-%02d-%02d\\&" (third d) (first d) (second d))
> +				      nil nil x))))
> +		(mapcar modify-description l))))

Why don't you simply write

  (mapcar (lambda (x) (string-match "]]" x) ...)
          l)

?

> +	   ;; Function to generate the list of annotated anniversaries
> +	   ;; for the given date d.
> +	   (gen-anniversaries
> +	    (lambda (d)
> +	      (let ((date d))
> +		;; rebind 'date' so that org-bbdb-anniversaries will be
> +		;; fooled into giving us the list for the given date
> +		;; and then annotate the descriptions for that date.

"Rebind".

> +		(funcall annotate-descriptions d (org-bbdb-anniversaries))))))
> +      ;; map the gen-anniversaries function over the dates
> +      ;; and nconc the results into a single list

Missing capitalization and full stop.

> +      (apply 'nconc (mapcar gen-anniversaries dates)))))

#'nconc (there is also `cl-mapcan').

Also, gen-anniversaries is a one-liner. I don't think it deserves its
own name.

IOW, wouldn't it be better to use less levels of indirection, i.e., less
helper functions?


Regards,

-- 
Nicolas Goaziou

  reply	other threads:[~2015-08-17  9:15 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-29 13:41 org-bbdb-birthday reminder Julien Cubizolles
2015-08-12 20:55 ` Matt Lundin
2015-08-13  1:36   ` Nick Dokos
2015-08-13  6:35   ` Julien Cubizolles
2015-08-13 13:07     ` Matt Lundin
2015-08-14  0:26       ` Nick Dokos
2015-08-14 14:30         ` Matt Lundin
2015-08-15 18:24           ` Julien Cubizolles
2015-08-16 17:25             ` [PATCH] Allow early-warning anniversaries in agends [was: Re: org-bbdb-birthday reminder] Nick Dokos
2015-08-17  9:16               ` Nicolas Goaziou [this message]
2015-08-17 16:42                 ` Nick Dokos
2015-08-17 16:57                   ` Rasmus
2015-08-17 17:35                     ` Nicolas Goaziou
2015-10-06 15:11                       ` Nick Dokos
2015-10-07 19:58                         ` Nicolas Goaziou
2015-10-13 15:54                           ` Nick Dokos
2015-10-14 23:21                           ` Nick Dokos
2015-08-16 13:46           ` org-bbdb-birthday reminder Bastien Guerry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lhda6zrg.fsf@nicolasgoaziou.fr \
    --to=mail@nicolasgoaziou.fr \
    --cc=emacs-orgmode@gnu.org \
    --cc=ndokos@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).