[PATCH] org-contacts: Update contacts cache if it contains markers with no buffer

[PATCH] org-contacts: Update contacts cache if it contains markers with no buffer

[Øyvind Stegard]

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

Hi list,

In contrib/lisp/org-contacts.el:

The function `org-contacts-db-need-update-p' does two checks to
determine if the contacts cache should be updated:
1. If the variable `org-contacts-last-update' is nil.
2. If modification time of any file containing contacts is more recent than
   timestamp recorded in `org-contacts-last-update'.

There is another case where an update is required: when marker objects
contained in the contact cache `org-contacts-db' suddenly point to no
buffer. If a buffer containing contacts is killed, but underlying file
is not modified, org-contacts will not detect this, and cached markers
that pointed to the now killed buffer will become dead (have no buffer
associated with them). This seems to cause problems at least in
`org-contacts-anniversaries', which if used as diary sexp in an agenda
file, will cause "Bad sexp" errors.

I have not done any thorough analysis of `org-contacts-anniversaries'
itself, but it seems to work OK whenever the markers in
`org-contacts-db' are OK. And it looks like the markers are used in that
code.

To reproduce:
1. Load up org-contacts and do a query with "M-x org-contacts".
2. Kill at least one org-buffer containing a contact with BIRTHDAY
   property set (but do not save underlying file).
3. Make sure to use %%(org-contacts-anniversaries) in some agenda file.
4. Opening agenda should produce a "Bad sexp" error.


I've attached a patch (against git master) which I am currently using.
It will cause org-contacts to re-load if a marker is found in cache that
points to no buffer. This looked to me like the quickest approach for
fixing it without getting to know org-contacts.el better.


Regards,

Øyvind
-- 
< Øyvind Stegard
 < http://stegard.net/


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-contacts-Ensure-contacts-cache-updated-on-dead-markers.patch --]
[-- Type: text/x-diff, Size: 2521 bytes --]

From e8d5f9ab71f75e5f5087d47e2e86115584db4a03 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=98yvind=20Stegard?= <oyvind.stegard@ifi.uio.no>
Date: Fri, 12 Jul 2013 12:17:12 +0200
Subject: [PATCH] org-contacts: Ensure contacts cache is updated if it contains
 markers with no buffer

* contrib/lisp/org-contacts.el (org-contacts-db-need-update-p):
Check if org-contacts-db cache contains markers with no buffer as well, when
determining if cache should be updated from files.

The function `org-contacts-db-need-update-p' does two checks to determine if the
contacts cache should be updated:
1. If the variable `org-contacts-last-update' is nil.
2. If modification time of any file containing contacts is more recent than
   timestamp recorded in `org-contacts-last-update'.

There is another case where an update is required: when marker objects contained
in the contact cache `org-contacts-db' suddenly point to no buffer. If a buffer
containing contacts is killed, but underlying file is not modified, org-contacts
will not detect this, and cached markers that pointed to the now killed buffer
will become "dead" (e.g. have no buffer associated with them). This seems to cause
problems for instance in `org-contacts-anniversaries', which if used as diary
sexp in agenda file, will cause "Bad sexp" errors.
---
 contrib/lisp/org-contacts.el | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/contrib/lisp/org-contacts.el b/contrib/lisp/org-contacts.el
