unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
@ 2021-08-11  7:13 Pankaj Jangid
  2021-08-13 19:35 ` Eric Abrahamsen
  0 siblings, 1 reply; 13+ messages in thread
From: Pankaj Jangid @ 2021-08-11  7:13 UTC (permalink / raw)
  To: 49995; +Cc: Eric Abrahamsen


Prerequisite: setup EBDB with a few records with anniversaries.

Steps:

1. M-x calendar RET

2. m (diary-mark-entries)

Result: highlights the entries from diary but doesn’t mark the
anniversaries from EBDB. However, pressing ‘d’ on a date with
anniversary shows the day’s calendar with the anniversary.

Expectation: Anniversaries from EBDB must also be highlighted when
(diary-mark-entries) is invoked.


In GNU Emacs 28.0.50 (build 1, x86_64-apple-darwin20.6.0, NS appkit-2022.60 Version 11.5.1 (Build 20G80))
 of 2021-08-11 built on mb2.local
Repository revision: a8e89964f3553f40b8807617c3b181f42cd22fd9
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2022
System Description:  macOS 11.5.1

Configured features:
ACL DBUS GIF GLIB GMP GNUTLS JPEG JSON LCMS2 LIBXML2 MODULES NOTIFY
KQUEUE NS PDUMPER PNG RSVG THREADS TIFF TOOLKIT_SCROLL_BARS XIM ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Info

Minor modes in effect:
  global-company-mode: t
  company-mode: t
  shell-dirtrack-mode: t
  savehist-mode: t
  desktop-save-mode: t
  show-paren-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
/Users/pankaj/.emacs.d/elpa/magit-20210810.800/magit-section-pkg hides /Users/pankaj/.emacs.d/elpa/magit-section-20210806.1607/magit-section-pkg
/Users/pankaj/.emacs.d/elpa/transient-20210809.1242/transient hides /Users/pankaj/Applications/Emacs.app/Contents/Resources/lisp/transient

Features:
(shadow emacsbug magit-utils dash appt url-http url-gw url-auth
smerge-mode diff gnus-html url-cache flow-fill shr-color color mailalias
smtpmail flyspell ispell sort smiley gnus-cite mm-archive mail-extr
gnus-async gnus-bcklg qp gnus-ml hl-line disp-table cursor-sensor
nndraft nnmh utf-7 nnml nnfolder epa-file gnutls network-stream nsm
gnus-agent gnus-srvr gnus-score score-mode nnvirtual nntp gnus-cache
.gnus cal-persia cal-coptic cal-mayan cal-french cal-move rfc1345
misearch multi-isearch cl-print help-fns radix-tree cus-start compose
quail cal-julian org-duration view cal-china lunar solar cal-dst
cal-bahai cal-islam cal-hebrew holidays hol-loaddefs cal-iso face-remap
org-agenda org-refile vc-dir diary-lib diary-loaddefs sh-script smie
executable dired-aux bug-reference conf-mode hideshow vc-mtn vc-hg
vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs vc eglot array jsonrpc ert pp
ewoc debug backtrace imenu ol-eww eww xdg url-queue mm-url ol-rmail
ol-mhe ol-irc ol-info ol-gnus nnselect gnus-search ol-docview doc-view
image-mode exif ol-bibtex bibtex ol-bbdb ol-w3m autorevert filenotify
checkdoc lisp-mnt vc-git diff-mode vc-dispatcher flymake-proc flymake
compile warnings thingatpt elec-pair jka-compr company-oddmuse
company-keywords company-etags etags fileloop xref project company-gtags
company-dabbrev-code company-dabbrev company-files company-clang
company-capf company-cmake company-semantic company-template
company-bbdb company init my-init enet org-element avl-tree generator
org org-macro org-footnote org-pcomplete org-list org-faces org-entities
noutline outline easy-mmode org-version ob-plantuml ob-sql ob-css ob-js
ob-java ob-C cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs ob-python python tramp-sh tramp
tramp-loaddefs trampver tramp-integration files-x tramp-compat shell
pcomplete ls-lisp ob-R ob ob-tangle org-src ob-ref ob-lob ob-table
ob-exp ob-comint comint ansi-color ring ob-emacs-lisp ob-core ob-eval
org-table ol org-keys org-compat advice org-macs org-loaddefs
format-spec server edmacro kmacro modus-operandi-theme modus-themes
exec-path-from-shell delight ebdb-message sendmail ebdb-gnus gnus-msg
gnus-art mm-uu mml2015 mm-view mml-smime smime dig gnus-sum shr kinsoku
svg dom gnus-group gnus-undo gnus-start gnus-dbus dbus xml gnus-cloud
nnimap nnmail mail-source utf7 netrc nnoo parse-time iso8601 gnus-spec
gnus-int gnus-range message rmc puny dired dired-loaddefs rfc822 mml
mml-sec epa derived epg epg-config mm-decode mm-bodies mm-encode
mail-parse rfc2231 gmm-utils mailheader gnus-win gnus nnheader gnus-util
rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums text-property-search
time-date mail-utils mm-util mail-prsvr wid-edit ebdb-mua ebdb-com crm
ebdb-format ebdb mailabbrev eieio-opt cl-extra help-mode speedbar
ezimage dframe find-func eieio-base pcase cal-menu calendar cal-loaddefs
timezone savehist desktop frameset avoid paren cus-load finder-inf
tex-site info package browse-url url url-proxy url-privacy url-expand
url-methods url-history url-cookie url-domsuf url-util mailcap
url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json subr-x map url-vars seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib early-init iso-transl
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/ns-win ns-win ucs-normalize mule-util term/common-win
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
tab-bar menu-bar rfn-eshadow isearch easymenu timer select scroll-bar
mouse jit-lock font-lock syntax font-core term/tty-colors frame
minibuffer cl-generic cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite charscript charprop case-table epa-hook jka-cmpr-hook help
simple abbrev obarray cl-preloaded nadvice button loaddefs faces
cus-face macroexp files window text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind kqueue cocoa ns lcms2 multi-tty
make-network-process emacs)

Memory information:
((conses 16 837741 107231)
 (symbols 48 45628 3)
 (strings 32 198620 15218)
 (string-bytes 1 5983840)
 (vectors 16 96769)
 (vector-slots 8 1944827 135657)
 (floats 8 997 1126)
 (intervals 56 8961 8042)
 (buffers 992 72))

-- 
Regards ~Pankaj





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
  2021-08-11  7:13 bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar Pankaj Jangid
@ 2021-08-13 19:35 ` Eric Abrahamsen
  2021-08-14 15:20   ` Michael Heerdegen
  0 siblings, 1 reply; 13+ messages in thread
From: Eric Abrahamsen @ 2021-08-13 19:35 UTC (permalink / raw)
  To: Pankaj Jangid; +Cc: 49995

Pankaj Jangid <pankaj@codeisgreat.org> writes:

> Prerequisite: setup EBDB with a few records with anniversaries.
>
> Steps:
>
> 1. M-x calendar RET
>
> 2. m (diary-mark-entries)
>
> Result: highlights the entries from diary but doesn’t mark the
> anniversaries from EBDB. However, pressing ‘d’ on a date with
> anniversary shows the day’s calendar with the anniversary.
>
> Expectation: Anniversaries from EBDB must also be highlighted when
> (diary-mark-entries) is invoked.

Oof, EBDB's diary integration was "write once and back away slowly"
code. I'm trying to understand diary-lib.el. So far as I can see, when
you add diary entries to `diary-entry-list' that list is consulted when
displaying entries for the day at point with "d" (as you note), but the
list is ignored by the code that marks days in the calendar, so you
don't see anything.

