Re: [PATCH] org-contacts.el: Catch 'nextfile in org-contacts-db

[phcrama.ebiz@gmx.com]

[-- Attachment #1: Type: text/html, Size: 547 bytes --]

[-- Attachment #2: 0001-Refactor-org-check-agenda-file-to-use-only-its-inter.patch --]
[-- Type: application/octet-stream, Size: 6309 bytes --]

From a8b92e49d0adfba0e7532be15d7d659bbb3159f5 Mon Sep 17 00:00:00 2001
From: "U-mathilde\\philippe" <phcrama.ebiz@gmx.com>
Date: Sat, 4 Jul 2015 23:01:56 +0200
Subject: [PATCH] Refactor org-check-agenda-file to use only its interactive
 part for org-contacts-files list

The org-contacts module uses org-check-agenda-file to filter the
`org-contacts-files' list.  However, when a file in that list was not
found, `org-check-agenda-file' would try to remove it from the
`org-agenda-files' list instead of the `org-contacts-files' list.

To keep one central place deciding what a valid agenda file
is (currently only file existence is checked, but who knows what other
checks might be added), I split off the UI part which will return NIL if
there's no problem, 'nextfile if the user decided to remove the file
from the list (note that the message doesn't say which list) and raises
a user-error if the user requests it.

I've then rewritten the `org-contacts-files' function to process the
`org-contacts-files' list using `org-check-agenda-file-1' and applying
the changes or (if `org-contacts-files' isn't define) to use
`org-check-agenda-file' on that list.
---
 contrib/lisp/org-contacts.el | 53 ++++++++++++++++++--------------------------
 lisp/org.el                  | 27 ++++++++++++++++++----
 2 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/contrib/lisp/org-contacts.el b/contrib/lisp/org-contacts.el
index edc09fe..fa0f7dc 100644
--- a/contrib/lisp/org-contacts.el
+++ b/contrib/lisp/org-contacts.el
@@ -227,7 +227,18 @@ A regexp matching strings of whitespace, `,' and `;'.")
 
 (defun org-contacts-files ()
   "Return list of Org files to use for contact management."
-  (or org-contacts-files (org-agenda-files t 'ifmode)))
+  (if org-contacts-files
+      (org-remove-if (lambda (file)
+                       (when (eq 'nextfile (org-check-agenda-file-1 file))
+                         (customize-save-variable
+			  'org-contacts-files
+			  (delete file org-contacts-files))))
+                     org-contacts-files)
+    (let (result)
+      (dolist (file (org-agenda-files t 'ifmode) (nreverse result))
+        (unless (catch 'nextfile
+                  (org-check-agenda-file file))
+          (push file result)))))))
 
 (defun org-contacts-db-need-update-p ()
   "Determine whether `org-contacts-db' needs to be refreshed."
@@ -261,37 +272,15 @@ to dead or no buffer."
 	     (make-progress-reporter "Updating Org Contacts Database..." 0 (length org-contacts-files)))
 	    (i 0))
 	(dolist (file (org-contacts-files))
-	  (if (catch 'nextfile
-                ;; if file doesn't exist and the user agrees to removing it
-                ;; from org-agendas-list, 'nextfile is thrown.  Catch it here
-                ;; and skip processing the file.
-                ;;
-                ;; TODO: suppose that the user has set an org-contacts-files
-                ;; list that contains an element that doesn't exist in the
-                ;; file system: in that case, the org-agenda-files list could
-                ;; be updated (and saved to the customizations of the user) if
-                ;; it contained the same file even though the org-agenda-files
-                ;; list wasn't actually used.  I don't think it is normal that
-                ;; org-contacts updates org-agenda-files in this case, but
-                ;; short of duplicating org-check-agenda-files and
-                ;; org-remove-files, I don't know how to avoid it.
-                ;;
-                ;; A side effect of the TODO is that the faulty
-                ;; org-contacts-files list never gets updated and thus the
-                ;; user is always queried about the missing files when
-                ;; org-contacts-db-need-update-p returns true.
-                (org-check-agenda-file file))
-              (message "Skipped %s removed from org-agenda-files list."
-                       (abbreviate-file-name file))
-	    (with-current-buffer (org-get-agenda-file-buffer file)
-	      (unless (eq major-mode 'org-mode)
-		(error "File %s is not in `org-mode'" file))
-	      (setf result
-		    (append result
-			    (org-scan-tags
-			     'org-contacts-at-point
-			     contacts-matcher
-			     todo-only)))))
+	  (with-current-buffer (org-get-agenda-file-buffer file)
+	    (unless (eq major-mode 'org-mode)
+	      (error "File %s is not in `org-mode'" file))
+	    (setf result
+		  (append result
+			  (org-scan-tags
+			   'org-contacts-at-point
+			   contacts-matcher
+			   todo-only))))
 	  (progress-reporter-update progress-reporter (setq i (1+ i))))
 	(setf org-contacts-db result
 	      org-contacts-last-update (current-time))
diff --git a/lisp/org.el b/lisp/org.el
index 085b763..26fc22e 100755
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -18559,18 +18559,37 @@ Optional argument FILE means use this file instead of the current."
 (defun org-file-menu-entry (file)
   (vector file (list 'find-file file) t))
 
-(defun org-check-agenda-file (file)
-  "Make sure FILE exists.  If not, ask user what to do."
+(defun org-check-agenda-file-1 (file)
+  "Make sure FILE exists.  If not, ask user what to do.
+
+This is an internal function.  See also `org-check-agenda-file'.  There are
+three possibilities:
+1. The file exists -> return NIL.
+2. The file doesn't exist and the user asks to remove it from the list
+   -> return 'next-file
+3. The file doesn't exist and the user asks to abort
+   -> raise error"
   (when (not (file-exists-p file))
     (message "Non-existent agenda file %s.  [R]emove from list or [A]bort?"
 	     (abbreviate-file-name file))
     (let ((r (downcase (read-char-exclusive))))
       (cond
        ((equal r ?r)
-	(org-remove-file file)
-	(throw 'nextfile t))
+	'nextfile)
        (t (user-error "Abort"))))))
 
+(defun org-check-agenda-file (file)
+  "Make sure FILE exists.  If not, ask user what to do."
+  (let ((problem (org-check-agenda-file-1 file)))
+    (cond
+     ((null problem) nil)
+     ((eql problem 'nextfile)
+      (org-remove-file file)
+      (throw 'nextfile t))
+     ;; NOTREAVHED
+     ;; (t (user-error "Abort"))
+     )))
+
 (defun org-get-agenda-file-buffer (file)
   "Get an agenda buffer visiting FILE.
 If the buffer needs to be created, add it to the list of buffers
-- 
2.3.5