Re: org-bbdb-anniversaries-future

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

Michael Welle <mwe012008@gmx.net> writes:

> Maybe it's a tiny change, if we strip the comments ;). It's not too
> sophisticated, everyone could have done it.

I definitely think it _is_ a tiny change.

Some comments follow.

> +(defun org-bbdb-anniversary-description (agenda-date anniv-date)
> +  "Return a string used to modify an agenda anniversary entry. The 

The first line needs to be a sentence on its own. IOW, the last "The"
belong to the next line.
> +   calculation of the string is based on the difference between

Indentation in this line and all subsequent ones needs to be removed.

> +   the anniversary date and the date on which the entry appears
> +   in the agenda. This makes it possible to have different entries

Docstrings follow US English conventions, i.e., two spaces are required
after a full stop.

> +   for the same event depending on if it occurs in the next few days
> +   or far away in the future."

You also need to document what are the arguments (type).  They are
written up-cased in the docstring.

> +  (let ((delta (- (calendar-absolute-from-gregorian anniv-date)
> +                  (calendar-absolute-from-gregorian agenda-date))))
> +
> +    (cond
> +     ((= delta 0) " -- today\\&")
> +     ((= delta 1) " -- tomorrow\\&")
> +     ((< delta 7) (format " -- in %d days\\&" delta))
> +     ((format " -- %d-%02d-%02d\\&" (third anniv-date) (first anniv-date) (second anniv-date))))))

You need to used "cl-" prefix, e.g., `cl-third'. OTOH, I'd rather use
pattern matching here, for clarity:

  (pcase-let ((`(,month ,day ,year) anniv-date))
    (format " -- %s-%02%d-%02d\\&" year month day))

Also, could you provide a commit message?

Thank you.

Regards,

-- 
Nicolas Goaziou