index 71f7bf4..a220993 100644
--- a/contrib/lisp/org-contacts.el
+++ b/contrib/lisp/org-contacts.el
@@ -224,7 +224,20 @@ A regexp matching strings of whitespace, `,' and `;'.")
       (org-find-if (lambda (file)
 		     (or (time-less-p org-contacts-last-update
 				      (elt (file-attributes file) 5))))
-		   (org-contacts-files))))
+		   (org-contacts-files))
+      (org-contacts-db-has-dead-markers-p org-contacts-db)))
+
+(defun org-contacts-db-has-dead-markers-p (org-contacts-db)
+  "Returns t if at least one dead marker is found in
+ORG-CONTACTS-DB. A dead marker in this case is a marker pointing
+to dead or no buffer."
+    ;; Scan contacts list looking for dead markers, and return t at first found.
+    (catch 'dead-marker-found
+      (while org-contacts-db
+        (unless (marker-buffer (nth 1 (car org-contacts-db)))
+          (throw 'dead-marker-found t))
+        (setq org-contacts-db (cdr org-contacts-db)))
+      nil))
 
 (defun org-contacts-db ()
   "Return the latest Org Contacts Database."
-- 
1.8.1.2

Re: [PATCH] org-contacts: Update contacts cache if it contains markers with no buffer

[Daimrod]

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

Øyvind Stegard <oyvinst@ifi.uio.no> writes:

> Hi list,

Hello,

> In contrib/lisp/org-contacts.el:
>
> The function `org-contacts-db-need-update-p' does two checks to
> determine if the contacts cache should be updated:
> 1. If the variable `org-contacts-last-update' is nil.
> 2. If modification time of any file containing contacts is more recent than
>    timestamp recorded in `org-contacts-last-update'.
>
> There is another case where an update is required: when marker objects
> contained in the contact cache `org-contacts-db' suddenly point to no
> buffer. If a buffer containing contacts is killed, but underlying file
> is not modified, org-contacts will not detect this, and cached markers
> that pointed to the now killed buffer will become dead (have no buffer
> associated with them). This seems to cause problems at least in
> `org-contacts-anniversaries', which if used as diary sexp in an agenda
> file, will cause "Bad sexp" errors.
>
> I have not done any thorough analysis of `org-contacts-anniversaries'
> itself, but it seems to work OK whenever the markers in
> `org-contacts-db' are OK. And it looks like the markers are used in that
> code.
>
> To reproduce:
> 1. Load up org-contacts and do a query with "M-x org-contacts".
> 2. Kill at least one org-buffer containing a contact with BIRTHDAY
>    property set (but do not save underlying file).
> 3. Make sure to use %%(org-contacts-anniversaries) in some agenda file.
> 4. Opening agenda should produce a "Bad sexp" error.
>
>
> I've attached a patch (against git master) which I am currently using.
> It will cause org-contacts to re-load if a marker is found in cache that
> points to no buffer. This looked to me like the quickest approach for
> fixing it without getting to know org-contacts.el better.
>

Thanks for the bug report!

Don't you think that checking if one of the buffer would be enough and
faster?
With something like (every #'get-file-buffer org-contacts-files)

>
> Regards,
>
> Øyvind

-- 
Daimrod/Greg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] org-contacts: Update contacts cache if it contains markers with no buffer

[Øyvind Stegard]

Daimrod <daimrod@gmail.com> writes:

[...]

> Thanks for the bug report!
>
> Don't you think that checking if one of the buffer would be enough and
> faster?
> With something like (every #'get-file-buffer org-contacts-files)

Hi,

I thought about that, yes. But what about when an org-contacts buffer is
killed and subsequently re-opened (like using `find-alternate-file' with
same file name). Then the buffer will exist for the file, but it will be
a new instance, and the markers in org-contacts-db will still be dead.
That's why I decided to just check the markers instead.


Regards,

Øyvind
-- 
< Øyvind Stegard
 < http://stegard.net/

Re: [PATCH] org-contacts: Update contacts cache if it contains markers with no buffer

[Daimrod]

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

Øyvind Stegard <oyvinst@ifi.uio.no> writes:

> Daimrod <daimrod@gmail.com> writes:
>
> [...]
>
>> Thanks for the bug report!
>>
>> Don't you think that checking if one of the buffer would be enough and
>> faster?
>> With something like (every #'get-file-buffer org-contacts-files)
>
> Hi,
>
> I thought about that, yes. But what about when an org-contacts buffer is
> killed and subsequently re-opened (like using `find-alternate-file' with
> same file name). Then the buffer will exist for the file, but it will be
> a new instance, and the markers in org-contacts-db will still be dead.
> That's why I decided to just check the markers instead.

You're right, good catch!

I've installed the patch and haven't felt any slowdown, but I've a small
contacts file. If people complain about slowdown we could add a buffer
local hook to ask before killing contacts files.

> Regards,
>
> Øyvind

-- 
Daimrod/Greg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] org-contacts: Update contacts cache if it contains markers with no buffer

[Øyvind Stegard]

Daimrod <daimrod@gmail.com> writes:

[...]

> You're right, good catch!
>
> I've installed the patch and haven't felt any slowdown, but I've a small
> contacts file. If people complain about slowdown we could add a buffer
> local hook to ask before killing contacts files.

If speed turns out to be an issue, then simply caching references to the
contact buffers and validating those references with `buffer-live-p'
should be quicker and just as reliable.

(Checking markers was just a quick solution on my part that did not
require changes to any data structures.)


Øyvind
-- 
< Øyvind Stegard
 < http://stegard.net/