I hope I'm wrong about that, but someone would have to tell me how.
Another option would be to add something terrible to the
`diary-mark-entries-hook', which manually marked the dates somehow.

A third option would be switching up the approach altogether, and having
EBDB write its own diary file, which can then be included in the user's
master diary file. Perhaps that would be the best approach, rather than
trying to "plug in" to the diary code at a lower level.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
  2021-08-13 19:35 ` Eric Abrahamsen
@ 2021-08-14 15:20   ` Michael Heerdegen
  2021-08-14 18:08     ` Eric Abrahamsen
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Heerdegen @ 2021-08-14 15:20 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 49995, Pankaj Jangid

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Oof, EBDB's diary integration was "write once and back away slowly"
> code. I'm trying to understand diary-lib.el. So far as I can see, when
> you add diary entries to `diary-entry-list' [...]

Suggestion: provide a new diary-sexp function, similar to
`diary-lunar-phases'.  Then marking would be handled by the diary.

That function just has to return nil or a string (or a mark and a
string) depending on the dynamical variable DATE.  That's already the
whole diary related part.  People then have to add that function as sexp
entry to their diary if they want.


Michael.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
  2021-08-14 15:20   ` Michael Heerdegen
@ 2021-08-14 18:08     ` Eric Abrahamsen
  2021-08-15  4:01       ` Eric Abrahamsen
  0 siblings, 1 reply; 13+ messages in thread
From: Eric Abrahamsen @ 2021-08-14 18:08 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49995, Pankaj Jangid

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Oof, EBDB's diary integration was "write once and back away slowly"
>> code. I'm trying to understand diary-lib.el. So far as I can see, when
>> you add diary entries to `diary-entry-list' [...]
>
> Suggestion: provide a new diary-sexp function, similar to
> `diary-lunar-phases'.  Then marking would be handled by the diary.
>
> That function just has to return nil or a string (or a mark and a
> string) depending on the dynamical variable DATE.  That's already the
> whole diary related part.  People then have to add that function as sexp
> entry to their diary if they want.

Oh, huh: sort of inverting the prior approach. I find all this a little
confusing, I've never spent any time with the diary, and its integration
with Org always seemed very mysterious to me. But I do think the
calendar integration is very useful.

So the idea is that there would be a `ebdb-diary-anniversaries'
function, that looks sort of like:

