From mboxrd@z Thu Jan 1 00:00:00 1970 From: phcrama.ebiz@gmx.com Subject: Re: [PATCH] org-contacts.el: Catch 'nextfile in org-contacts-db Date: Sat, 4 Jul 2015 23:12:32 +0200 Message-ID: References: <395DF5E7-3C6F-4CDD-8A88-8D1525BBE5AC@gmx.com>, <87ioa3opiu.fsf@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=rehcsedm-dbce625c-554c-4884-bb1d-46b9614bff7c Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:58964) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZBUjl-0000fL-Qq for emacs-orgmode@gnu.org; Sat, 04 Jul 2015 17:12:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZBUji-0000Ls-Li for emacs-orgmode@gnu.org; Sat, 04 Jul 2015 17:12:37 -0400 Received: from mout.gmx.net ([212.227.15.19]:58767) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZBUji-0000Lb-BL for emacs-orgmode@gnu.org; Sat, 04 Jul 2015 17:12:34 -0400 In-Reply-To: <87ioa3opiu.fsf@gmail.com> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Daimrod Cc: emacs-orgmode@gnu.org --rehcsedm-dbce625c-554c-4884-bb1d-46b9614bff7c Content-Type: text/html; charset=UTF-8
Hi Greg,
 
thank you for your remarks.
 
I agree with all of them but like the idea that there's one central point checking what a valid org agenda file is (even if for now only the file existence is checked).  So let me propose a different patch (attached).
 
Best regards,
--
Philippe Crama
--rehcsedm-dbce625c-554c-4884-bb1d-46b9614bff7c Content-Type: application/octet-stream Content-Disposition: attachment; filename=0001-Refactor-org-check-agenda-file-to-use-only-its-inter.patch >From a8b92e49d0adfba0e7532be15d7d659bbb3159f5 Mon Sep 17 00:00:00 2001 From: "U-mathilde\\philippe" 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 --rehcsedm-dbce625c-554c-4884-bb1d-46b9614bff7c--