org-unmodified asks to revert buffer if file was modified outside emacs

org-unmodified asks to revert buffer if file was modified outside emacs

[Nicolas Richard]

Hello,

Short story is : if you call org-agenda-list and some agenda files have
been modified outside emacs (e.g. by some revision control system), the
command fails if the user choose to revert the file when asked, leaving
a blank agenda in which 'g' does not work. Does it need a fix ? Which
fix ?

Here is the recipe, from emacs -Q :
(require 'org)
(setq org-agenda-files (list (make-temp-file "test" nil ".org")))
;; the file must be non empty...
(with-temp-file (car org-agenda-files) (insert "foo"))
;; ...and visited
(find-file-noselect (car org-agenda-files))
(progn 
  ;; modify from outside emacs, randomly.
  (shell-command (format "echo '%S' > %s" (current-time) (car org-agenda-files)))
  (org-agenda-list))

At this point, you are asked a question :
testsomething.org changed on disk; really edit the buffer? (y, n, r or
C-h)

If you say 'y' (3 times), you get an agenda. If you say 'n' or 'r', you
get an empty window in which 'g' produces an error.

I'm not sure what is the best way to fix this. Does "fix" mean avoid the
blank window or make sure that 'g' never produces an error ?

Obviously (global-)auto-revert-mode could be part of the solution.
Another possibility is that org-unmodified should in fact make sure that
the buffer should be modifiable without a warning (that's the approach
of the patch below). Or the code using org-unmodified should do that on
its own if it needs to. Or we could catch the 'file-supersession signal,
but in this case we need a way to discriminate between the user having
chosen 'n' or 'r' (i.e. if trying to modify the buffer again will in
fact annoy the user or not). Or we can leave things as they are, but I
think we still need to avoid the error produced by 'g' in the blank
agenda buffer.

Anyway, here's a patch that modifies org-unmodified as proposed above. I
noticed, while writing this, that at multiple places, (buffer-modified-p)
is manually saved and restored. Should these make use of (org-unmodified) ?


From 0f170f72b2d080619bf1fcfd621113d51a7fc6ce Mon Sep 17 00:00:00 2001
From: "Nicolas Richard" <theonewiththeevillook@yahoo.fr>
Date: Thu, 20 Dec 2012 09:07:50 +0100
Subject: [PATCH] Do not let org-agenda-list ask for buffer reversion.

* lisp/org-macs.el (ask-user-about-supersession-threat): Advice to
suppress warning when a buffer is going to be modified while it was
already modified outside emacs.
(org-unmodified): Use the above advice to allow buffer modifications
with no warning.
* lisp/org.el (org-agenda-prepare-buffers): Use org-unmodified instead
of changing buffer-modified-p manually.
---
 lisp/org-macs.el |   12 ++++++++++++
 lisp/org.el      |   56 ++++++++++++++++++++++++++----------------------------
 2 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index a44bdbe..7abb3fd 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -87,6 +87,13 @@
 Otherwise return nil."
   (and v (not (equal v "nil")) v))
 