(defun ebdb-diary-anniversaries (&optional mark)
  (with-suppressed-warnings ((lexical date))
    (defvar date))
  (when-let ((anniv-today (ebdb-get-anniversaries-for-date date)))
    (cons mark (mapconcat #'identity anniv-today "\n"))))

Something like that, anyway, and then users put

%%(ebdb-diary-anniversaries)

In their diary file? And that would get called once per visible date, so
potentially a whole lot, so `ebdb-get-anniversaries-for-date' should be
quick...

Anyway, thanks for this alternate suggestion! This wouldn't have
occurred to me.

Eric





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
  2021-08-14 18:08     ` Eric Abrahamsen
@ 2021-08-15  4:01       ` Eric Abrahamsen
  2021-08-15 13:18         ` Michael Heerdegen
  0 siblings, 1 reply; 13+ messages in thread
From: Eric Abrahamsen @ 2021-08-15  4:01 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49995, Pankaj Jangid

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

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>>> Oof, EBDB's diary integration was "write once and back away slowly"
>>> code. I'm trying to understand diary-lib.el. So far as I can see, when
>>> you add diary entries to `diary-entry-list' [...]
>>
>> Suggestion: provide a new diary-sexp function, similar to
>> `diary-lunar-phases'.  Then marking would be handled by the diary.
>>
>> That function just has to return nil or a string (or a mark and a
>> string) depending on the dynamical variable DATE.  That's already the
>> whole diary related part.  People then have to add that function as sexp
>> entry to their diary if they want.
>
> Oh, huh: sort of inverting the prior approach. I find all this a little
> confusing, I've never spent any time with the diary, and its integration
> with Org always seemed very mysterious to me. But I do think the
> calendar integration is very useful.

Okay, here's a version of how it might work. I've learned a little bit
more about the diary (and as a result will likely use it more! I'd
always thought it was just a poor cousin to Org, but I see it has its
own strengths), and have a solution that is a bit funky, but might be
okay. If Michael or anyone with a better understanding of diary than me
would comment on this, I'd very much appreciate it.

So the user puts "%%(ebdb-diary-anniversaries)" as a sexp in their
diary. That function gets called in two situations: marking dates in the
calendar (it gets called *many* times but only has to return a boolean),
and listing diary entries for a particular date (it only gets called
once, but has to produce more detailed information). The function checks
(bound-and-true-p 'original-date) to know which situation it's in.

At load time EBDB builds some prep data in a hash table, and that data
feels messy, but the strategy is to do a medium amount of work at load
time, the absolute minimum amount of work when marking dates in the
*Calendar* (essentially checks "does this hash key have a value"), and
again a medium amount of work when displaying diary entries for a
particular day.

I would love some feedback on this!

Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rework-ebdb-diary-anniversaries.diff --]
[-- Type: text/x-patch, Size: 5796 bytes --]

diff --git a/ebdb.el b/ebdb.el
index d3b7d9480a..b5075dfb86 100644
--- a/ebdb.el
+++ b/ebdb.el
@@ -282,6 +282,10 @@ do not set this to nil."
   "Customizations for EBDB utilities."
   :group 'ebdb)
 
+(defgroup ebdb-utilities-anniv nil
+  "Customizations for EBDB anniversaries."
+  :group 'ebdb)
+
 (defgroup ebdb-utilities-dialing nil
   "EBDB customizations for phone number dialing."
   :group 'ebdb-utilities)
@@ -373,6 +377,10 @@ Emacs, always query before reverting."
   :group 'ebdb-utilities-anniv
   :type 'boolean)
 
+(make-obsolete-variable
+ 'ebdb-use-diary
+ "Add %%(ebdb-diary-anniversaries) to your diary file instead" "0.8")
+
 (defcustom ebdb-anniversary-md-format "%B %d"
   "Format string used for displaying month-day anniversary dates.
 See the docstring of `format-time-string' for the meaning of
@@ -389,26 +397,12 @@ month, and day values are available."
   :group 'ebdb-utilities-anniv
   :type 'string)
 
-(defvar ebdb-diary-entries nil
-  "A list of all anniversary diary entries.
-Entries are added and removed in the `ebdb-init-field' and
-`ebdb-delete-field' methods of the `ebdb-field-anniversary'
-class, and added with the `ebdb-diary-add-entries' function.
-
-Each entry is a two-element list: a string representation of the
-anniversary date, and the sexp (as a string):
-
-\(diary-anniversary MM DD YYYY) (the year is optional)")
-
-;; Dynamic var needed by `diary-sexp-entry'.
-(defvar original-date)
-
-(defun ebdb-diary-add-entries ()
-  "Add anniversaries from EBDB to the diary."
-  (pcase-dolist (`(,entry ,sexp) ebdb-diary-entries)
-    (let ((parsed (cdr-safe (diary-sexp-entry sexp entry original-date))))
-      (when parsed
-	(diary-add-to-list original-date parsed sexp)))))
+(defvar ebdb-diary-entries (make-hash-table :test #'equal)
+  "Hash table holding anniversary entries for the diary.
+Keys are dates in the format (MONTH DAY YEAR), values are lists
+of anniversary strings.  Instances of `ebdb-field-anniversary'
+fields can push descriptive strings into the hash entries for
+their dates.  Also see `ebdb-diary-anniversaries'.")
 
 (defcustom ebdb-before-load-hook nil
   "Hook run before loading databases."
@@ -2199,12 +2193,31 @@ Eventually this method will go away."
 				    (list month day year))
 			 obj)))
 
+(defun ebdb-diary-anniversaries (&optional mark)
+  (with-no-warnings
+    (defvar date)
+    (defvar original-date))
+  (when-let ((entries (gethash (seq-subseq date 0 2) ebdb-diary-entries)))
+    (cons mark
+	  (mapconcat (pcase-lambda (`(,entry ,sexp))
+		       (if (bound-and-true-p original-date)
+			   ;; If we have `original-date', we're
+			   ;; displaying the diary list, so we need
+			   ;; the detailed string.
+			   (cdr (diary-sexp-entry
+				 sexp entry original-date))
+			 ;; If not, we're just marking dates on the
+			 ;; calendar, so any non-nil response value is
+			 ;; fine.
+			 entry))
+		     entries "; "))))
+
 ;; `ebdb-field-anniv-diary-entry' is defined below.
 (cl-defmethod ebdb-init-field ((anniv ebdb-field-anniversary) record)
-  (when ebdb-use-diary
-    (add-to-list
-     'ebdb-diary-entries
-     (ebdb-field-anniv-diary-entry anniv record))))
+  (let ((diary-entry (ebdb-field-anniv-diary-entry anniv record))
+	(date (seq-subseq (slot-value anniv 'date)
+			  0 2)))
+    (push diary-entry (gethash date ebdb-diary-entries))))
 
 (cl-defmethod ebdb-string ((ann ebdb-field-anniversary))
   (let* ((date (slot-value ann 'date))
@@ -2226,11 +2239,17 @@ Eventually this method will go away."
 
 (cl-defmethod ebdb-delete-field ((anniv ebdb-field-anniversary)
 				 record &optional _unload)
-  (when ebdb-use-diary
-    (setq
-     ebdb-diary-entries
-     (delete (ebdb-field-anniv-diary-entry anniv record)
-	     ebdb-diary-entries))))
+  (let ((entry-car (car (ebdb-field-anniv-diary-entry anniv record)))
+	(date (seq-subseq (slot-value anniv 'date)
+			  0 2)))
+    (puthash date
+	     (seq-remove (lambda (e)
+			   ;; Use the car of the entry (the text with
+			   ;; the record's name in it) as a key for
+			   ;; removing the whole entry.
+			   (equal entry-car (car e)))
+			 (gethash date ebdb-diary-entries))
+	     ebdb-diary-entries)))
 
 ;;; Id field
 
@@ -3219,18 +3238,17 @@ If FIELD doesn't specify a year, use the current year."
 
 (cl-defmethod ebdb-field-anniv-diary-entry ((field ebdb-field-anniversary)
 					    (record ebdb-record))
-  "Add a diary entry for FIELD's date."
-  (let ((cal-date (slot-value field 'date)))
+  "Produce a diary entry for FIELD's date.
+The return value is added to `ebdb-diary-entries' in the init
+method for the field, and tailored for consumption by
+`ebdb-diary-anniversaries'."
+  (pcase-let ((`(,month ,day ,year) (slot-value field 'date)))
     (list (concat (format "%s's "
 			  (ebdb-string record))
-		  (if (nth 2 cal-date)
-		      "%d%s "
-		    "%s ")
+		  (if year "%d%s " "")
 		  (slot-value field 'label))
-	  (apply #'format (if (nth 2 cal-date)
-			      "(diary-anniversary %s %s %s)"
-			    "(diary-anniversary %s %s)")
-		 cal-date))))
+	  (format "(diary-anniversary %s %s%s)"
+		  month day (if year (format " %s" year) "")))))
 
 ;;; `ebdb-record' subclasses
 
@@ -4338,6 +4356,7 @@ process.")
 	ebdb-record-tracker nil)
   (clrhash ebdb-org-hashtable)
   (clrhash ebdb-hashtable)
+  (clrhash ebdb-diary-entries)
   (clrhash ebdb-relation-hashtable))
 
 ;; Changing which database a record belongs to.
@@ -5372,8 +5391,6 @@ All the important work is done by the `ebdb-db-load' method."
        (cons db-file-regexp 'lisp-data-mode)
        auto-mode-alist))
     (run-hooks 'ebdb-after-load-hook)
-    (when ebdb-use-diary
-      (add-hook 'diary-list-entries-hook #'ebdb-diary-add-entries))
     (add-hook 'kill-emacs-hook #'ebdb-save-on-emacs-exit)
     (length ebdb-record-tracker)))
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
  2021-08-15  4:01       ` Eric Abrahamsen
@ 2021-08-15 13:18         ` Michael Heerdegen
  2021-08-15 14:28           ` Eric Abrahamsen
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Heerdegen @ 2021-08-15 13:18 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 49995, Pankaj Jangid

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Okay, here's a version of how it might work. I've learned a little bit
> more about the diary (and as a result will likely use it more! I'd
> always thought it was just a poor cousin to Org, but I see it has its
> own strengths), and have a solution that is a bit funky, but might be
> okay.

Looks quite good.

I would try to get rid of `diary-anniversary'.  All it does is checking
the date and calling `format' - things that you already do.  You now
effectively get `eval' inside `eval' when calling `diary', you have an
extra layer.  I hope removing that will also get rid of the need to look
at `original-date'.

[BTW: The only nontrivial thing `diary-anniversary' does is handling of
birthdays on 2/28, you may want to have a look if you need to handle
that case specially.]


Regards,

Michael.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
  2021-08-15 13:18         ` Michael Heerdegen
@ 2021-08-15 14:28           ` Eric Abrahamsen
  2021-08-15 15:57             ` Eric Abrahamsen
  0 siblings, 1 reply; 13+ messages in thread
From: Eric Abrahamsen @ 2021-08-15 14:28 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49995, Pankaj Jangid


On 08/15/21 15:18 PM, Michael Heerdegen wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Okay, here's a version of how it might work. I've learned a little bit
>> more about the diary (and as a result will likely use it more! I'd
>> always thought it was just a poor cousin to Org, but I see it has its
>> own strengths), and have a solution that is a bit funky, but might be
>> okay.
>
> Looks quite good.

Thanks for checking!

> I would try to get rid of `diary-anniversary'.  All it does is checking
> the date and calling `format' - things that you already do.  You now
> effectively get `eval' inside `eval' when calling `diary', you have an
> extra layer.  I hope removing that will also get rid of the need to look
> at `original-date'.
>
> [BTW: The only nontrivial thing `diary-anniversary' does is handling of
> birthdays on 2/28, you may want to have a look if you need to handle
> that case specially.]

It seems to me that it isn't `diary-anniversary' that needs to be gotten
rid of, so much as `diary-sexp-entry' -- that's the function that's
basically just eval'ling a string. If I get rid of `diary-anniversary',
I'll basically just end up re-writing it.

At init time, instead of building up strings, I could just build up
closures holding the appropriate dynamic value for DATE and ENTRY, and
calling `diary-anniversary': essentially replace `diary-sexp-entry'.
That's at least one less layer.

I can get rid of the check for `original-date', but I'd still like to
know if we're in a calendar-marking situation vs a listing-the-diary
situation: how else would I do that?

Thanks again,
Eric





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
  2021-08-15 14:28           ` Eric Abrahamsen
@ 2021-08-15 15:57             ` Eric Abrahamsen
  2021-08-15 20:16               ` Eric Abrahamsen
  0 siblings, 1 reply; 13+ messages in thread
From: Eric Abrahamsen @ 2021-08-15 15:57 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49995, Pankaj Jangid

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


On 08/15/21 07:28 AM, Eric Abrahamsen wrote:
> On 08/15/21 15:18 PM, Michael Heerdegen wrote:
>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>
>>> Okay, here's a version of how it might work. I've learned a little bit
>>> more about the diary (and as a result will likely use it more! I'd
>>> always thought it was just a poor cousin to Org, but I see it has its
>>> own strengths), and have a solution that is a bit funky, but might be
>>> okay.
>>
>> Looks quite good.
>
> Thanks for checking!
>
>> I would try to get rid of `diary-anniversary'.  All it does is checking
>> the date and calling `format' - things that you already do.  You now
>> effectively get `eval' inside `eval' when calling `diary', you have an
>> extra layer.  I hope removing that will also get rid of the need to look
>> at `original-date'.
>>
>> [BTW: The only nontrivial thing `diary-anniversary' does is handling of
>> birthdays on 2/28, you may want to have a look if you need to handle
>> that case specially.]
>
> It seems to me that it isn't `diary-anniversary' that needs to be gotten
> rid of, so much as `diary-sexp-entry' -- that's the function that's
> basically just eval'ling a string. If I get rid of `diary-anniversary',
> I'll basically just end up re-writing it.
>
> At init time, instead of building up strings, I could just build up
> closures holding the appropriate dynamic value for DATE and ENTRY, and
> calling `diary-anniversary': essentially replace `diary-sexp-entry'.
> That's at least one less layer.

Just for fun, here's a version with closures. The need for
`calendar-dlet' (or something that does that job) is unfortunate, and
maybe sufficient argument for writing my own version of
`diary-anniversary'. But this was a fun experiment in understanding
lexical binding and closures.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ebdb-diary-closures.diff --]
[-- Type: text/x-patch, Size: 5900 bytes --]

diff --git a/ebdb.el b/ebdb.el
index d3b7d9480a..7ffe41e097 100644
--- a/ebdb.el
+++ b/ebdb.el
@@ -282,6 +282,10 @@ do not set this to nil."
   "Customizations for EBDB utilities."
   :group 'ebdb)
 
+(defgroup ebdb-utilities-anniv nil
+  "Customizations for EBDB anniversaries."
+  :group 'ebdb)
+
 (defgroup ebdb-utilities-dialing nil
   "EBDB customizations for phone number dialing."
   :group 'ebdb-utilities)
@@ -373,6 +377,10 @@ Emacs, always query before reverting."
   :group 'ebdb-utilities-anniv
   :type 'boolean)
 
+(make-obsolete-variable
+ 'ebdb-use-diary
+ "Add %%(ebdb-diary-anniversaries) to your diary file instead" "0.8")
+
 (defcustom ebdb-anniversary-md-format "%B %d"
   "Format string used for displaying month-day anniversary dates.
 See the docstring of `format-time-string' for the meaning of
@@ -389,26 +397,12 @@ month, and day values are available."
   :group 'ebdb-utilities-anniv
   :type 'string)
 
-(defvar ebdb-diary-entries nil
-  "A list of all anniversary diary entries.
-Entries are added and removed in the `ebdb-init-field' and
-`ebdb-delete-field' methods of the `ebdb-field-anniversary'
-class, and added with the `ebdb-diary-add-entries' function.
-
-Each entry is a two-element list: a string representation of the
-anniversary date, and the sexp (as a string):
-
-\(diary-anniversary MM DD YYYY) (the year is optional)")
-
-;; Dynamic var needed by `diary-sexp-entry'.
-(defvar original-date)
-
-(defun ebdb-diary-add-entries ()
-  "Add anniversaries from EBDB to the diary."
-  (pcase-dolist (`(,entry ,sexp) ebdb-diary-entries)
-    (let ((parsed (cdr-safe (diary-sexp-entry sexp entry original-date))))
-      (when parsed
-	(diary-add-to-list original-date parsed sexp)))))
+(defvar ebdb-diary-entries (make-hash-table :test #'equal)
+  "Hash table holding anniversary entries for the diary.
+Keys are dates in the format (MONTH DAY YEAR), values are lists
+of anniversary strings.  Instances of `ebdb-field-anniversary'
+fields can push descriptive strings into the hash entries for
+their dates.  Also see `ebdb-diary-anniversaries'.")
 
 (defcustom ebdb-before-load-hook nil
   "Hook run before loading databases."
@@ -2199,12 +2193,30 @@ Eventually this method will go away."
 				    (list month day year))
 			 obj)))
 
+(defun ebdb-diary-anniversaries (&optional mark)
+  (with-no-warnings
+    (defvar date)
+    (defvar original-date))
+  (when-let ((entries (gethash (seq-subseq date 0 2) ebdb-diary-entries)))
+    (cons mark
+	  (mapconcat (pcase-lambda (`(,entry ,form))
+		       (if (bound-and-true-p original-date)
+			   ;; If we have `original-date', we're
+			   ;; displaying the diary list, so we need
+			   ;; the detailed string.
+			   (funcall form)
+			 ;; If not, we're just marking dates on the
+			 ;; calendar, so any non-nil response value is
+			 ;; fine.
+			 entry))
+		     entries "; "))))
+
 ;; `ebdb-field-anniv-diary-entry' is defined below.
 (cl-defmethod ebdb-init-field ((anniv ebdb-field-anniversary) record)
-  (when ebdb-use-diary
-    (add-to-list
-     'ebdb-diary-entries
-     (ebdb-field-anniv-diary-entry anniv record))))
+  (let ((diary-entry (ebdb-field-anniv-diary-entry anniv record))
+	(date (seq-subseq (slot-value anniv 'date)
+			  0 2)))
+    (push diary-entry (gethash date ebdb-diary-entries))))
 
 (cl-defmethod ebdb-string ((ann ebdb-field-anniversary))
   (let* ((date (slot-value ann 'date))
@@ -2226,11 +2238,17 @@ Eventually this method will go away."
 
 (cl-defmethod ebdb-delete-field ((anniv ebdb-field-anniversary)
 				 record &optional _unload)
-  (when ebdb-use-diary
-    (setq
-     ebdb-diary-entries
-     (delete (ebdb-field-anniv-diary-entry anniv record)
-	     ebdb-diary-entries))))
+  (let ((entry-car (car (ebdb-field-anniv-diary-entry anniv record)))
+	(date (seq-subseq (slot-value anniv 'date)
+			  0 2)))
+    (puthash date
+	     (seq-remove (lambda (e)
+			   ;; Use the car of the entry (the text with
+			   ;; the record's name in it) as a key for
+			   ;; removing the whole entry.
+			   (equal entry-car (car e)))
+			 (gethash date ebdb-diary-entries))
+	     ebdb-diary-entries)))
 
 ;;; Id field
 
@@ -3219,18 +3237,19 @@ If FIELD doesn't specify a year, use the current year."
 
 (cl-defmethod ebdb-field-anniv-diary-entry ((field ebdb-field-anniversary)
 					    (record ebdb-record))
-  "Add a diary entry for FIELD's date."
-  (let ((cal-date (slot-value field 'date)))
-    (list (concat (format "%s's "
-			  (ebdb-string record))
-		  (if (nth 2 cal-date)
-		      "%d%s "
-		    "%s ")
-		  (slot-value field 'label))
-	  (apply #'format (if (nth 2 cal-date)
-			      "(diary-anniversary %s %s %s)"
-			    "(diary-anniversary %s %s)")
-		 cal-date))))
+  "Produce a diary entry for FIELD's date.
+The return value is added to `ebdb-diary-entries' in the init
+method for the field, and tailored for consumption by
+`ebdb-diary-anniversaries'."
+  (pcase-let* ((`(,month ,day ,year) (slot-value field 'date))
+	       (entry (concat (format "%s's "
+				      (ebdb-string record))
+			      (if year "%d%s " "")
+			      (slot-value field 'label))))
+    (list entry
+	  (lambda ()
+	    (calendar-dlet ((entry entry))
+	     (cdr (diary-anniversary month day year)))))))
 
 ;;; `ebdb-record' subclasses
 
@@ -4338,6 +4357,7 @@ process.")
 	ebdb-record-tracker nil)
   (clrhash ebdb-org-hashtable)
   (clrhash ebdb-hashtable)
+  (clrhash ebdb-diary-entries)
   (clrhash ebdb-relation-hashtable))
 
 ;; Changing which database a record belongs to.
@@ -5372,8 +5392,6 @@ All the important work is done by the `ebdb-db-load' method."
        (cons db-file-regexp 'lisp-data-mode)
        auto-mode-alist))
     (run-hooks 'ebdb-after-load-hook)
-    (when ebdb-use-diary
-      (add-hook 'diary-list-entries-hook #'ebdb-diary-add-entries))
     (add-hook 'kill-emacs-hook #'ebdb-save-on-emacs-exit)
     (length ebdb-record-tracker)))
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
  2021-08-15 15:57             ` Eric Abrahamsen
@ 2021-08-15 20:16               ` Eric Abrahamsen
  2021-08-17 17:16                 ` Michael Heerdegen
  0 siblings, 1 reply; 13+ messages in thread
From: Eric Abrahamsen @ 2021-08-15 20:16 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49995, Pankaj Jangid

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


On 08/15/21 08:57 AM, Eric Abrahamsen wrote:
> On 08/15/21 07:28 AM, Eric Abrahamsen wrote:
>> On 08/15/21 15:18 PM, Michael Heerdegen wrote:
>>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>>>
>>>> Okay, here's a version of how it might work. I've learned a little bit
>>>> more about the diary (and as a result will likely use it more! I'd
>>>> always thought it was just a poor cousin to Org, but I see it has its
>>>> own strengths), and have a solution that is a bit funky, but might be
>>>> okay.
>>>
>>> Looks quite good.
>>
>> Thanks for checking!
>>
>>> I would try to get rid of `diary-anniversary'.  All it does is checking
>>> the date and calling `format' - things that you already do.  You now
>>> effectively get `eval' inside `eval' when calling `diary', you have an
>>> extra layer.  I hope removing that will also get rid of the need to look
>>> at `original-date'.
>>>
>>> [BTW: The only nontrivial thing `diary-anniversary' does is handling of
>>> birthdays on 2/28, you may want to have a look if you need to handle
>>> that case specially.]
>>
>> It seems to me that it isn't `diary-anniversary' that needs to be gotten
>> rid of, so much as `diary-sexp-entry' -- that's the function that's
>> basically just eval'ling a string. If I get rid of `diary-anniversary',
>> I'll basically just end up re-writing it.
>>
>> At init time, instead of building up strings, I could just build up
>> closures holding the appropriate dynamic value for DATE and ENTRY, and
>> calling `diary-anniversary': essentially replace `diary-sexp-entry'.
>> That's at least one less layer.
>
> Just for fun, here's a version with closures. The need for
> `calendar-dlet' (or something that does that job) is unfortunate, and
> maybe sufficient argument for writing my own version of
> `diary-anniversary'. But this was a fun experiment in understanding
> lexical binding and closures.

And, because I apparently have nothing else to do on a weekend, here's a
version that just calls a function directly, nothing fancy. Some overlap
with `diary-anniversary', but nothing terrible. This is probably the
best approach.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ebdb-diary-function.diff --]
[-- Type: text/x-patch, Size: 6303 bytes --]

diff --git a/ebdb.el b/ebdb.el
index d3b7d9480a..3264c77f1e 100644
--- a/ebdb.el
+++ b/ebdb.el
@@ -62,8 +62,7 @@
 (autoload 'widget-group-match "wid-edit")
 (autoload 'ebdb-migrate-from-bbdb "ebdb-migrate")
 (autoload 'eieio-customize-object "eieio-custom")
-(autoload 'diary-sexp-entry "diary-lib")
-(autoload 'diary-add-to-list "diary-lib")
+(autoload 'diary-ordinal-suffix "diary-lib")
 (autoload 'org-agenda-list "org-agenda")
 (autoload 'org-make-tags-matcher "org")
 (defvar ebdb-i18n-countries)
@@ -282,6 +281,10 @@ do not set this to nil."
   "Customizations for EBDB utilities."
   :group 'ebdb)
 
+(defgroup ebdb-utilities-anniv nil
+  "Customizations for EBDB anniversaries."
+  :group 'ebdb)
+
 (defgroup ebdb-utilities-dialing nil
   "EBDB customizations for phone number dialing."
   :group 'ebdb-utilities)
@@ -373,6 +376,10 @@ Emacs, always query before reverting."
   :group 'ebdb-utilities-anniv
   :type 'boolean)
 
+(make-obsolete-variable
+ 'ebdb-use-diary
+ "Add %%(ebdb-diary-anniversaries) to your diary file instead" "0.8")
+
 (defcustom ebdb-anniversary-md-format "%B %d"
   "Format string used for displaying month-day anniversary dates.
 See the docstring of `format-time-string' for the meaning of
@@ -389,26 +396,12 @@ month, and day values are available."
   :group 'ebdb-utilities-anniv
   :type 'string)
 
-(defvar ebdb-diary-entries nil
-  "A list of all anniversary diary entries.
-Entries are added and removed in the `ebdb-init-field' and
-`ebdb-delete-field' methods of the `ebdb-field-anniversary'
-class, and added with the `ebdb-diary-add-entries' function.
-
-Each entry is a two-element list: a string representation of the
-anniversary date, and the sexp (as a string):
-
-\(diary-anniversary MM DD YYYY) (the year is optional)")
-
-;; Dynamic var needed by `diary-sexp-entry'.
-(defvar original-date)
-
-(defun ebdb-diary-add-entries ()
-  "Add anniversaries from EBDB to the diary."
-  (pcase-dolist (`(,entry ,sexp) ebdb-diary-entries)
-    (let ((parsed (cdr-safe (diary-sexp-entry sexp entry original-date))))
-      (when parsed
-	(diary-add-to-list original-date parsed sexp)))))
+(defvar ebdb-diary-entries (make-hash-table :test #'equal)
+  "Hash table holding anniversary entries for the diary.
+Keys are dates in the format (MONTH DAY YEAR), values are lists
+of anniversary strings.  Instances of `ebdb-field-anniversary'
+fields can push descriptive strings into the hash entries for
+their dates.  Also see `ebdb-diary-anniversaries'.")
 
 (defcustom ebdb-before-load-hook nil
   "Hook run before loading databases."
@@ -2199,12 +2192,29 @@ Eventually this method will go away."
 				    (list month day year))
 			 obj)))
 
-;; `ebdb-field-anniv-diary-entry' is defined below.
+(defun ebdb-diary-anniversaries (&optional mark)
+  (with-no-warnings
+    (defvar date)
+    (defvar original-date))
+  (when-let ((entries (gethash (seq-subseq date 0 2) ebdb-diary-entries)))
+    (cons mark
+	  (mapconcat (pcase-lambda (`(,field ,record))
+		       (if (bound-and-true-p original-date)
+			   ;; If we have `original-date', we're
+			   ;; displaying the diary list, so we need
+			   ;; the detailed string.
+			   (ebdb-field-anniv-diary-entry
+			    field record (nth 2 date))
+			 ;; If not, we're just marking dates on the
+			 ;; calendar, so any non-nil response value is
+			 ;; fine.
+			 entry))
+		     entries "; "))))
+
 (cl-defmethod ebdb-init-field ((anniv ebdb-field-anniversary) record)
-  (when ebdb-use-diary
-    (add-to-list
-     'ebdb-diary-entries
-     (ebdb-field-anniv-diary-entry anniv record))))
+  (with-slots (date) anniv
+    (push (list anniv record)
+	  (gethash (seq-subseq date 0 2) ebdb-diary-entries))))
 
 (cl-defmethod ebdb-string ((ann ebdb-field-anniversary))
   (let* ((date (slot-value ann 'date))
@@ -2226,11 +2236,12 @@ Eventually this method will go away."
 
 (cl-defmethod ebdb-delete-field ((anniv ebdb-field-anniversary)
 				 record &optional _unload)
-  (when ebdb-use-diary
-    (setq
-     ebdb-diary-entries
-     (delete (ebdb-field-anniv-diary-entry anniv record)
-	     ebdb-diary-entries))))
+  (with-slots (date) anniv
+    (puthash (seq-subseq date 0 2)
+	     (seq-remove (lambda (e)
+			   (equal e (list anniv record)))
+			 (gethash (seq-subseq date 0 2) ebdb-diary-entries))
+	     ebdb-diary-entries)))
 
 ;;; Id field
 
@@ -3218,19 +3229,22 @@ If FIELD doesn't specify a year, use the current year."
      (format "%d-%d-%d" year (nth 0 date) (nth 1 date)))))
 
 (cl-defmethod ebdb-field-anniv-diary-entry ((field ebdb-field-anniversary)
-					    (record ebdb-record))
-  "Add a diary entry for FIELD's date."
-  (let ((cal-date (slot-value field 'date)))
-    (list (concat (format "%s's "
-			  (ebdb-string record))
-		  (if (nth 2 cal-date)
-		      "%d%s "
-		    "%s ")
-		  (slot-value field 'label))
-	  (apply #'format (if (nth 2 cal-date)
-			      "(diary-anniversary %s %s %s)"
-			    "(diary-anniversary %s %s)")
-		 cal-date))))
+					    (record ebdb-record)
+					    &optional now-year)
+  "Produce a diary entry for FIELD's date.
+The entry is a string noting how many years have passed for
+RECORD's FIELD anniversary, relative to NOW-YEAR."
+  ;; Essentially a re-write of `diary-anniversary'.
+  (pcase-let* ((`(,month ,day ,year) (slot-value field 'date))
+	       (label (slot-value field 'label))
+	       (num-years (when (and year now-year)
+			    (- now-year year))))
+    (concat (format "%s's " (ebdb-string record))
+	    (when year
+	      (format "%d%s " num-years (diary-ordinal-suffix num-years)))
+	    label
+	    (unless (string= label "birthday")
+	      " anniversary"))))
 
 ;;; `ebdb-record' subclasses
 
@@ -4338,6 +4352,7 @@ process.")
 	ebdb-record-tracker nil)
   (clrhash ebdb-org-hashtable)
   (clrhash ebdb-hashtable)
+  (clrhash ebdb-diary-entries)
   (clrhash ebdb-relation-hashtable))
 
 ;; Changing which database a record belongs to.
@@ -5372,8 +5387,6 @@ All the important work is done by the `ebdb-db-load' method."
        (cons db-file-regexp 'lisp-data-mode)
        auto-mode-alist))
     (run-hooks 'ebdb-after-load-hook)
-    (when ebdb-use-diary
-      (add-hook 'diary-list-entries-hook #'ebdb-diary-add-entries))
     (add-hook 'kill-emacs-hook #'ebdb-save-on-emacs-exit)
     (length ebdb-record-tracker)))
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
  2021-08-15 20:16               ` Eric Abrahamsen
@ 2021-08-17 17:16                 ` Michael Heerdegen
  2021-08-17 19:45                   ` Eric Abrahamsen
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Heerdegen @ 2021-08-17 17:16 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 49995, Pankaj Jangid

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> And, because I apparently have nothing else to do on a weekend, here's a
> version that just calls a function directly, nothing fancy. Some overlap
> with `diary-anniversary', but nothing terrible. This is probably the
> best approach.

Yes, looks like more or less what I had suggested.  Hope it turns out it
was the right advice.

> -;; `ebdb-field-anniv-diary-entry' is defined below.
> +(defun ebdb-diary-anniversaries (&optional mark)
> +  (with-no-warnings
> +    (defvar date)
> +    (defvar original-date))
> +  (when-let ((entries (gethash (seq-subseq date 0 2) ebdb-diary-entries)))
> +    (cons mark
> +	  (mapconcat (pcase-lambda (`(,field ,record))
> +		       (if (bound-and-true-p original-date)
> +			   ;; If we have `original-date', we're
> +			   ;; displaying the diary list, so we need
> +			   ;; the detailed string.
> +			   (ebdb-field-anniv-diary-entry
> +			    field record (nth 2 date))
> +			 ;; If not, we're just marking dates on the
> +			 ;; calendar, so any non-nil response value is
> +			 ;; fine.
> +			 entry))
> +		     entries "; "))))

Do you really expect that this if-clause has a measurable effect on
performance?  Most people will not have thousands of anniversaries in
their database (and even then, they probably don't want to have them all
listed).  OTOH you now need to rely on an internal aspect of the
implementation.

Some more thoughts about this matter:

Do have a version for the Org agenda?  I see BBDB has
`org-bbdb-anniversaries'.  It handles the 2/29 problem btw.

What I as a user would wish (for the Diary and Org) would be a way to
control on a per-field basis (1) which anniversaries are listed, (2) how
they are presented and (3) a way to allow reminders for some (I might
need some weeks time to buy a present for some people, while I only want
to congratulate others so I don't need a reminder for most).

Regards,

Michael.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
  2021-08-17 17:16                 ` Michael Heerdegen
@ 2021-08-17 19:45                   ` Eric Abrahamsen
  2021-08-18 15:57                     ` Michael Heerdegen
  0 siblings, 1 reply; 13+ messages in thread
From: Eric Abrahamsen @ 2021-08-17 19:45 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49995, Pankaj Jangid


On 08/17/21 19:16 PM, Michael Heerdegen wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> And, because I apparently have nothing else to do on a weekend, here's a
>> version that just calls a function directly, nothing fancy. Some overlap
>> with `diary-anniversary', but nothing terrible. This is probably the
>> best approach.
>
> Yes, looks like more or less what I had suggested.  Hope it turns out it
> was the right advice.

I'm sure it will, I just had to waste a few hours trying out alternate
approaches first :)

>> -;; `ebdb-field-anniv-diary-entry' is defined below.
>> +(defun ebdb-diary-anniversaries (&optional mark)
>> +  (with-no-warnings
>> +    (defvar date)
>> +    (defvar original-date))
>> +  (when-let ((entries (gethash (seq-subseq date 0 2) ebdb-diary-entries)))
>> +    (cons mark
>> +	  (mapconcat (pcase-lambda (`(,field ,record))
>> +		       (if (bound-and-true-p original-date)
>> +			   ;; If we have `original-date', we're
>> +			   ;; displaying the diary list, so we need
>> +			   ;; the detailed string.
>> +			   (ebdb-field-anniv-diary-entry
>> +			    field record (nth 2 date))
>> +			 ;; If not, we're just marking dates on the
>> +			 ;; calendar, so any non-nil response value is
>> +			 ;; fine.
>> +			 entry))
>> +		     entries "; "))))
>
> Do you really expect that this if-clause has a measurable effect on
> performance?  Most people will not have thousands of anniversaries in
> their database (and even then, they probably don't want to have them all
> listed).  OTOH you now need to rely on an internal aspect of the
> implementation.

Dunno, I just figured that, when marking, the function gets called ~90
times in a row, potentially over and over again as the user scrolls the
Calendar. Other diary functions come with warnings about potential
slowdowns during marking, and the user might have a large number of
these various functions, so I thought I'd just try to make this as
polite as possible. Probably I'm over-thinking it, but on the other
hand, the code's already written...

> Some more thoughts about this matter:
>
> Do have a version for the Org agenda?  I see BBDB has
> `org-bbdb-anniversaries'.  It handles the 2/29 problem btw.

Oops, I've already handled the 2/29 problem, it just didn't make it into
the diff because I hadn't committed that bit yet.

There's nothing explicit for the agenda, I guess
`ebdb-diary-anniversaries' behaves pretty much the same as
`org-bbdb-anniversaries': you can either stick it in your diary file, if
you use the diary, or in an Org file, if you don't.

> What I as a user would wish (for the Diary and Org) would be a way to
> control on a per-field basis (1) which anniversaries are listed, (2) how
> they are presented and (3) a way to allow reminders for some (I might
> need some weeks time to buy a present for some people, while I only want
> to congratulate others so I don't need a reminder for most).

Okay, thanks for these suggestions! There are many, many aspects of EBDB
where I know I could be doing more, and it's great to have some explicit
requests.

I'm thinking about how best to separate concerns. The user might want
notifications about EBDB contacts, and might be using the diary, or Org,
or maybe neither of those. EBDB fields should store basic information
like: notify or don't notify, an optional custom notification string,
and an optional number of days in advance to notify. If a field has a
number of advance days, it puts itself in the hash table twice: on its
own date, and on its advance date.

Then there's a `ebdb-use-notifications' option: if non-nil, EBDB
displays messages itself, at load time and also record display time.

Otherwise, `ebdb-diary-anniversaries' works the way it does now, for
only the diary, or in Org.

Maybe, additionally, we provide a `ebdb-export-to-org' command that
writes an Org file holding all our anniversaries as headings, with
advance notifications implemented as DEADLINE lines with a warning
period. I could use custom properties to identify headlines, so running
the command multiple times would only add headings that aren't there
already, allowing the user to edit the headlines or add more stuff after
export.

How does all of that sound?

In the meantime, I'll get this code in and a new EBDB version released.
I'm sure Pankaj has been thrilled to get dragged through this long
thread, but that's no reason to delay :)

Eric





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
  2021-08-17 19:45                   ` Eric Abrahamsen
@ 2021-08-18 15:57                     ` Michael Heerdegen
  2021-08-18 17:13                       ` Eric Abrahamsen
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Heerdegen @ 2021-08-18 15:57 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: 49995, Pankaj Jangid

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Otherwise, `ebdb-diary-anniversaries' works the way it does now, for
> only the diary, or in Org.

Ok, great.

>
> Maybe, additionally, we provide a `ebdb-export-to-org' command that
> writes an Org file holding all our anniversaries as headings, with
> advance notifications implemented as DEADLINE lines with a warning
> period. [...]

While the idea has some charm, you would loose the speed advantages from
using a hash table for lookup, right?  Or would Org build and use one
internally?

Anyway, I think personally I would not want to have a second place for
anniversaries - unless it would then be beneficial to use only that file
in the future.

> How does all of that sound?

All of that sounded good, thanks for your time.

Michael.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar
  2021-08-18 15:57                     ` Michael Heerdegen
@ 2021-08-18 17:13                       ` Eric Abrahamsen
  0 siblings, 0 replies; 13+ messages in thread
From: Eric Abrahamsen @ 2021-08-18 17:13 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: 49995-done, Pankaj Jangid

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Otherwise, `ebdb-diary-anniversaries' works the way it does now, for
>> only the diary, or in Org.
>
> Ok, great.
>
>>
>> Maybe, additionally, we provide a `ebdb-export-to-org' command that
>> writes an Org file holding all our anniversaries as headings, with
>> advance notifications implemented as DEADLINE lines with a warning
>> period. [...]
>
> While the idea has some charm, you would loose the speed advantages from
> using a hash table for lookup, right?  Or would Org build and use one
> internally?
>
> Anyway, I think personally I would not want to have a second place for
> anniversaries - unless it would then be beneficial to use only that file
> in the future.

This would just be an alternate way of getting EBDB anniversaries into
your Org agenda. Instead of sticking %%(ebdb-diary-anniversaries)
somewhere in your Org files, EBDB would export a separate Org file
holding its anniversaries as headings, which you would include in the
Agenda. It would only make sense if the user wanted to make lots of
edits to the anniversary entries.

I'm probably just overthinking things!

I've just pushed the changes so far, and released a new version, and now
will work on further customization. I want to implement this for the ID
field as well: I meant to do that quite a while ago, and my own passport
expired because I hadn't yet gotten around to writing the code... :(





^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-08-18 17:13 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-11  7:13 bug#49995: 28.0.50; EBDB Anniversaries do not appear marked in calendar Pankaj Jangid
2021-08-13 19:35 ` Eric Abrahamsen
2021-08-14 15:20   ` Michael Heerdegen
2021-08-14 18:08     ` Eric Abrahamsen
2021-08-15  4:01       ` Eric Abrahamsen
2021-08-15 13:18         ` Michael Heerdegen
2021-08-15 14:28           ` Eric Abrahamsen
2021-08-15 15:57             ` Eric Abrahamsen
2021-08-15 20:16               ` Eric Abrahamsen
2021-08-17 17:16                 ` Michael Heerdegen
2021-08-17 19:45                   ` Eric Abrahamsen
2021-08-18 15:57                     ` Michael Heerdegen
2021-08-18 17:13                       ` Eric Abrahamsen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.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).