+(defadvice ask-user-about-supersession-threat (around silent-supersession-threat disable)
+  "Do not ask about supersession threat at all.
+While activated, this means that no warning will be issued
+whenever a buffer is modified while it had been already modified
+outside emacs. This does not however suppress the warning when
+saving such a buffer.")
+
 (defmacro org-unmodified (&rest body)
   "Execute body without changing `buffer-modified-p'.
 Also, do not record undo information."
@@ -94,6 +101,11 @@ Also, do not record undo information."
     (prog1 (buffer-modified-p)
       (let ((buffer-undo-list t)
 	    before-change-functions after-change-functions)
+	(ad-enable-advice 'ask-user-about-supersession-threat 'around 'silent-supersession-threat)
+	(ad-activate 'ask-user-about-supersession-threat)
+	(set-buffer-modified-p t)
+	(ad-disable-advice 'ask-user-about-supersession-threat 'around 'silent-supersession-threat)
+	(ad-activate 'ask-user-about-supersession-threat)
 	,@body))))
 (def-edebug-spec org-unmodified (body))
 
diff --git a/lisp/org.el b/lisp/org.el
index 9409fec..c408fa0 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -17012,35 +17012,33 @@ When a buffer is unmodified, it is just killed.  When modified, it is saved
 	      (org-check-agenda-file file)
 	      (set-buffer (org-get-agenda-file-buffer file)))
 	    (widen)
-	    (setq bmp (buffer-modified-p))
-	    (org-refresh-category-properties)
-	    (org-refresh-properties org-effort-property 'org-effort)
-	    (org-refresh-properties "APPT_WARNTIME" 'org-appt-warntime)
-	    (setq org-todo-keywords-for-agenda
-		  (append org-todo-keywords-for-agenda org-todo-keywords-1))
-	    (setq org-done-keywords-for-agenda
-		  (append org-done-keywords-for-agenda org-done-keywords))
-	    (setq org-todo-keyword-alist-for-agenda
-		  (append org-todo-keyword-alist-for-agenda org-todo-key-alist))
-	    (setq org-drawers-for-agenda
-		  (append org-drawers-for-agenda org-drawers))
-	    (setq org-tag-alist-for-agenda
-		  (append org-tag-alist-for-agenda org-tag-alist))
-
-	    (save-excursion
-	      (remove-text-properties (point-min) (point-max) pall)
-	      (when org-agenda-skip-archived-trees
-		(goto-char (point-min))
-		(while (re-search-forward rea nil t)
-		  (if (org-at-heading-p t)
-		      (add-text-properties (point-at-bol) (org-end-of-subtree t) pa))))
-	      (goto-char (point-min))
-	      (setq re (format org-heading-keyword-regexp-format
-			       org-comment-string))
-	      (while (re-search-forward re nil t)
-		(add-text-properties
-		 (match-beginning 0) (org-end-of-subtree t) pc)))
-	    (set-buffer-modified-p bmp)))))
+	    (org-unmodified
+	     (org-refresh-category-properties)
+	     (org-refresh-properties org-effort-property 'org-effort)
+	     (org-refresh-properties "APPT_WARNTIME" 'org-appt-warntime)
+	     (setq org-todo-keywords-for-agenda
+		   (append org-todo-keywords-for-agenda org-todo-keywords-1))
+	     (setq org-done-keywords-for-agenda
+		   (append org-done-keywords-for-agenda org-done-keywords))
+	     (setq org-todo-keyword-alist-for-agenda
+		   (append org-todo-keyword-alist-for-agenda org-todo-key-alist))
+	     (setq org-drawers-for-agenda
+		   (append org-drawers-for-agenda org-drawers))
+	     (setq org-tag-alist-for-agenda
+		   (append org-tag-alist-for-agenda org-tag-alist))
+	     (save-excursion
+	       (remove-text-properties (point-min) (point-max) pall)
+	       (when org-agenda-skip-archived-trees
+		 (goto-char (point-min))
+		 (while (re-search-forward rea nil t)
+		   (if (org-at-heading-p t)
+		       (add-text-properties (point-at-bol) (org-end-of-subtree t) pa))))
+	       (goto-char (point-min))
+	       (setq re (format org-heading-keyword-regexp-format
+				org-comment-string))
+	       (while (re-search-forward re nil t)
+		 (add-text-properties
+		  (match-beginning 0) (org-end-of-subtree t) pc))))))))
     (setq org-todo-keywords-for-agenda
           (org-uniquify org-todo-keywords-for-agenda))
     (setq org-todo-keyword-alist-for-agenda
-- 
1.7.9.5

Re: org-unmodified asks to revert buffer if file was modified outside emacs

[Bastien]

Hi Nicolas,

"Nicolas Richard" <theonewiththeevillook@yahoo.fr> writes:

> Short story is : if you call org-agenda-list and some agenda files have
> been modified outside emacs (e.g. by some revision control system), the
> command fails if the user choose to revert the file when asked, leaving
> a blank agenda in which 'g' does not work. Does it need a fix ? Which
> fix ?

Good questions.

I think it's acceptable to get this warning when an agenda file has
been changed outside of Emacs.

But having it twice is clearly wrong.

The problems lies in `org-agenda-prepare-buffers' which modifies the
properties of agenda files.  The deeper problem comes from Emacs, as
IMHO changing the properties of the buffer should be fine even when
the file has been changed outside, because the text properties of
the buffer are part of the buffer, not of the file.

remove-text-properties, set-text-properties etc. always ask for a
confirmation when the underlying file has been changed outside Emacs.

So for now I don't know how to fix this in Org.

I will ping Emacs developers for the rational behind the current
behavior of remove-text-properties and friends.

Thanks,

-- 
 Bastien

Re: org-unmodified asks to revert buffer if file was modified outside emacs

[Bastien]

Bastien <bzg@altern.org> writes:

> I will ping Emacs developers for the rational behind the current
> behavior of remove-text-properties and friends.

Got the answer: `with-silent-modifications' comes to the rescue.

So the issue is now fixed in master, thanks!

-- 
 Bastien