filenotify.el

filenotify.el

[Michael Albinus]

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

Hi,

I have written filenotify.el, which is intended as upper layer for
gfilenotify.c, inotify.c and w32notify.c. This is a simplified
interface, which offers just file change or file attribute change
notifications. I believe this is sufficient for the use-cases in Emacs;
if more fine granular notifications are needed, one could still use one
of the low-level packages.

The package is appended, together with needed changes in subr.el and
autorevert.el. I've tested it with gfilenotify and inotify; the tests
for w32notify I cannot perform as usual. There are still some changes
needed, but basic functionality shall be available.

Beside further tests and bug fixing, I plan to write ert tests as well
as a Tramp file name handler. Documentation is also lacking. But these
steps could be applied later, once there is an agreement about the
interface.

Comments?

Best regards, Michael.


[-- Attachment #2: filenotify.diff --]
[-- Type: text/x-diff, Size: 21329 bytes --]

=== modified file 'lisp/ChangeLog'
*** lisp/ChangeLog	2013-06-25 09:18:09 +0000
--- lisp/ChangeLog	2013-06-25 11:44:00 +0000
***************
*** 1,11 ****
  2013-06-25  Rüdiger Sonderfeld  <ruediger@c-plusplus.de>
  
! 	* lisp/textmodes/bibtex.el (bibtex-generate-url-list): Add support
  	for DOI URLs.
  
  2013-06-25  Rüdiger Sonderfeld  <ruediger@c-plusplus.de>
  
! 	* lisp/textmodes/bibtex.el (bibtex-mode, bibtex-set-dialect):
  	Update imenu-support when dialect changes.
  
  2013-06-25  Leo Liu  <sdl.web@gmail.com>
--- 1,23 ----
+ 2013-06-25  Michael Albinus  <michael.albinus@gmx.de>
+ 
+ 	* filenotify.el: New package.
+ 
+ 	* autorevert.el (top): Require filenotify.el.
+ 	(auto-revert-notify-enabled): Remove.  Use `file-notify-support'
+ 	instead.
+ 	(auto-revert-notify-rm-watch, auto-revert-notify-add-watch)
+ 	(auto-revert-notify-handler): Use `file-notify-*' functions.
+ 
+ 	* subr.el (file-notify-handle-event): Move function to filenotify.el.
+ 
  2013-06-25  Rüdiger Sonderfeld  <ruediger@c-plusplus.de>
  
! 	* textmodes/bibtex.el (bibtex-generate-url-list): Add support
  	for DOI URLs.
  
  2013-06-25  Rüdiger Sonderfeld  <ruediger@c-plusplus.de>
  
! 	* textmodes/bibtex.el (bibtex-mode, bibtex-set-dialect):
  	Update imenu-support when dialect changes.
  
  2013-06-25  Leo Liu  <sdl.web@gmail.com>

=== modified file 'lisp/autorevert.el'
*** lisp/autorevert.el	2013-06-05 19:57:10 +0000
--- lisp/autorevert.el	2013-06-25 11:41:17 +0000
***************
*** 103,108 ****
--- 103,109 ----
  
  (eval-when-compile (require 'cl-lib))
  (require 'timer)
+ (require 'filenotify)
  
  ;; Custom Group:
  ;;
***************
*** 270,290 ****
    :type 'boolean
    :version "24.4")
  
! (defconst auto-revert-notify-enabled
!   (or (featurep 'gfilenotify) (featurep 'inotify) (featurep 'w32notify))
!   "Non-nil when Emacs has been compiled with file notification support.")
! 
! (defcustom auto-revert-use-notify auto-revert-notify-enabled
    "If non-nil Auto Revert Mode uses file notification functions.
  This requires Emacs being compiled with file notification
! support (see `auto-revert-notify-enabled').  You should set this
! variable through Custom."
    :group 'auto-revert
    :type 'boolean
    :set (lambda (variable value)
! 	 (set-default variable (and auto-revert-notify-enabled value))
  	 (unless (symbol-value variable)
! 	   (when auto-revert-notify-enabled
  	     (dolist (buf (buffer-list))
  	       (with-current-buffer buf
  		 (when (symbol-value 'auto-revert-notify-watch-descriptor)
--- 271,287 ----
    :type 'boolean
    :version "24.4")
  
! (defcustom auto-revert-use-notify file-notify-support
    "If non-nil Auto Revert Mode uses file notification functions.
  This requires Emacs being compiled with file notification
! support (see `file-notify-support').  You should set this variable
! through Custom."
    :group 'auto-revert
    :type 'boolean
    :set (lambda (variable value)
! 	 (set-default variable (and file-notify-support value))
  	 (unless (symbol-value variable)
! 	   (when file-notify-support
  	     (dolist (buf (buffer-list))
  	       (with-current-buffer buf
  		 (when (symbol-value 'auto-revert-notify-watch-descriptor)
***************
*** 502,513 ****
  	     (puthash key value auto-revert-notify-watch-descriptor-hash-list)
  	   (remhash key auto-revert-notify-watch-descriptor-hash-list)
  	   (ignore-errors
! 	     (funcall
! 	      (cond
! 	       ((fboundp 'gfile-rm-watch) 'gfile-rm-watch)
! 	       ((fboundp 'inotify-rm-watch) 'inotify-rm-watch)
! 	       ((fboundp 'w32notify-rm-watch) 'w32notify-rm-watch))
! 	      auto-revert-notify-watch-descriptor)))))
       auto-revert-notify-watch-descriptor-hash-list)
      (remove-hook 'kill-buffer-hook 'auto-revert-notify-rm-watch))
    (setq auto-revert-notify-watch-descriptor nil
--- 499,505 ----
  	     (puthash key value auto-revert-notify-watch-descriptor-hash-list)
  	   (remhash key auto-revert-notify-watch-descriptor-hash-list)
  	   (ignore-errors
! 	     (file-notify-rm-watch auto-revert-notify-watch-descriptor)))))
       auto-revert-notify-watch-descriptor-hash-list)
      (remove-hook 'kill-buffer-hook 'auto-revert-notify-rm-watch))
    (setq auto-revert-notify-watch-descriptor nil
***************
*** 522,621 ****
  
    (when (and buffer-file-name auto-revert-use-notify
  	     (not auto-revert-notify-watch-descriptor))
!     (let ((func
! 	   (cond
! 	    ((fboundp 'gfile-add-watch) 'gfile-add-watch)
! 	    ((fboundp 'inotify-add-watch) 'inotify-add-watch)
! 	    ((fboundp 'w32notify-add-watch) 'w32notify-add-watch)))
! 	  (aspect
! 	   (cond
! 	    ((fboundp 'gfile-add-watch) '(watch-mounts))
! 	    ;; `attrib' is needed for file modification time.
! 	    ((fboundp 'inotify-add-watch) '(attrib create modify moved-to))
! 	    ((fboundp 'w32notify-add-watch) '(size last-write-time))))
! 	  (file (if (or (fboundp 'gfile-add-watch) (fboundp 'inotify-add-watch))
! 		    (directory-file-name (expand-file-name default-directory))
! 		  (buffer-file-name))))
!       (setq auto-revert-notify-watch-descriptor
! 	    (ignore-errors
! 	      (funcall func file aspect 'auto-revert-notify-handler)))
!       (if auto-revert-notify-watch-descriptor
! 	  (progn
! 	    (puthash
! 	     auto-revert-notify-watch-descriptor
! 	     (cons (current-buffer)
! 		   (gethash auto-revert-notify-watch-descriptor
! 			    auto-revert-notify-watch-descriptor-hash-list))
! 	     auto-revert-notify-watch-descriptor-hash-list)
! 	    (add-hook (make-local-variable 'kill-buffer-hook)
! 		      'auto-revert-notify-rm-watch))
! 	;; Fallback to file checks.
! 	(set (make-local-variable 'auto-revert-use-notify) nil)))))
! 
! (defun auto-revert-notify-event-p (event)
!   "Check that event is a file notification event."
!   (and (listp event)
!        (cond ((featurep 'gfilenotify)
! 	      (and (>= (length event) 3) (stringp (nth 2 event))))
! 	     ((featurep 'inotify)
! 	      (= (length event) 4))
! 	     ((featurep 'w32notify)
! 	      (and (= (length event) 3) (stringp (nth 2 event)))))))
! 
! (defun auto-revert-notify-event-descriptor (event)
!   "Return watch descriptor of file notification event, or nil."
!   (and (auto-revert-notify-event-p event) (car event)))
! 
! (defun auto-revert-notify-event-action (event)
!   "Return action of file notification event, or nil."
!   (and (auto-revert-notify-event-p event) (nth 1 event)))
! 
! (defun auto-revert-notify-event-file-name (event)
!   "Return file name of file notification event, or nil."
!   (and (auto-revert-notify-event-p event)
!        (cond ((featurep 'gfilenotify) (nth 2 event))
! 	     ((featurep 'inotify) (nth 3 event))
! 	     ((featurep 'w32notify) (nth 2 event)))))
  
  (defun auto-revert-notify-handler (event)
    "Handle an EVENT returned from file notification."
!   (when (auto-revert-notify-event-p event)
!     (let* ((descriptor (auto-revert-notify-event-descriptor event))
! 	   (action (auto-revert-notify-event-action event))
! 	   (file (auto-revert-notify-event-file-name event))
! 	   (buffers (gethash descriptor
! 			     auto-revert-notify-watch-descriptor-hash-list)))
!       (ignore-errors
! 	;; Check, that event is meant for us.
! 	;; TODO: Filter events which stop watching, like `move' or `removed'.
! 	(cl-assert descriptor)
! 	(cond
! 	 ((featurep 'gfilenotify)
! 	  (cl-assert (memq action '(attribute-changed changed created deleted
!                                     ;; FIXME: I keep getting this action, so I
!                                     ;; added it here, but I have no idea what
!                                     ;; I'm doing.  --Stef
!                                     changes-done-hint))
!                      t))
! 	 ((featurep 'inotify)
! 	  (cl-assert (or (memq 'attrib action)
! 			 (memq 'create action)
! 			 (memq 'modify action)
! 			 (memq 'moved-to action))))
! 	 ((featurep 'w32notify) (cl-assert (eq 'modified action))))
! 	;; Since we watch a directory, a file name must be returned.
! 	(cl-assert (stringp file))
! 	(dolist (buffer buffers)
! 	  (when (buffer-live-p buffer)
! 	    (with-current-buffer buffer
! 	      (when (and (stringp buffer-file-name)
! 			 (string-equal
! 			  (file-name-nondirectory file)
! 			  (file-name-nondirectory buffer-file-name)))
! 		;; Mark buffer modified.
! 		(setq auto-revert-notify-modified-p t)
! 		;; No need to check other buffers.
! 		(cl-return)))))))))
  
  (defun auto-revert-active-p ()
    "Check if auto-revert is active (in current buffer or globally)."
--- 514,562 ----
  
    (when (and buffer-file-name auto-revert-use-notify
  	     (not auto-revert-notify-watch-descriptor))
!     (setq auto-revert-notify-watch-descriptor
! 	  (ignore-errors
! 	    (file-notify-add-watch
! 	     (directory-file-name (expand-file-name default-directory))
! 	     '(change attribute-change) 'auto-revert-notify-handler)))
!     (if auto-revert-notify-watch-descriptor
! 	(progn
! 	  (puthash
! 	   auto-revert-notify-watch-descriptor
! 	   (cons (current-buffer)
! 		 (gethash auto-revert-notify-watch-descriptor
! 			  auto-revert-notify-watch-descriptor-hash-list))
! 	   auto-revert-notify-watch-descriptor-hash-list)
! 	  (add-hook (make-local-variable 'kill-buffer-hook)
! 		    'auto-revert-notify-rm-watch))
!       ;; Fallback to file checks.
!       (set (make-local-variable 'auto-revert-use-notify) nil))))
  
  (defun auto-revert-notify-handler (event)
    "Handle an EVENT returned from file notification."
!   (let* ((descriptor (car event))
! 	 (action (nth 1 event))
! 	 (file (nth 2 event))
! 	 (buffers (gethash descriptor
! 			   auto-revert-notify-watch-descriptor-hash-list)))
!     (ignore-errors
!       ;; Check, that event is meant for us.
!       ;; TODO: Filter events which stop watching, like `move' or `removed'.
!       (cl-assert descriptor)
!       (cl-assert (memq action '(attribute-changed changed created deleted)) t)
!       ;; Since we watch a directory, a file name must be returned.
!       (cl-assert (stringp file))
!       (dolist (buffer buffers)
! 	(when (buffer-live-p buffer)
! 	  (with-current-buffer buffer
! 	    (when (and (stringp buffer-file-name)
! 		       (string-equal
! 			(file-name-nondirectory file)
! 			(file-name-nondirectory buffer-file-name)))
! 	      ;; Mark buffer modified.
! 	      (setq auto-revert-notify-modified-p t)
! 	      ;; No need to check other buffers.
! 	      (cl-return))))))))
  
  (defun auto-revert-active-p ()
    "Check if auto-revert is active (in current buffer or globally)."

=== added file 'lisp/filenotify.el'
*** lisp/filenotify.el	1970-01-01 00:00:00 +0000
--- lisp/filenotify.el	2013-06-25 11:36:23 +0000
***************
*** 0 ****
--- 1,244 ----
+ ;;; filenotify.el --- watch files for changes on disk
+ 
+ ;; Copyright (C) 2013 Free Software Foundation, Inc.
+ 
+ ;; Author: Michael Albinus <michael.albinus@gmx.de>
+ 
+ ;; This file is part of GNU Emacs.
+ 
+ ;; GNU Emacs is free software: you can redistribute it and/or modify
+ ;; it under the terms of the GNU General Public License as published by
+ ;; the Free Software Foundation, either version 3 of the License, or
+ ;; (at your option) any later version.
+ 
+ ;; GNU Emacs is distributed in the hope that it will be useful,
+ ;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+ ;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ ;; GNU General Public License for more details.
+ 
+ ;; You should have received a copy of the GNU General Public License
+ ;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+ 
+ ;;; Commentary
+ 
+ ;; This package is an abstraction layer from the different low-level
+ ;; file notification packages `gfilenotify', `inotify' and
+ ;; `w32notify'.
+ 
+ ;;; Code:
+ 
+ ;; We check that there is exactly one low-level package.
+ ;;;###autoload
+ (defconst file-notify-support
+   (or (and (featurep 'gfilenotify)
+ 	   (not (featurep 'inotify))
+ 	   (not (featurep 'w32notify)))
+       (and (not (featurep 'gfilenotify))
+ 	   (featurep 'inotify)
+ 	   (not (featurep 'w32notify)))
+       (and (not (featurep 'gfilenotify))
+ 	   (not (featurep 'inotify))
+ 	   (featurep 'w32notify)))
+   "Non-nil when Emacs has been compiled with file notification support.")
+ 
+ ;; Shouldn't this be `file-notify-error'?
+ (defconst filewatch-error nil
+   "The file notification error symbol")
+ (put 'filewatch-error 'error-conditions '(filewatch-error file-error error))
+ (put 'filewatch-error 'error-message "Filewatch error")
+ 
+ (defvar file-notify-descriptors (make-hash-table :test 'equal)
+   "Hash table for registered file notification descriptors.
+ A key in this hash table is the descriptor as returned from a
+ `gfilenotify', `inotify' or `w32notify'.  The value in the hash
+ table is the cons cell (FILE . CALLBACK).")
+ 
+ ;; This function is used by `gfilenotify', `inotify' and `w32notify' events.
+ ;;;###autoload
+ (defun file-notify-handle-event (event)
+   "Handle file system monitoring event.
+ If EVENT is a filewatch event, call its callback.
+ Otherwise, signal a `filewatch-error'."
+   (interactive "e")
+   (if (and (eq (car event) 'file-notify)
+ 	   (>= (length event) 3))
+       (funcall (nth 2 event) (nth 1 event))
+     (signal 'filewatch-error
+ 	    (cons "Not a valid file-notify event" event))))
+ 
+ (defun file-notify-event-file-name (event)
+   "Return file name of file notification event, or nil."
+   (cond ((featurep 'gfilenotify) (nth 2 event))
+ 	((featurep 'inotify) (nth 3 event))
+ 	((featurep 'w32notify) (nth 2 event))))
+ 
+ (defun file-notify-event-file1-name (event)
+   "Return second file name of file notification event, or nil.
+ This is available in case a file has been moved."
+   (cond ((featurep 'gfilenotify) (nth 3 event))
+ 	((featurep 'inotify) (nth 4 event))
+ 	((featurep 'w32notify) (nth 3 event))))
+ 
+ ;; The callback function used to map between specific flags of the
+ ;; respective file notifications, and the ones we return.
+ (defun file-notify-callback (event)
+   "Handle an EVENT returned from file notification.
+ EVENT is the same one as in `file-notify-handle-event' except the
+ car of that event, which is the symbol `file-notify'."
+   (let ((descriptor (car event))
+ 	(action (nth 1 event))
+ 	(file (file-notify-event-file-name event))
+ 	(file1 (file-notify-event-file1-name event))
+ 	 callback tmp)
+     ;; Check, that event is meant for us.
+     (unless (setq callback (cdr (gethash descriptor file-notify-descriptors)))
+       (signal 'filewatch-error (list "Not a valid descriptor" descriptor)))
+ 
+     ;; Map action.  We ignore all events which cannot be mapped.
+     (setq action
+ 	  (cond
+ 	   ((featurep 'gfilenotify)
+ 	    (cond
+ 	     ((memq action '(attribute-changed changed created deleted moved))
+ 	      action)))
+ 	   ((featurep 'inotify)
+ 	    (cond
+ 	     ((memq 'attrib action) 'attribute-changed)
+ 	     ((memq 'create action) 'created)
+ 	     ((memq 'modify action) 'changed)
+ 	     ((memq 'delete action) 'deleted)
+ 	     ((memq 'delete-self action) 'deleted)
+ 	     ((memq 'moved-from action) 'deleted)
+ 	     ((memq 'moved-to action) 'created)
+ 	     ((memq 'move-self action) 'deleted)))
+ 	   ((featurep 'w32notify)
+ 	    (cond
+ 	     ((eq 'added action) 'created)
+ 	     ((eq 'modified action) 'changed)
+ 	     ((eq 'removed action) 'deleted)
+ 	     ((eq 'renamed-from action) 'deleted)
+ 	     ((eq 'renamed-to action) 'created)))))
+ 
+     ;; Apply callback.
+     (when (and (stringp file) (eq action 'moved))
+       (funcall callback (list descriptor 'deleted file))
+       (setq action 'created file file1))
+     (when (and (stringp file) action)
+       (funcall callback (list descriptor action file)))))
+ 
+ ;; We do not return a `renamed' action, for simplicity.  This could be
+ ;; added later on.
+ (defun file-notify-add-watch (file flags callback)
+   "Add a watch for filesystem events pertaining to FILE.
+ This arranges for filesystem events pertaining to FILE to be reported
+ to Emacs.  Use `file-notify-rm-watch' to cancel the watch.
+ 
+ The returned value is a descriptor for the added watch.  If the
+ file cannot be watched for some reason, this function signals a
+ `file-error' error.
+ 
+ FLAGS is a list of conditions to set what will be watched for.  It can
+ include the following symbols:
+ 
+   `change'           -- watch for file changes
+   `attribute-change' -- watch for file attributes changes, like
+                         permissions or modification time
+ 
+ If FILE is a directory, 'change' watches for file creation or
+ deletion in that directory.
+ 
+ When any event happens, Emacs will call the CALLBACK function passing
+ it a single argument EVENT, which is of the form
+ 
+   (DESCRIPTOR ACTION FILE [FILE1])
+ 
+ DESCRIPTOR is the same object as the one returned by this function.
+ ACTION is the description of the event.  It could be any one of the
+ following:
+ 
+   `created'           -- FILE was created
+   `deleted'           -- FILE was deleted
+   `changed'           -- FILE has changed
+   `attribute-changed' -- a FILE attribute was changed
+ 
+ FILE is the name of the file whose event is being reported."
+   ;; Check arguments.
+   (unless (stringp file)
+     (signal 'wrong-type-argument (list file)))
+   (setq file (expand-file-name file))
+   (unless (and (consp flags)
+ 	       (null (delq 'change (delq 'attribute-change flags))))
+     (signal 'wrong-type-argument (list flags)))
+   (unless (functionp callback)
+     (signal 'wrong-type-argument (list callback)))
+ 
+   (let ((handler (find-file-name-handler file 'file-notify-add-watch))
+ 	func fl desc)
+     (if handler
+ 	;; A file name handler could exist even if there is no local
+ 	;; file notification support.
+ 	(setq desc (funcall handler 'file-notify-add-watch file flags callback))
+ 
+       ;; Check, whether Emacs has been compiled with file notification support.
+       (unless file-notify-support
+ 	(signal 'filewatch-error '("No file notification package available")))
+ 
+       ;; Check, whether this has been registered already.
+       (maphash
+        (lambda (key value)
+ 	 (when (equal (cons file callback) value) (setq desc key)))
+        file-notify-descriptors)
+ 
+       (unless desc
+ 	;; Determine local function to be called.
+ 	(setq func (cond
+ 		    ((fboundp 'gfile-add-watch) 'gfile-add-watch)
+ 		    ((fboundp 'inotify-add-watch) 'inotify-add-watch)
+ 		    ((fboundp 'w32notify-add-watch) 'w32notify-add-watch)))
+ 
+ 	;; Determine respective flags.
+ 	(if (fboundp 'gfile-add-watch)
+ 	    (setq fl '(watch-mounts))
+ 	  (when (memq 'change flags)
+ 	    (setq
+ 	     fl
+ 	     (cond
+ 	      ((fboundp 'inotify-add-watch) '(create modify move delete))
+ 	      ((fboundp 'w32notify-add-watch) '(size last-write-time)))))
+ 	  (when (memq 'attribute-change flags)
+ 	    (add-to-list
+ 	     'fl
+ 	     (cond
+ 	      ((fboundp 'inotify-add-watch) '(attrib))
+ 	      ((fboundp 'w32notify-add-watch) '(attributes))))))
+ 
+ 	;; Call low-level function.
+ 	(setq desc (funcall func file fl 'file-notify-callback))))
+ 
+     ;; Return descriptor.
+     (puthash desc (cons file callback) file-notify-descriptors)
+     desc))
+ 
+ (defun file-notify-rm-watch (descriptor)
+   "Remove an existing watch specified by its DESCRIPTOR.
+ DESCRIPTOR should be an object returned by `file-notify-add-watch'."
+   (let ((file (car (gethash descriptor file-notify-descriptors)))
+ 	handler)
+ 
+     (when (stringp file)
+       (setq handler (find-file-name-handler file 'file-notify-rm-watch))
+       (if handler
+ 	  (funcall handler 'file-notify-rm-watch descriptor)
+ 	(funcall
+ 	 (cond
+ 	  ((fboundp 'gfile-rm-watch) 'gfile-rm-watch)
+ 	  ((fboundp 'inotify-rm-watch) 'inotify-rm-watch)
+ 	  ((fboundp 'w32notify-add-watch) 'w32notify-rm-watch))
+ 	 descriptor)))
+ 
+     (remhash descriptor file-notify-descriptors)))
+ 
+ ;; The end:
+ (provide 'filenotify)
+ 
+ ;;; filenotify.el ends here

=== modified file 'lisp/subr.el'
*** lisp/subr.el	2013-06-20 14:15:42 +0000
--- lisp/subr.el	2013-06-24 07:49:32 +0000
***************
*** 4494,4513 ****
         nil ,@(cdr (cdr spec)))))
  
  \f
- ;;;; Support for watching filesystem events.
- 
- (defun file-notify-handle-event (event)
-   "Handle file system monitoring event.
- If EVENT is a filewatch event, call its callback.
- Otherwise, signal a `filewatch-error'."
-   (interactive "e")
-   (if (and (eq (car event) 'file-notify)
- 	   (>= (length event) 3))
-       (funcall (nth 2 event) (nth 1 event))
-     (signal 'filewatch-error
- 	    (cons "Not a valid file-notify event" event))))
- 
- \f
  ;;;; Comparing version strings.
  
  (defconst version-separator "."
--- 4494,4499 ----

Re: filenotify.el

[Rüdiger Sonderfeld]

Hi,

On Tuesday 25 June 2013 14:02:38 Michael Albinus wrote:
> I have written filenotify.el, which is intended as upper layer for
> gfilenotify.c, inotify.c and w32notify.c. This is a simplified
> interface, which offers just file change or file attribute change
> notifications. I believe this is sufficient for the use-cases in Emacs;
> if more fine granular notifications are needed, one could still use one
> of the low-level packages.

Thanks for your work so far.  I agree for the upper layer portability is most 
important and not fine granularity.

> Beside further tests and bug fixing, I plan to write ert tests as well
> as a Tramp file name handler. Documentation is also lacking. But these
> steps could be applied later, once there is an agreement about the
> interface.

Yes, absolutely.  There is a very basic test for inotify which could be 
adopted.

> Comments?

I have a few comments:

+   (or (and (featurep 'gfilenotify)
+ 	   (not (featurep 'inotify))
+ 	   (not (featurep 'w32notify)))
+       (and (not (featurep 'gfilenotify))
+ 	   (featurep 'inotify)
+ 	   (not (featurep 'w32notify)))
+       (and (not (featurep 'gfilenotify))
+ 	   (not (featurep 'inotify))
+ 	   (featurep 'w32notify)))

Why is the support exclusive?

Maybe we could set `file-notify-support' to the name of the low-level feature.  
This would provide an easy way to identify which one is used.

+ ;; Shouldn't this be `file-notify-error'?

Yes, this would be more consistent in my opinion.

+ (defun file-notify-event-file1-name (event)
...
+ 	((featurep 'inotify) (nth 4 event))

The event layout for inotify is (WATCH-DESCRIPTOR ASPECTS COOKIE NAME).  The 
new file name will be NAME in the `moved-to' event and it is connected with 
the COOKIE to the `moved-from' event.

+     ;; Check, that event is meant for us.
+     (unless (setq callback (cdr (gethash descriptor file-notify-
descriptors)))
+       (signal 'filewatch-error (list "Not a valid descriptor" descriptor)))

I'm not sure if this should be treated as an error.  There could be the case 
that a file event is still in the queue while the watch-descriptor is removed 
and it will still be delivered.

But I just saw that this behaviour was also changed in `file-notify-handle-
event' a while ago.  I think this needs a bit more discussion.

+ 	     ((memq 'moved-from action) 'deleted)
+ 	     ((memq 'moved-to action) 'created)
...
+ 	     ((eq 'renamed-from action) 'deleted)
+ 	     ((eq 'renamed-to action) 'created)))))
...
+     ;; Apply callback.
+     (when (and (stringp file) (eq action 'moved))
+       (funcall callback (list descriptor 'deleted file))
+       (setq action 'created file file1))
+     (when (and (stringp file) action)
+       (funcall callback (list descriptor action file)))))

I don't think this is very useful because there is no way for the user to 
connect both events.  Inotify provides COOKIE to do this.

Maybe we should introduce a moved-from/moved-to event using a cookie instead.  
This should be easier to implement for all APIs.

+ 	     ((memq 'move-self action) 'deleted)))

Are you sure this is correct?  If I remember correctly then inotify sets the 
file name to the new file name.  I'll look into it.

Regards
Rüdiger

Re: filenotify.el

[Michael Albinus]

Rüdiger Sonderfeld <ruediger@c-plusplus.de> writes:

> Hi,

Hi Rüdiger,

> I have a few comments:
>
> +   (or (and (featurep 'gfilenotify)
> + 	   (not (featurep 'inotify))
> + 	   (not (featurep 'w32notify)))
> +       (and (not (featurep 'gfilenotify))
> + 	   (featurep 'inotify)
> + 	   (not (featurep 'w32notify)))
> +       (and (not (featurep 'gfilenotify))
> + 	   (not (featurep 'inotify))
> + 	   (featurep 'w32notify)))
>
> Why is the support exclusive?

Just to be sure. The different low-level packages are not fully
compatible (event names differ, for example); it would be a nightmare
for filenotify.el when the different packages compete under the hood.

In general, it could be possible to apply several low-level file
notification libraries in parallel. But this wouldn't bring additional
features, so we should avoid this.

(Out of curiosity, I let compete file monitoring for "/tmp" and
"/ssh::/tmp" in parallel. Very funny.)

> Maybe we could set `file-notify-support' to the name of the low-level
> feature.  This would provide an easy way to identify which one is
> used.

Really? I don't see a serious difference between (featurep 'inotify) and
(eq file-notify-support 'inotify)

> + ;; Shouldn't this be `file-notify-error'?
>
> Yes, this would be more consistent in my opinion.

If nobody objects, I will change it here as well as in the low-level
packages, which use `file-error'.

> + (defun file-notify-event-file1-name (event)
> ...
> + 	((featurep 'inotify) (nth 4 event))
>
> The event layout for inotify is (WATCH-DESCRIPTOR ASPECTS COOKIE NAME).  The 
> new file name will be NAME in the `moved-to' event and it is connected with 
> the COOKIE to the `moved-from' event.

Just now, this function is used only for gfilenotify, event `moved'. I
will remove the other two cases from that function.

> +     ;; Check, that event is meant for us.
> +     (unless (setq callback (cdr (gethash descriptor file-notify-
> descriptors)))
> +       (signal 'filewatch-error (list "Not a valid descriptor" descriptor)))
>
> I'm not sure if this should be treated as an error.  There could be the case 
> that a file event is still in the queue while the watch-descriptor is removed 
> and it will still be delivered.
>
> But I just saw that this behaviour was also changed in `file-notify-handle-
> event' a while ago.  I think this needs a bit more discussion.

I believe, that the callback function needs special care from the
callee. In `auto-revert-notify-handler', I wrap the sensitive code by
`ignore-errors'. For example.

But I agree, this might be discussed.

> + 	     ((memq 'moved-from action) 'deleted)
> + 	     ((memq 'moved-to action) 'created)
> ...
> + 	     ((eq 'renamed-from action) 'deleted)
> + 	     ((eq 'renamed-to action) 'created)))))
> ...
> +     ;; Apply callback.
> +     (when (and (stringp file) (eq action 'moved))
> +       (funcall callback (list descriptor 'deleted file))
> +       (setq action 'created file file1))
> +     (when (and (stringp file) action)
> +       (funcall callback (list descriptor action file)))))
>
> I don't think this is very useful because there is no way for the user to 
> connect both events.  Inotify provides COOKIE to do this.
>
> Maybe we should introduce a moved-from/moved-to event using a cookie instead.  
> This should be easier to implement for all APIs.

That's also my intention. But Glib's implementation does not implement
this for all backends, see the comment in
<https://developer.gnome.org/gio/2.36/GFile.html#GFileMonitorFlags-enum>.

The w32notify case I couldn't test yet.

It is on my todo list; once there is a robust implementation for all
low-level packages, we shall add a `renamed' event to be returned.

> + 	     ((memq 'move-self action) 'deleted)))
>
> Are you sure this is correct?  If I remember correctly then inotify sets the 
> file name to the new file name.  I'll look into it.

I'll check also.

> Regards
> Rüdiger

Best regards, Michael.

Re: filenotify.el

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Tue, 25 Jun 2013 21:00:43 +0200
> Cc: emacs-devel@gnu.org
> 
> > +     ;; Apply callback.
> > +     (when (and (stringp file) (eq action 'moved))
> > +       (funcall callback (list descriptor 'deleted file))
> > +       (setq action 'created file file1))
> > +     (when (and (stringp file) action)
> > +       (funcall callback (list descriptor action file)))))
> >
> > I don't think this is very useful because there is no way for the user to 
> > connect both events.  Inotify provides COOKIE to do this.
> >
> > Maybe we should introduce a moved-from/moved-to event using a cookie instead.  
> > This should be easier to implement for all APIs.
> 
> That's also my intention. But Glib's implementation does not implement
> this for all backends, see the comment in
> <https://developer.gnome.org/gio/2.36/GFile.html#GFileMonitorFlags-enum>.
> 
> The w32notify case I couldn't test yet.

w32notify produces 2 events for a file that was renamed: a
'renamed-from' event, followed by a 'renamed-to' event.

Re: filenotify.el

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

> w32notify produces 2 events for a file that was renamed: a
> 'renamed-from' event, followed by a 'renamed-to' event.

Is there a guarantee, that `renamed-to' will always follow
`renamed-from'? No other event in between?

How long shall we wait, until we give up, and show 'renamed-from' as
`deleted'? Events could be lost.

Btw, the same question arises for inotify. But here we have the cookie,
which gives a better correlation.

Re: filenotify.el

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: ruediger@c-plusplus.de,  emacs-devel@gnu.org
> Date: Tue, 25 Jun 2013 21:27:57 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > w32notify produces 2 events for a file that was renamed: a
> > 'renamed-from' event, followed by a 'renamed-to' event.
> 
> Is there a guarantee, that `renamed-to' will always follow
> `renamed-from'? No other event in between?
> 
> How long shall we wait, until we give up, and show 'renamed-from' as
> `deleted'? Events could be lost.

I'd say, if the very next event is not renamed-to, stop waiting.

Re: filenotify.el

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> How long shall we wait, until we give up, and show 'renamed-from' as
>> `deleted'? Events could be lost.
>
> I'd say, if the very next event is not renamed-to, stop waiting.

Thanks, I'll implement it this way. If there is a pending `renamed-from'
event, and the next event is not `renamed-to', I'll fire the pending event
as `removed' before handling the incoming event.

Best regards, Michael.

Re: filenotify.el

[Stefan Monnier]

> Just to be sure. The different low-level packages are not fully
> compatible (event names differ, for example); it would be a nightmare
> for filenotify.el when the different packages compete under the hood.

Why would they compete?  In the unlikely case that both gfilenotify and
inotify are provided, can't the code just use one of the two without
being impacted by the other?  If not, why not?  Can we fix it so as to
avoid such conflicts?

> In general, it could be possible to apply several low-level file
> notification libraries in parallel. But this wouldn't bring additional
> features, so we should avoid this.

Agreed, but the better option is to make sure they don't conflict with
each other, and then make filenotify.el choose one of the available ones
"arbitrarily".

We may also want to tweak configure to avod building Emacs with support
for multiple file notification systems, but if we assume a future with
FFI, then multiple notification systems may occur dynamically and there
isn't any good reason to really disallow it.


        Stefan

Re: filenotify.el

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Just to be sure. The different low-level packages are not fully
>> compatible (event names differ, for example); it would be a nightmare
>> for filenotify.el when the different packages compete under the hood.
>
> Why would they compete?  In the unlikely case that both gfilenotify and
> inotify are provided, can't the code just use one of the two without
> being impacted by the other?  If not, why not?  Can we fix it so as to
> avoid such conflicts?

[...]

> Agreed, but the better option is to make sure they don't conflict with
> each other, and then make filenotify.el choose one of the available ones
> "arbitrarily".

Well, filenotify.el shall decide for one of the available low-level
packages, and use it. I'll change accordingly.

However, any Lisp package could still decide to activate "the other"
low-level file notification package if linked to Emacs. In this case, we
must ensure that the incoming file-notify events can be distinguished.

Such an event looks like (DESCRIPTOR ACTION FILE). Different low-level
packages must use unique DESCRIPTORs then. As of today, this is not
guaranteed; both gfilenotify and inotify use integers for example.

There's no urgency for this; as of today Emacs' configure decides for
one and only one package. And in Tramp I use already descriptors which
are unique for Tramp (it's the internal Tramp vector for the file to be
monitored).

>         Stefan

Best regards, Michael.

Re: filenotify.el

[Stefan Monnier]

> However, any Lisp package could still decide to activate "the other"
> low-level file notification package if linked to Emacs.

So what?

> In this case, we must ensure that the incoming file-notify events can
> be distinguished.

Where?  In the C code or in filenotify.el?  AFAICT, at the Elisp level,
a callback is only called for a given "watcher" (which is even more
specific than a "notification library"), so I see no reason why
a callback setup with inotify-add-watch should be called by gfilenotify,
for example.

> Such an event looks like (DESCRIPTOR ACTION FILE). Different low-level
> packages must use unique DESCRIPTORs then.

Just use (inotify DESCRIPTOR ACTION FILE) instead, if that's really needed.


        Stefan

Re: filenotify.el

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> In this case, we must ensure that the incoming file-notify events can
>> be distinguished.
>
> Where?  In the C code or in filenotify.el?  AFAICT, at the Elisp level,
> a callback is only called for a given "watcher" (which is even more
> specific than a "notification library"), so I see no reason why
> a callback setup with inotify-add-watch should be called by gfilenotify,
> for example.

Imagine there are two registered watchers:

(gfile-add-watch "/tmp" flags1 'callback1)
(inotify-add-watch "/tmp" flags2 'callback2)

Both return the same descriptor, let's say 1. What would you call, when
there is an incoming event (1 flag "123") for the file "/tmp/123"? Would
you apply callback1 or callback2? You don't know whether the event comes
from gfilenotify or inotify. Maybe one could distinguish wrt the
returned flag, but I wouldn't bet that's always unique.

>> Such an event looks like (DESCRIPTOR ACTION FILE). Different low-level
>> packages must use unique DESCRIPTORs then.
>
> Just use (inotify DESCRIPTOR ACTION FILE) instead, if that's really needed.

I would make the descriptor a cons cell '(inotify . number), but it is
the same idea. Again, for Tramp I use already a vector as descriptor.
The only promise we have given is, that descriptors returned by
*-add-watch functions can be distinguished via `equal'.

No need to change it today, because there are no two packages linked in
parallel yet. But this might be needed in the future.

>         Stefan

Best regards, Michael.

Re: filenotify.el

[Stefan Monnier]

> Imagine there are two registered watchers:
> (gfile-add-watch "/tmp" flags1 'callback1)
> (inotify-add-watch "/tmp" flags2 'callback2)
> Both return the same descriptor, let's say 1. What would you call, when
> there is an incoming event (1 flag "123") for the file "/tmp/123"?

Who's "you"?  IIUC it's the C code, so if the C code calls inotify's
callback when receiving gfile's event it's a bug in the C code that
needs fixing regardless of filenotify.el.

>>> Such an event looks like (DESCRIPTOR ACTION FILE). Different low-level
>>> packages must use unique DESCRIPTORs then.
>> Just use (inotify DESCRIPTOR ACTION FILE) instead, if that's really needed.
> I would make the descriptor a cons cell '(inotify . number), but it is
> the same idea.

I don't see any need to make any change to the Elisp visible behavior.

> Again, for Tramp I use already a vector as descriptor.
> The only promise we have given is, that descriptors returned by
> *-add-watch functions can be distinguished via `equal'.

Also that a callback is only called for those events on which it
was added.


        Stefan

Re: filenotify.el

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Imagine there are two registered watchers:
>> (gfile-add-watch "/tmp" flags1 'callback1)
>> (inotify-add-watch "/tmp" flags2 'callback2)
>> Both return the same descriptor, let's say 1. What would you call, when
>> there is an incoming event (1 flag "123") for the file "/tmp/123"?
>
> Who's "you"?  IIUC it's the C code, so if the C code calls inotify's
> callback when receiving gfile's event it's a bug in the C code that
> needs fixing regardless of filenotify.el.

"you" is `file-notify-handle-event'. It is in subr.el, I will move it
to filenotify.el. It does not know of gfilenotify, inotify or w32notify.

>         Stefan

Best regards, Michael.

Re: filenotify.el

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Wed, 26 Jun 2013 10:28:24 -0400
> Cc: Rüdiger Sonderfeld <ruediger@c-plusplus.de>,
> 	emacs-devel@gnu.org
> 
> > Imagine there are two registered watchers:
> > (gfile-add-watch "/tmp" flags1 'callback1)
> > (inotify-add-watch "/tmp" flags2 'callback2)
> > Both return the same descriptor, let's say 1. What would you call, when
> > there is an incoming event (1 flag "123") for the file "/tmp/123"?
> 
> Who's "you"?  IIUC it's the C code

No.  C code just pushes an event onto the event queue.  The rest is
done in Lisp.

Re: filenotify.el

[Stefan Monnier]

>> > Imagine there are two registered watchers:
>> > (gfile-add-watch "/tmp" flags1 'callback1)
>> > (inotify-add-watch "/tmp" flags2 'callback2)
>> > Both return the same descriptor, let's say 1. What would you call, when
>> > there is an incoming event (1 flag "123") for the file "/tmp/123"?
>> Who's "you"?  IIUC it's the C code
> No.  C code just pushes an event onto the event queue.  The rest is
> done in Lisp.

Then the event should have an `inotify', `w32notify' or `gfile' tag.
Of course, that could be included in the "cookie" (which really should
be a new object type either in Lisp_Misc or in Lisp_Vectorlike).


        Stefan

Re: filenotify.el

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Then the event should have an `inotify', `w32notify' or `gfile' tag.
> Of course, that could be included in the "cookie" (which really should
> be a new object type either in Lisp_Misc or in Lisp_Vectorlike).

We have had that in the beginning ... it has been changed, in order to
apply the same event handler for all the notification events.

Which tag would you give an event when it is pushed by Tramp? `remote'?

Honestly, I don't believe it is worth the trouble just now, since we
haven't a problem yet. We could think about when it is necessary.

Changing the event name wouldn't break anything; it is used internally
only in the different *notify.{c,el} packages.

And putting the tag into the descriptor, as I have proposed, would even
not require a change in the event name. The descriptors must just be
unique, that's all.

>         Stefan

Best regards, Michael.

Re: filenotify.el

[Rüdiger Sonderfeld]

One minor nitpick I forgot:  You should use two hyphens for functions which 
are not meant to be used by the user.  I guess this should also include `file-
notify--handle-event'.

It seems those symbols

* file-notify-callback
* file-notify-descriptors
* file-notify-event-file-name
* file-notify-event-file1-name
* file-notify-callback

should not be directly used by the user.

On Tuesday 25 June 2013 21:00:43 you wrote:
> Just to be sure. The different low-level packages are not fully
> compatible (event names differ, for example); it would be a nightmare
> for filenotify.el when the different packages compete under the hood.
>
> In general, it could be possible to apply several low-level file
> notification libraries in parallel. But this wouldn't bring additional
> features, so we should avoid this.
>
> (Out of curiosity, I let compete file monitoring for "/tmp" and
> "/ssh::/tmp" in parallel. Very funny.)

I agree with Stefan here.  I don't see a reason why Emacs couldn't be compiled 
with several filenotify APIs.  Both inotify and gfilenotify are available on 
the same system.  filenotify.el should of course only pick one.  But it 
shouldn't really be affected if there are more available.

> > Maybe we could set `file-notify-support' to the name of the low-level
> > feature.  This would provide an easy way to identify which one is
> > used.
> 
> Really? I don't see a serious difference between (featurep 'inotify) and
> (eq file-notify-support 'inotify)

This would of course only make sense if the support isn't exclusive.

> I believe, that the callback function needs special care from the
> callee. In `auto-revert-notify-handler', I wrap the sensitive code by
> `ignore-errors'. For example.

But this error couldn't be handled by `ignore-errors' because it happens 
before the callback is called.  Or am I misunderstanding you?

Regards
Rüdiger

Re: filenotify.el

[Michael Albinus]

Rüdiger Sonderfeld <ruediger@c-plusplus.de> writes:

> One minor nitpick I forgot:  You should use two hyphens for functions which 
> are not meant to be used by the user.

Ahh. This is a rather new coding style in Emacs; it didn't even find
its way into the Lisp Refefence Manual yet :-)

But it's useful; I'll apply.

>  I guess this should also include `file-notify--handle-event'.

Don't know. I'm not decided yet whether to use this function also in
Tramp.

>> Really? I don't see a serious difference between (featurep 'inotify) and
>> (eq file-notify-support 'inotify)
>
> This would of course only make sense if the support isn't exclusive.

OK, so I set it to the used package symbol. But it must be documented
clearly, that this is applied for *local* file systems only; Tramp will use
a different implementation very likely.

Later, when there are several low-level file notification packages
linked to Emacs in parallel, we could change `file-notify-support' into
a defcustom. The user could decide which of the packages to use.

Maybe we shall give it another name for that purpose.

>> I believe, that the callback function needs special care from the
>> callee. In `auto-revert-notify-handler', I wrap the sensitive code by
>> `ignore-errors'. For example.
>
> But this error couldn't be handled by `ignore-errors' because it happens 
> before the callback is called.  Or am I misunderstanding you?

Where shall this happen? In `file-notify-handle-event'? This function
does not depend on a registered file watcher.

> Regards
> Rüdiger

Best regards, Michael.

About `<prefix>--' (was Re: filenotify.el)

[Stephen Berman]

On Wed, 26 Jun 2013 08:40:11 +0200 Michael Albinus <michael.albinus@gmx.de> wrote:

> Rüdiger Sonderfeld <ruediger@c-plusplus.de> writes:
>
>> One minor nitpick I forgot:  You should use two hyphens for functions which 
>> are not meant to be used by the user.
>
> Ahh. This is a rather new coding style in Emacs; it didn't even find
> its way into the Lisp Refefence Manual yet :-)

Actually, it did, but just barely, in (elisp)Coding Conventions:

     Separate the prefix from the rest of the name with a hyphen, `-'.
     Use two hyphens if the symbol is not meant to be used by other
     packages.  This practice helps avoid name conflicts, since all
     global variables in Emacs Lisp share the same name space, and all
     functions share another name space

I have two problems with this.  First, the "Use two hyphens" sentence
shouldn't come between the other two, since "This practice" in the
following sentence only refers to using the prefix, not two hyphens.
Second, "not meant to be used by other packages" is too vague for me to
feel confident about how to apply it.  For example, in the new version
of todo-mode.el I recently committed to the trunk, there aren't really
any symbols, including those naming commands and user options, meant to
be used by other packages, since it's not a general purpose library.  On
the other hand, it does contain some internal functions and variables of
a rather general character, needed to implement parts of the UI, which I
included because Emacs doesn't have them (e.g. a powerset function), and
these certainly could be used by other packages, but more likely they
would be reimplemented using the package prefix.  So it's not clear to
me where the dividing line is.  It could be taken to be everything
except commands and user options, as Rüdiger seems to imply (unless he
meant "the user" is a package, not a human).  But it's clearly not been
used so restrictively in Emacs to date (indeed, mpc.el even contains a
command `mpc--faster').  So I would find it helpful if there were more
specific guidelines about how to apply this convention.

Steve Berman

Re: About `<prefix>--' (was Re: filenotify.el)

[Stefan Monnier]

> I have two problems with this.  First, the "Use two hyphens" sentence
> shouldn't come between the other two, since "This practice" in the
> following sentence only refers to using the prefix, not two hyphens.

Good point.

> Second, "not meant to be used by other packages" is too vague for me to
> feel confident about how to apply it.  For example, in the new version

It means that something defined with "--" can completely change (or
disappear) from one release to the other.


        Stefan

Re: About `<prefix>--' (was Re: filenotify.el)

[Stephen Berman]

On Wed, 26 Jun 2013 09:06:52 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>> Second, "not meant to be used by other packages" is too vague for me to
>> feel confident about how to apply it.  For example, in the new version
>
> It means that something defined with "--" can completely change (or
> disappear) from one release to the other.

Can I borrow your crystal ball before the next release?

Steve Berman

Re: About `<prefix>--' (was Re: filenotify.el)

[Stefan Monnier]

>>> Second, "not meant to be used by other packages" is too vague for me to
>>> feel confident about how to apply it.  For example, in the new version
>> It means that something defined with "--" can completely change (or
>> disappear) from one release to the other.
> Can I borrow your crystal ball before the next release?

Sure, I'll put it on GNU ELPA, so everyone can benefit,


        Stefan

Re: About `<prefix>--' (was Re: filenotify.el)

[Josh]

On Wed, Jun 26, 2013 at 12:50 AM, Stephen Berman <stephen.berman@gmx.net> wrote:
> [...] On
> the other hand, it does contain some internal functions and variables of
> a rather general character, needed to implement parts of the UI, which I
> included because Emacs doesn't have them (e.g. a powerset function), and
> these certainly could be used by other packages, but more likely they
> would be reimplemented using the package prefix.

Coincidentally enough, I needed a power set function just yesterday.
After fruitlessly grepping for one on my system I ended up writing
my own.  I wish I had procrastinated a bit longer so I could have
used yours :)

It would be great if more of these generally useful functions were
"promoted" from the libraries where they originated into the core of
Emacs, stripped of package prefixes and properly documented in the
Emacs or Elisp info manuals in order to make them more discoverable
for use elsewhere.  Doubtless many wheels have been reinvented after
failed searches for functionality that already ships with Emacs.  For
example, I have implemented and later stumbled across existing
implementations of anaphora (later found in lisp/ibuf-macs.el) and a
recursive directory-files function (later found in
lisp/gnus/gnus-util.el).  I'd expect discoverability to worsen as
libraries continue to move out of grep's reach in the core to GNU
ELPA, Marmalade, Github, etc., which in turn would result in greater
duplication of effort as well as proliferation of similar but subtly
incompatible implementations of general-purpose functions.

Users' interest in a richer set of elisp building blocks is evident
from the existence and popularity of libraries such as Magnar Sveen's
dash[0] ("a modern list library for emacs") and s[1] ("the long lost
Emacs string manipulation library").  IMO it would be a worthwhile
goal to move toward shipping canonical implementations of these
building blocks with Emacs.  What do others think?

Josh

[0] https://github.com/magnars/dash.el
[1] https://github.com/magnars/s.el

Re: About `<prefix>--' (was Re: filenotify.el)

[Xue Fuqiao]

On Fri, Jun 28, 2013 at 7:19 AM, Josh <josh@foxtail.org> wrote:
> It would be great if more of these generally useful functions were
> "promoted" from the libraries where they originated into the core of
> Emacs, stripped of package prefixes and properly documented in the
> Emacs or Elisp info manuals in order to make them more discoverable
> for use elsewhere.  Doubtless many wheels have been reinvented after
> failed searches for functionality that already ships with Emacs.

> Users' interest in a richer set of elisp building blocks is evident
> from the existence and popularity of libraries such as Magnar Sveen's
> dash[0] ("a modern list library for emacs") and s[1] ("the long lost
> Emacs string manipulation library").  IMO it would be a worthwhile
> goal to move toward shipping canonical implementations of these
> building blocks with Emacs.  What do others think?

Agreed.  XOR is also an example.  Some generally useful features have
been added to core Emacs Lisp recently (e.g., generalized variables),
and more ones are expected, I think.

--
Best regards, Xue Fuqiao.
http://www.gnu.org/software/emacs/

Re: About `<prefix>--' (was Re: filenotify.el)

[Stefan Monnier]

> Agreed.  XOR is also an example.  Some generally useful features have
> been added to core Emacs Lisp recently (e.g., generalized variables),

Notice that these are language features, whereas powerset is in
a different category of "occasionally useful library function".

We sadly don't have a good place to put such generic functions.
Maybe we could create a new "set" library of set functions
(union, intersection, difference, powerset, membership, negation, ...).


        Stefan

Re: About `<prefix>--' (was Re: filenotify.el)

[Xue Fuqiao]

On Sat, Jun 29, 2013 at 6:54 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> Agreed.  XOR is also an example.  Some generally useful features have
>> been added to core Emacs Lisp recently (e.g., generalized variables),
>
> Notice that these are language features, whereas powerset is in
> a different category of "occasionally useful library function".

Yes, if you subdivide.  I organized them by one category: generally
useful features (which can be used by many other libraries).

> We sadly don't have a good place to put such generic functions.
> Maybe we could create a new "set" library of set functions
> (union, intersection, difference, powerset, membership, negation, ...).

And some other suggestions (I did not think too much):

* A list library (to provide something subr.el and cl-lib missed)
* A string manipulation library
* A math library (maybe Calc is enough?)
* A hash library (implements a common interface to different secure hash
  algorithms.)
* A time access and conversion library
* A logging facility library
* The long lost concurrency and FFI libraries
* An I18N library


[OT]
And some not-so-general suggestions:
* Interface to the DBM database
* Interface to the SQLite database (like this one:
http://www.emacswiki.org/emacs/sqlite.el)
* A CSV file reading and writing library (maybe csv-mode from GNU ELPA
  is a good choice)
* A JSON encoder and decoder
* Access to the POSIX locale database and functionality.

--
Best regards, Xue Fuqiao.
http://www.gnu.org/software/emacs/

Re: About `<prefix>--' (was Re: filenotify.el)

[Stefan Monnier]

> * A list library (to provide something subr.el and cl-lib missed)

We could have a list library (with "list-" prefix).  I'm not sure if
it'll be worth the trouble now that we have cl-lib (before, with cl.el
most of its list functions were out of reach, because cl.el could not
be used at run-time, but this problem doesn't plague cl-lib).

> * A string manipulation library
> * A math library (maybe Calc is enough?)
> * A hash library (implements a common interface to different secure hash
>   algorithms.)
> * A time access and conversion library
> * A logging facility library
> * The long lost concurrency and FFI libraries
> * An I18N library

All sounds fine.  For some of them, we already have a file for them
(e.g. time-date.el).

The main problem is for functions which would most naturally end up in
subr.el: since subr.el is preloaded, we'd rather put there only
functions which are very likely to be used.


        Stefan

Re: About `<prefix>--' (was Re: filenotify.el)

[Richard Stallman]

        [ To any NSA and FBI agents reading my email: please consider
        [ whether defending the US Constitution against all enemies,
        [ foreign or domestic, requires you to follow Snowden's example.

    It would be great if more of these generally useful functions were
    "promoted" from the libraries where they originated into the core of
    Emacs, stripped of package prefixes and properly documented in the
    Emacs or Elisp info manuals in order to make them more discoverable
    for use elsewhere.

The downside of this is that it would require documenting all these
functions in the Emacs Lisp Reference Manual, which is already
unwieldy.  It might be better for people to reinvent these as needed.
Or put them in a separate category with the prefix `aux-' and document
them in a different manual.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call.

Re: About `<prefix>--' (was Re: filenotify.el)

[Josh]

On Fri, Jun 28, 2013 at 2:59 PM, Richard Stallman <rms@gnu.org> wrote:
> The downside of this is that it would require documenting all these
> functions in the Emacs Lisp Reference Manual, which is already
> unwieldy.

What does "unwieldy" mean in the context of an easily searchable and
navigable hypertext resource like the elisp manual?

> It might be better for people to reinvent these as needed.
> Or put them in a separate category with the prefix `aux-' and document
> them in a different manual.

People are already reinventing such functions in large numbers today,
but I believe this is frequently not "as needed" but because we
wrongly believe it doesn't exist yet after failing to find evidence
to the contrary in the documentation and/or source code where it
would logically appear if it did.  It seems to me that prefixing some
set of functions (which ones?) with "aux-" and documenting them in a
separate manual could only exacerbate this problem.

filenotify.el (2)

[Michael Albinus]

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

Hi,

Enclosed is the updated version of the patch. I have incorporated the
comments, and I have also modified inotify.c and gfilenotify.c in order
to use `file-notify-error'. This shall also be changed in w32notify.c;
since I cannot test the build on Windows I haven't done it yet.

I have also started to write test/automated/file-notify-tests.el, but
this is far from being presentable yet.

Best regards, Michael.


[-- Attachment #2: filenotify.diff --]
[-- Type: text/x-diff, Size: 33775 bytes --]

=== modified file 'lisp/ChangeLog'
*** lisp/ChangeLog	2013-06-27 09:26:54 +0000
--- lisp/ChangeLog	2013-06-27 12:03:14 +0000
***************
*** 1,3 ****
--- 1,15 ----
+ 2013-06-27  Michael Albinus  <michael.albinus@gmx.de>
+ 
+ 	* filenotify.el: New package.
+ 
+ 	* autorevert.el (top): Require filenotify.el.
+ 	(auto-revert-notify-enabled): Remove.  Use `file-notify-support'
+ 	instead.
+ 	(auto-revert-notify-rm-watch, auto-revert-notify-add-watch)
+ 	(auto-revert-notify-handler): Use `file-notify-*' functions.
+ 
+ 	* subr.el (file-notify-handle-event): Move function to filenotify.el.
+ 
  2013-06-27  Dmitry Gutov  <dgutov@yandex.ru>
  
  	* emacs-lisp/package-x.el (package-upload-buffer-internal): Adapt
***************
*** 153,164 ****
  
  2013-06-25  Rüdiger Sonderfeld  <ruediger@c-plusplus.de>
  
! 	* lisp/textmodes/bibtex.el (bibtex-generate-url-list): Add support
  	for DOI URLs.
  
  2013-06-25  Rüdiger Sonderfeld  <ruediger@c-plusplus.de>
  
! 	* lisp/textmodes/bibtex.el (bibtex-mode, bibtex-set-dialect):
  	Update imenu-support when dialect changes.
  
  2013-06-25  Leo Liu  <sdl.web@gmail.com>
--- 165,176 ----
  
  2013-06-25  Rüdiger Sonderfeld  <ruediger@c-plusplus.de>
  
! 	* textmodes/bibtex.el (bibtex-generate-url-list): Add support
  	for DOI URLs.
  
  2013-06-25  Rüdiger Sonderfeld  <ruediger@c-plusplus.de>
  
! 	* textmodes/bibtex.el (bibtex-mode, bibtex-set-dialect):
  	Update imenu-support when dialect changes.
  
  2013-06-25  Leo Liu  <sdl.web@gmail.com>

=== modified file 'lisp/autorevert.el'
*** lisp/autorevert.el	2013-06-05 19:57:10 +0000
--- lisp/autorevert.el	2013-06-27 09:00:44 +0000
***************
*** 103,108 ****
--- 103,109 ----
  
  (eval-when-compile (require 'cl-lib))
  (require 'timer)
+ (require 'filenotify)
  
  ;; Custom Group:
  ;;
***************
*** 270,290 ****
    :type 'boolean
    :version "24.4")
  
! (defconst auto-revert-notify-enabled
!   (or (featurep 'gfilenotify) (featurep 'inotify) (featurep 'w32notify))
!   "Non-nil when Emacs has been compiled with file notification support.")
! 
! (defcustom auto-revert-use-notify auto-revert-notify-enabled
    "If non-nil Auto Revert Mode uses file notification functions.
  This requires Emacs being compiled with file notification
! support (see `auto-revert-notify-enabled').  You should set this
! variable through Custom."
    :group 'auto-revert
    :type 'boolean
    :set (lambda (variable value)
! 	 (set-default variable (and auto-revert-notify-enabled value))
  	 (unless (symbol-value variable)
! 	   (when auto-revert-notify-enabled
  	     (dolist (buf (buffer-list))
  	       (with-current-buffer buf
  		 (when (symbol-value 'auto-revert-notify-watch-descriptor)
--- 271,287 ----
    :type 'boolean
    :version "24.4")
  
! (defcustom auto-revert-use-notify (and file-notify-support t)
    "If non-nil Auto Revert Mode uses file notification functions.
  This requires Emacs being compiled with file notification
! support (see `file-notify-support').  You should set this variable
! through Custom."
    :group 'auto-revert
    :type 'boolean
    :set (lambda (variable value)
! 	 (set-default variable (and file-notify-support value))
  	 (unless (symbol-value variable)
! 	   (when file-notify-support
  	     (dolist (buf (buffer-list))
  	       (with-current-buffer buf
  		 (when (symbol-value 'auto-revert-notify-watch-descriptor)
***************
*** 502,513 ****
  	     (puthash key value auto-revert-notify-watch-descriptor-hash-list)
  	   (remhash key auto-revert-notify-watch-descriptor-hash-list)
  	   (ignore-errors
! 	     (funcall
! 	      (cond
! 	       ((fboundp 'gfile-rm-watch) 'gfile-rm-watch)
! 	       ((fboundp 'inotify-rm-watch) 'inotify-rm-watch)
! 	       ((fboundp 'w32notify-rm-watch) 'w32notify-rm-watch))
! 	      auto-revert-notify-watch-descriptor)))))
       auto-revert-notify-watch-descriptor-hash-list)
      (remove-hook 'kill-buffer-hook 'auto-revert-notify-rm-watch))
    (setq auto-revert-notify-watch-descriptor nil
--- 499,505 ----
  	     (puthash key value auto-revert-notify-watch-descriptor-hash-list)
  	   (remhash key auto-revert-notify-watch-descriptor-hash-list)
  	   (ignore-errors
! 	     (file-notify-rm-watch auto-revert-notify-watch-descriptor)))))
       auto-revert-notify-watch-descriptor-hash-list)
      (remove-hook 'kill-buffer-hook 'auto-revert-notify-rm-watch))
    (setq auto-revert-notify-watch-descriptor nil
***************
*** 522,621 ****
  
    (when (and buffer-file-name auto-revert-use-notify
  	     (not auto-revert-notify-watch-descriptor))
!     (let ((func
! 	   (cond
! 	    ((fboundp 'gfile-add-watch) 'gfile-add-watch)
! 	    ((fboundp 'inotify-add-watch) 'inotify-add-watch)
! 	    ((fboundp 'w32notify-add-watch) 'w32notify-add-watch)))
! 	  (aspect
! 	   (cond
! 	    ((fboundp 'gfile-add-watch) '(watch-mounts))
! 	    ;; `attrib' is needed for file modification time.
! 	    ((fboundp 'inotify-add-watch) '(attrib create modify moved-to))
! 	    ((fboundp 'w32notify-add-watch) '(size last-write-time))))
! 	  (file (if (or (fboundp 'gfile-add-watch) (fboundp 'inotify-add-watch))
! 		    (directory-file-name (expand-file-name default-directory))
! 		  (buffer-file-name))))
!       (setq auto-revert-notify-watch-descriptor
! 	    (ignore-errors
! 	      (funcall func file aspect 'auto-revert-notify-handler)))
!       (if auto-revert-notify-watch-descriptor
! 	  (progn
! 	    (puthash
! 	     auto-revert-notify-watch-descriptor
! 	     (cons (current-buffer)
! 		   (gethash auto-revert-notify-watch-descriptor
! 			    auto-revert-notify-watch-descriptor-hash-list))
! 	     auto-revert-notify-watch-descriptor-hash-list)
! 	    (add-hook (make-local-variable 'kill-buffer-hook)
! 		      'auto-revert-notify-rm-watch))
! 	;; Fallback to file checks.
! 	(set (make-local-variable 'auto-revert-use-notify) nil)))))
! 
! (defun auto-revert-notify-event-p (event)
!   "Check that event is a file notification event."
!   (and (listp event)
!        (cond ((featurep 'gfilenotify)
! 	      (and (>= (length event) 3) (stringp (nth 2 event))))
! 	     ((featurep 'inotify)
! 	      (= (length event) 4))
! 	     ((featurep 'w32notify)
! 	      (and (= (length event) 3) (stringp (nth 2 event)))))))
! 
! (defun auto-revert-notify-event-descriptor (event)
!   "Return watch descriptor of file notification event, or nil."
!   (and (auto-revert-notify-event-p event) (car event)))
! 
! (defun auto-revert-notify-event-action (event)
!   "Return action of file notification event, or nil."
!   (and (auto-revert-notify-event-p event) (nth 1 event)))
! 
! (defun auto-revert-notify-event-file-name (event)
!   "Return file name of file notification event, or nil."
!   (and (auto-revert-notify-event-p event)
!        (cond ((featurep 'gfilenotify) (nth 2 event))
! 	     ((featurep 'inotify) (nth 3 event))
! 	     ((featurep 'w32notify) (nth 2 event)))))
  
  (defun auto-revert-notify-handler (event)
    "Handle an EVENT returned from file notification."
!   (when (auto-revert-notify-event-p event)
!     (let* ((descriptor (auto-revert-notify-event-descriptor event))
! 	   (action (auto-revert-notify-event-action event))
! 	   (file (auto-revert-notify-event-file-name event))
  	   (buffers (gethash descriptor
  			     auto-revert-notify-watch-descriptor-hash-list)))
!       (ignore-errors
! 	;; Check, that event is meant for us.
! 	;; TODO: Filter events which stop watching, like `move' or `removed'.
! 	(cl-assert descriptor)
! 	(cond
! 	 ((featurep 'gfilenotify)
! 	  (cl-assert (memq action '(attribute-changed changed created deleted
!                                     ;; FIXME: I keep getting this action, so I
!                                     ;; added it here, but I have no idea what
!                                     ;; I'm doing.  --Stef
!                                     changes-done-hint))
!                      t))
! 	 ((featurep 'inotify)
! 	  (cl-assert (or (memq 'attrib action)
! 			 (memq 'create action)
! 			 (memq 'modify action)
! 			 (memq 'moved-to action))))
! 	 ((featurep 'w32notify) (cl-assert (eq 'modified action))))
! 	;; Since we watch a directory, a file name must be returned.
! 	(cl-assert (stringp file))
! 	(dolist (buffer buffers)
! 	  (when (buffer-live-p buffer)
! 	    (with-current-buffer buffer
! 	      (when (and (stringp buffer-file-name)
! 			 (string-equal
! 			  (file-name-nondirectory file)
! 			  (file-name-nondirectory buffer-file-name)))
! 		;; Mark buffer modified.
! 		(setq auto-revert-notify-modified-p t)
! 		;; No need to check other buffers.
! 		(cl-return)))))))))
  
  (defun auto-revert-active-p ()
    "Check if auto-revert is active (in current buffer or globally)."
--- 514,571 ----
  
    (when (and buffer-file-name auto-revert-use-notify
  	     (not auto-revert-notify-watch-descriptor))
!     (setq auto-revert-notify-watch-descriptor
! 	  (ignore-errors
! 	    (file-notify-add-watch
! 	     (directory-file-name (expand-file-name default-directory))
! 	     '(change attribute-change) 'auto-revert-notify-handler)))
!     (if auto-revert-notify-watch-descriptor
! 	(progn
! 	  (puthash
! 	   auto-revert-notify-watch-descriptor
! 	   (cons (current-buffer)
! 		 (gethash auto-revert-notify-watch-descriptor
! 			  auto-revert-notify-watch-descriptor-hash-list))
! 	   auto-revert-notify-watch-descriptor-hash-list)
! 	  (add-hook (make-local-variable 'kill-buffer-hook)
! 		    'auto-revert-notify-rm-watch))
!       ;; Fallback to file checks.
!       (set (make-local-variable 'auto-revert-use-notify) nil))))
  
  (defun auto-revert-notify-handler (event)
    "Handle an EVENT returned from file notification."
!   (ignore-errors
!     (let* ((descriptor (car event))
! 	   (action (nth 1 event))
! 	   (file (nth 2 event))
! 	   (file1 (nth 3 event)) ;; Target of `renamed'.
  	   (buffers (gethash descriptor
  			     auto-revert-notify-watch-descriptor-hash-list)))
!       ;; Check, that event is meant for us.
!       (cl-assert descriptor)
!       ;; We do not handle `deleted', because nothing has to be refreshed.
!       (cl-assert (memq action '(attribute-changed changed created renamed)) t)
!       ;; Since we watch a directory, a file name must be returned.
!       (cl-assert (stringp file))
!       (when (eq action 'renamed) (cl-assert (stringp file1)))
!       ;; Loop over all buffers, in order to find the intended one.
!       (dolist (buffer buffers)
! 	(when (buffer-live-p buffer)
! 	  (with-current-buffer buffer
! 	    (when (and (stringp buffer-file-name)
! 		       (or
! 			(and (memq action '(attribute-changed changed created))
! 			     (string-equal
! 			      (file-name-nondirectory file)
! 			      (file-name-nondirectory buffer-file-name)))
! 			(and (eq action 'renamed)
! 			     (string-equal
! 			      (file-name-nondirectory file1)
! 			      (file-name-nondirectory buffer-file-name)))))
! 	      ;; Mark buffer modified.
! 	      (setq auto-revert-notify-modified-p t)
! 	      ;; No need to check other buffers.
! 	      (cl-return))))))))
  
  (defun auto-revert-active-p ()
    "Check if auto-revert is active (in current buffer or globally)."

=== added file 'lisp/filenotify.el'
*** lisp/filenotify.el	1970-01-01 00:00:00 +0000
--- lisp/filenotify.el	2013-06-27 06:49:24 +0000
***************
*** 0 ****
--- 1,292 ----
+ ;;; filenotify.el --- watch files for changes on disk
+ 
+ ;; Copyright (C) 2013 Free Software Foundation, Inc.
+ 
+ ;; Author: Michael Albinus <michael.albinus@gmx.de>
+ 
+ ;; This file is part of GNU Emacs.
+ 
+ ;; GNU Emacs is free software: you can redistribute it and/or modify
+ ;; it under the terms of the GNU General Public License as published by
+ ;; the Free Software Foundation, either version 3 of the License, or
+ ;; (at your option) any later version.
+ 
+ ;; GNU Emacs is distributed in the hope that it will be useful,
+ ;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+ ;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ ;; GNU General Public License for more details.
+ 
+ ;; You should have received a copy of the GNU General Public License
+ ;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+ 
+ ;;; Commentary
+ 
+ ;; This package is an abstraction layer from the different low-level
+ ;; file notification packages `gfilenotify', `inotify' and
+ ;; `w32notify'.
+ 
+ ;;; Code:
+ 
+ ;;;###autoload
+ (defconst file-notify-support
+   (cond
+    ((featurep 'gfilenotify) 'gfilenotify)
+    ((featurep 'inotify) 'inotify)
+    ((featurep 'w32notify) 'w32notify))
+   "Non-nil when Emacs has been compiled with file notification support.
+ The value is the name of the low-level file notification package
+ to be used for local file systems.  Remote file notifications
+ could use another implementation.")
+ 
+ (defvar file-notify-descriptors (make-hash-table :test 'equal)
+   "Hash table for registered file notification descriptors.
+ A key in this hash table is the descriptor as returned from a
+ `gfilenotify', `inotify' or `w32notify'.  The value in the hash
+ table is the cons cell (FILE . CALLBACK).")
+ 
+ ;; This function is used by `gfilenotify', `inotify' and `w32notify' events.
+ ;;;###autoload
+ (defun file-notify-handle-event (event)
+   "Handle file system monitoring event.
+ If EVENT is a filewatch event, call its callback.
+ Otherwise, signal a `file-notify-error'."
+   (interactive "e")
+   (if (and (eq (car event) 'file-notify)
+ 	   (>= (length event) 3))
+       (funcall (nth 2 event) (nth 1 event))
+     (signal 'file-notify-error
+ 	    (cons "Not a valid file-notify event" event))))
+ 
+ (defvar file-notify--pending-event nil
+   "Pending file notification event for a future `renamed' action.
+ This is a list (DESCRIPTOR ACTION COOKIE FILE), or nil.  ACTION
+ is either `moved-from' or `renamed-from'.")
+ 
+ (defun file-notify--event-file-name (event)
+   "Return file name of file notification event, or nil."
+   (cond ((eq file-notify-support 'gfilenotify) (nth 2 event))
+ 	((eq file-notify-support 'inotify) (nth 3 event))
+ 	((eq file-notify-support 'w32notify) (nth 2 event))))
+ 
+ ;; Only `gfilenotify' could return two file names.
+ (defun file-notify--event-file1-name (event)
+   "Return second file name of file notification event, or nil.
+ This is available in case a file has been moved."
+   (and (eq file-notify-support 'gfilenotify) (nth 3 event)))
+ 
+ ;; Cookies are offered by `inotify' only.
+ (defun file-notify--event-cookie (event)
+   "Return cookie of file notification event, or nil.
+ This is available in case a file has been moved."
+   (and (eq file-notify-support 'inotify) (nth 2 event)))
+ 
+ ;; The callback function used to map between specific flags of the
+ ;; respective file notifications, and the ones we return.
+ (defun file-notify-callback (event)
+   "Handle an EVENT returned from file notification.
+ EVENT is the same one as in `file-notify-handle-event' except the
+ car of that event, which is the symbol `file-notify'."
+   (let ((descriptor (car event))
+ 	(actions (nth 1 event))
+ 	(file (file-notify--event-file-name event))
+ 	file1 cookie callback tmp)
+     ;; Check, that event is meant for us.
+     (unless (setq callback (cdr (gethash descriptor file-notify-descriptors)))
+       (signal 'file-notify-error (list "Not a valid descriptor" descriptor)))
+ 
+     ;; Make actions a list.
+     (unless (consp actions) (setq actions (cons actions nil)))
+ 
+     ;; Loop over actions.  In fact, more than one action happens only
+     ;; for `inotify'.
+     (dolist (action actions)
+ 
+       ;; Send pending event, if it doesn't match.
+       (when (and file-notify--pending-event
+ 		 ;; The cookie doesn't match.
+ 		 (not (eq (nth 2 file-notify--pending-event)
+ 			  (file-notify--event-cookie event)))
+ 		 (or
+ 		  ;; inotify.
+ 		  (and (eq (nth 1 file-notify--pending-event) 'moved-from)
+ 		       (not (eq action 'moved-to)))
+ 		  ;; w32notify.
+ 		  (and (eq (nth 1 file-notify--pending-event) 'renamed-from)
+ 		       (not (eq action 'renamed-to)))))
+ 	(funcall callback
+ 		 (list (nth 0 file-notify--pending-event)
+ 		       'deleted (nth 3 file-notify--pending-event)))
+ 	(setq file-notify--pending-event nil))
+ 
+       ;; Map action.  We ignore all events which cannot be mapped.
+       (setq action
+ 	    (cond
+ 	     ;; gfilenotify.
+ 	     ((memq action '(attribute-changed changed created deleted)) action)
+ 	     ((eq action 'moved)
+ 	      (setq file1 (file-notify--event-file1-name event))
+ 	      'renamed)
+ 
+ 	     ;; inotify.
+ 	     ((eq action 'attrib) 'attribute-changed)
+ 	     ((eq action 'create) 'created)
+ 	     ((eq action 'modify) 'changed)
+ 	     ((memq action '(delete 'delete-self move-self)) 'deleted)
+ 	     ;; Make the event pending.
+ 	     ((eq action 'moved-from)
+ 	      (setq file-notify--pending-event
+ 		    (list descriptor action
+ 			  (file-notify--event-cookie event) file))
+ 	      nil)
+ 	     ;; Look for pending event.
+ 	     ((eq action 'moved-to)
+ 	      (if (null file-notify--pending-event)
+ 		  'created
+ 		(setq file1 file
+ 		      file (nth 3 file-notify--pending-event)
+ 		      file-notify--pending-event nil)
+ 		'renamed))
+ 
+ 	     ;; w32notify.
+ 	     ((eq action 'added) 'created)
+ 	     ((eq action 'modified) 'changed)
+ 	     ((eq action 'removed) 'deleted)
+ 	     ;; Make the event pending.
+ 	     ((eq 'renamed-from action)
+ 	      (setq file-notify--pending-event
+ 		    (list descriptor action nil file))
+ 	      nil)
+ 	     ;; Look for pending event.
+ 	     ((eq 'renamed-to action)
+ 	      (if (null file-notify--pending-event)
+ 		  'created
+ 		(setq file1 file
+ 		      file (nth 3 file-notify--pending-event)
+ 		      file-notify--pending-event nil)
+ 		'renamed))))
+ 
+       ;; Apply callback.
+       (when action
+ 	(if file1
+ 	    (funcall callback (list descriptor action file file1))
+ 	  (funcall callback (list descriptor action file)))))))
+ 
+ (defun file-notify-add-watch (file flags callback)
+   "Add a watch for filesystem events pertaining to FILE.
+ This arranges for filesystem events pertaining to FILE to be reported
+ to Emacs.  Use `file-notify-rm-watch' to cancel the watch.
+ 
+ The returned value is a descriptor for the added watch.  If the
+ file cannot be watched for some reason, this function signals a
+ `file-error' error.
+ 
+ FLAGS is a list of conditions to set what will be watched for.  It can
+ include the following symbols:
+ 
+   `change'           -- watch for file changes
+   `attribute-change' -- watch for file attributes changes, like
+                         permissions or modification time
+ 
+ If FILE is a directory, 'change' watches for file creation or
+ deletion in that directory.
+ 
+ When any event happens, Emacs will call the CALLBACK function passing
+ it a single argument EVENT, which is of the form
+ 
+   (DESCRIPTOR ACTION FILE [FILE1])
+ 
+ DESCRIPTOR is the same object as the one returned by this function.
+ ACTION is the description of the event.  It could be any one of the
+ following:
+ 
+   `created'           -- FILE was created
+   `deleted'           -- FILE was deleted
+   `changed'           -- FILE has changed
+   `renamed'           -- FILE has been renamed to FILE1
+   `attribute-changed' -- a FILE attribute was changed
+ 
+ FILE is the name of the file whose event is being reported."
+   ;; Check arguments.
+   (unless (stringp file)
+     (signal 'wrong-type-argument (list file)))
+   (setq file (expand-file-name file))
+   (unless (and (consp flags)
+ 	       (null (delq 'change (delq 'attribute-change (copy-tree flags)))))
+     (signal 'wrong-type-argument (list flags)))
+   (unless (functionp callback)
+     (signal 'wrong-type-argument (list callback)))
+ 
+   (let ((handler (find-file-name-handler file 'file-notify-add-watch))
+ 	func fl desc)
+ 
+     ;; Check, whether this has been registered already.
+     (maphash
+      (lambda (key value)
+        (when (equal (cons file callback) value) (setq desc key)))
+      file-notify-descriptors)
+ 
+     (unless desc
+       (if handler
+ 	  ;; A file name handler could exist even if there is no local
+ 	  ;; file notification support.
+ 	  (setq desc
+ 		(funcall handler 'file-notify-add-watch file flags callback))
+ 
+ 	;; Check, whether Emacs has been compiled with file
+ 	;; notification support.
+ 	(unless file-notify-support
+ 	  (signal 'file-notify-error
+ 		  '("No file notification package available")))
+ 
+ 	;; Determine local function to be called.
+ 	(setq func (cond
+ 		    ((eq file-notify-support 'gfilenotify) 'gfile-add-watch)
+ 		    ((eq file-notify-support 'inotify) 'inotify-add-watch)
+ 		    ((eq file-notify-support 'w32notify) 'w32notify-add-watch)))
+ 
+ 	;; Determine respective flags.
+ 	(if (eq file-notify-support 'gfilenotify)
+ 	    (setq fl '(watch-mounts send-moved))
+ 	  (when (memq 'change flags)
+ 	    (setq
+ 	     fl
+ 	     (cond
+ 	      ((eq file-notify-support 'inotify) '(create modify move delete))
+ 	      ((eq file-notify-support 'w32notify) '(size last-write-time)))))
+ 	  (when (memq 'attribute-change flags)
+ 	    (add-to-list
+ 	     'fl
+ 	     (cond
+ 	      ((eq file-notify-support 'inotify) 'attrib)
+ 	      ((eq file-notify-support 'w32notify) 'attributes)))))
+ 
+ 	;; Call low-level function.
+ 	(setq desc (funcall func file fl 'file-notify-callback))))
+ 
+     ;; Return descriptor.
+     (puthash desc (cons file callback) file-notify-descriptors)
+     desc))
+ 
+ (defun file-notify-rm-watch (descriptor)
+   "Remove an existing watch specified by its DESCRIPTOR.
+ DESCRIPTOR should be an object returned by `file-notify-add-watch'."
+   (let ((file (car (gethash descriptor file-notify-descriptors)))
+ 	handler)
+ 
+     (when (stringp file)
+       (setq handler (find-file-name-handler file 'file-notify-rm-watch))
+       (if handler
+ 	  (funcall handler 'file-notify-rm-watch descriptor)
+ 	(funcall
+ 	 (cond
+ 	  ((eq file-notify-support 'gfilenotify) 'gfile-rm-watch)
+ 	  ((eq file-notify-support 'inotify) 'inotify-rm-watch)
+ 	  ((eq file-notify-support 'w32notify) 'w32notify-rm-watch))
+ 	 descriptor)))
+ 
+     (remhash descriptor file-notify-descriptors)))
+ 
+ ;; The end:
+ (provide 'filenotify)
+ 
+ ;;; filenotify.el ends here

=== modified file 'lisp/subr.el'
*** lisp/subr.el	2013-06-20 14:15:42 +0000
--- lisp/subr.el	2013-06-24 07:49:32 +0000
***************
*** 4494,4513 ****
         nil ,@(cdr (cdr spec)))))
  
  \f
- ;;;; Support for watching filesystem events.
- 
- (defun file-notify-handle-event (event)
-   "Handle file system monitoring event.
- If EVENT is a filewatch event, call its callback.
- Otherwise, signal a `filewatch-error'."
-   (interactive "e")
-   (if (and (eq (car event) 'file-notify)
- 	   (>= (length event) 3))
-       (funcall (nth 2 event) (nth 1 event))
-     (signal 'filewatch-error
- 	    (cons "Not a valid file-notify event" event))))
- 
- \f
  ;;;; Comparing version strings.
  
  (defconst version-separator "."
--- 4494,4499 ----

=== modified file 'src/ChangeLog'
*** src/ChangeLog	2013-06-24 00:31:31 +0000
--- src/ChangeLog	2013-06-27 11:56:19 +0000
***************
*** 1,3 ****
--- 1,13 ----
+ 2013-06-27  Michael Albinus  <michael.albinus@gmx.de>
+ 
+ 	* fileio.c (Qfile_notify_error): New error symbol.
+ 
+ 	* gfilenotify.c (Fgfile_add_watch, Fgfile_rm_watch):
+ 	* inotify.c (inotify_callback, symbol_to_inotifymask)
+ 	(Finotify_add_watch, Finotify_rm_watch): Use it.
+ 
+ 	* lisp.h (Qfile_notify_error): Declare.
+ 
  2013-06-23  Paul Eggert  <eggert@cs.ucla.edu>
  
  	A more-conservative workaround for Cygwin SIGCHLD issues (Bug#14569).

=== modified file 'src/fileio.c'
*** src/fileio.c	2013-06-18 07:42:37 +0000
--- src/fileio.c	2013-06-26 07:56:12 +0000
***************
*** 148,154 ****
  #ifdef WINDOWSNT
  #endif
  
! Lisp_Object Qfile_error;
  static Lisp_Object Qfile_already_exists, Qfile_date_error;
  static Lisp_Object Qexcl;
  Lisp_Object Qfile_name_history;
--- 148,154 ----
  #ifdef WINDOWSNT
  #endif
  
! Lisp_Object Qfile_error, Qfile_notify_error;
  static Lisp_Object Qfile_already_exists, Qfile_date_error;
  static Lisp_Object Qexcl;
  Lisp_Object Qfile_name_history;
***************
*** 5887,5892 ****
--- 5887,5893 ----
    DEFSYM (Qfile_error, "file-error");
    DEFSYM (Qfile_already_exists, "file-already-exists");
    DEFSYM (Qfile_date_error, "file-date-error");
+   DEFSYM (Qfile_notify_error, "file-notify-error");
    DEFSYM (Qexcl, "excl");
  
    DEFVAR_LISP ("file-name-coding-system", Vfile_name_coding_system,
***************
*** 5925,5930 ****
--- 5926,5936 ----
    Fput (Qfile_date_error, Qerror_message,
  	build_pure_c_string ("Cannot set file date"));
  
+   Fput (Qfile_notify_error, Qerror_conditions,
+ 	Fpurecopy (list3 (Qfile_notify_error, Qfile_error, Qerror)));
+   Fput (Qfile_notify_error, Qerror_message,
+ 	build_pure_c_string ("File notification error"));
+ 
    DEFVAR_LISP ("file-name-handler-alist", Vfile_name_handler_alist,
  	       doc: /* Alist of elements (REGEXP . HANDLER) for file names handled specially.
  If a file name matches REGEXP, all I/O on that file is done by calling

=== modified file 'src/gfilenotify.c'
*** src/gfilenotify.c	2013-06-06 07:04:35 +0000
--- src/gfilenotify.c	2013-06-27 11:47:42 +0000
***************
*** 132,146 ****
  to Emacs.  Use `gfile-rm-watch' to cancel the watch.
  
  Value is a descriptor for the added watch.  If the file cannot be
! watched for some reason, this function signals a `file-error' error.
  
  FLAGS is a list of conditions to set what will be watched for.  It can
  include the following symbols:
  
    'watch-mounts' -- watch for mount events
    'send-moved'   -- pair 'deleted' and 'created' events caused by file
!                     renames (moves) and send a single 'event-moved'
!                     event instead
  
  When any event happens, Emacs will call the CALLBACK function passing
  it a single argument EVENT, which is of the form
--- 132,145 ----
  to Emacs.  Use `gfile-rm-watch' to cancel the watch.
  
  Value is a descriptor for the added watch.  If the file cannot be
! watched for some reason, this function signals a `file-notify-error' error.
  
  FLAGS is a list of conditions to set what will be watched for.  It can
  include the following symbols:
  
    'watch-mounts' -- watch for mount events
    'send-moved'   -- pair 'deleted' and 'created' events caused by file
!                     renames and send a single 'renamed' event instead
  
  When any event happens, Emacs will call the CALLBACK function passing
  it a single argument EVENT, which is of the form
***************
*** 193,199 ****
    /* Enable watch.  */
    monitor = g_file_monitor (gfile, gflags, NULL, NULL);
    if (! monitor)
!     xsignal2 (Qfile_error, build_string ("Cannot watch file"), file);
  
    /* On all known glib platforms, converting MONITOR directly to a
       Lisp_Object value results is a Lisp integer, which is safe.  This
--- 192,198 ----
    /* Enable watch.  */
    monitor = g_file_monitor (gfile, gflags, NULL, NULL);
    if (! monitor)
!     xsignal2 (Qfile_notify_error, build_string ("Cannot watch file"), file);
  
    /* On all known glib platforms, converting MONITOR directly to a
       Lisp_Object value results is a Lisp integer, which is safe.  This
***************
*** 202,208 ****
    if (! INTEGERP (watch_descriptor))
      {
        g_object_unref (monitor);
!       xsignal2 (Qfile_error, build_string ("Unsupported file watcher"), file);
      }
  
    g_signal_connect (monitor, "changed",
--- 201,208 ----
    if (! INTEGERP (watch_descriptor))
      {
        g_object_unref (monitor);
!       xsignal2 (Qfile_notify_error, build_string ("Unsupported file watcher"),
! 		file);
      }
  
    g_signal_connect (monitor, "changed",
***************
*** 226,239 ****
    Lisp_Object watch_object = assq_no_quit (watch_descriptor, watch_list);
  
    if (! CONSP (watch_object))
!     xsignal2 (Qfile_error, build_string ("Not a watch descriptor"),
  	      watch_descriptor);
  
    eassert (INTEGERP (watch_descriptor));
    int_monitor = XLI (watch_descriptor);
    monitor = (GFileMonitor *) int_monitor;
    if (!g_file_monitor_cancel (monitor))
!     xsignal2 (Qfile_error, build_string ("Could not rm watch"),
  	      watch_descriptor);
  
    /* Remove watch descriptor from watch list. */
--- 226,239 ----
    Lisp_Object watch_object = assq_no_quit (watch_descriptor, watch_list);
  
    if (! CONSP (watch_object))
!     xsignal2 (Qfile_notify_error, build_string ("Not a watch descriptor"),
  	      watch_descriptor);
  
    eassert (INTEGERP (watch_descriptor));
    int_monitor = XLI (watch_descriptor);
    monitor = (GFileMonitor *) int_monitor;
    if (!g_file_monitor_cancel (monitor))
!     xsignal2 (Qfile_notify_error, build_string ("Could not rm watch"),
  	      watch_descriptor);
  
    /* Remove watch descriptor from watch list. */

=== modified file 'src/inotify.c'
*** src/inotify.c	2013-01-02 16:30:50 +0000
--- src/inotify.c	2013-06-27 08:42:14 +0000
***************
*** 158,172 ****
  
    to_read = 0;
    if (ioctl (fd, FIONREAD, &to_read) == -1)
!     report_file_error ("Error while trying to retrieve file system events",
!                        Qnil);
    buffer = xmalloc (to_read);
    n = read (fd, buffer, to_read);
    if (n < 0)
      {
        xfree (buffer);
!       report_file_error ("Error while trying to read file system events",
!                          Qnil);
      }
  
    EVENT_INIT (event);
--- 158,174 ----
  
    to_read = 0;
    if (ioctl (fd, FIONREAD, &to_read) == -1)
!     xsignal1
!       (Qfile_notify_error,
!        build_string ("Error while trying to retrieve file system events"));
    buffer = xmalloc (to_read);
    n = read (fd, buffer, to_read);
    if (n < 0)
      {
        xfree (buffer);
!       xsignal1
!       (Qfile_notify_error,
!        build_string ("Error while trying to read file system events"));
      }
  
    EVENT_INIT (event);
***************
*** 242,248 ****
    else if (EQ (symb, Qt) || EQ (symb, Qall_events))
      return IN_ALL_EVENTS;
    else
!     signal_error ("Unknown aspect", symb);
  }
  
  static uint32_t
--- 244,250 ----
    else if (EQ (symb, Qt) || EQ (symb, Qall_events))
      return IN_ALL_EVENTS;
    else
!       xsignal2 (Qfile_notify_error, build_string ("Unknown aspect"), symb);
  }
  
  static uint32_t
***************
*** 335,342 ****
        if (inotifyfd == -1)
          {
            inotifyfd = uninitialized;
!           report_file_error ("File watching feature (inotify) is not available",
!                              Qnil);
          }
        watch_list = Qnil;
        add_read_fd (inotifyfd, &inotify_callback, NULL);
--- 337,345 ----
        if (inotifyfd == -1)
          {
            inotifyfd = uninitialized;
! 	  xsignal1
! 	    (Qfile_notify_error,
! 	     build_string ("File watching feature (inotify) is not available"));
          }
        watch_list = Qnil;
        add_read_fd (inotifyfd, &inotify_callback, NULL);
***************
*** 346,352 ****
    encoded_file_name = ENCODE_FILE (file_name);
    watchdesc = inotify_add_watch (inotifyfd, SSDATA (encoded_file_name), mask);
    if (watchdesc == -1)
!     report_file_error ("Could not add watch for file", Fcons (file_name, Qnil));
  
    watch_descriptor = make_watch_descriptor (watchdesc);
  
--- 349,356 ----
    encoded_file_name = ENCODE_FILE (file_name);
    watchdesc = inotify_add_watch (inotifyfd, SSDATA (encoded_file_name), mask);
    if (watchdesc == -1)
!     xsignal2 (Qfile_notify_error,
! 	      build_string ("Could not add watch for file"), file_name);
  
    watch_descriptor = make_watch_descriptor (watchdesc);
  
***************
*** 375,382 ****
    int wd = XINT (watch_descriptor);
  
    if (inotify_rm_watch (inotifyfd, wd) == -1)
!     report_file_error ("Could not rm watch", Fcons (watch_descriptor,
!                                                     Qnil));
  
    /* Remove watch descriptor from watch list. */
    watch_object = Fassoc (watch_descriptor, watch_list);
--- 379,386 ----
    int wd = XINT (watch_descriptor);
  
    if (inotify_rm_watch (inotifyfd, wd) == -1)
!     xsignal2 (Qfile_notify_error,
! 	      build_string ("Could not rm watch"), watch_descriptor);
  
    /* Remove watch descriptor from watch list. */
    watch_object = Fassoc (watch_descriptor, watch_list);

=== modified file 'src/lisp.h'
*** src/lisp.h	2013-06-21 20:11:44 +0000
--- src/lisp.h	2013-06-26 07:41:17 +0000
***************
*** 3799,3804 ****
--- 3799,3805 ----
  /* Defined in fileio.c.  */
  
  extern Lisp_Object Qfile_error;
+ extern Lisp_Object Qfile_notify_error;
  extern Lisp_Object Qfile_exists_p;
  extern Lisp_Object Qfile_directory_p;
  extern Lisp_Object Qinsert_file_contents;

Re: filenotify.el (2)

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi,

> Enclosed is the updated version of the patch.

Installed in the trunk. Also file name handlers for Tramp, and a first
version of a related ert test file.

Best regards, Michael.

Re: filenotify.el

[Rüdiger Sonderfeld]

Hello,

I've written a new "Auto Update" module for magit using  the new API: 
https://github.com/magit/magit/pull/721

Best regards,
Rüdiger

On Tuesday 25 June 2013 14:02:38 Michael Albinus wrote:
> Hi,
> 
> I have written filenotify.el, which is intended as upper layer for
> gfilenotify.c, inotify.c and w32notify.c. This is a simplified
> interface, which offers just file change or file attribute change
> notifications. I believe this is sufficient for the use-cases in Emacs;
> if more fine granular notifications are needed, one could still use one
> of the low-level packages.
> 
> The package is appended, together with needed changes in subr.el and
> autorevert.el. I've tested it with gfilenotify and inotify; the tests
> for w32notify I cannot perform as usual. There are still some changes
> needed, but basic functionality shall be available.
> 
> Beside further tests and bug fixing, I plan to write ert tests as well
> as a Tramp file name handler. Documentation is also lacking. But these
> steps could be applied later, once there is an agreement about the
> interface.
> 
> Comments?
> 
> Best regards, Michael.

Re: filenotify.el

[Michael Albinus]

Rüdiger Sonderfeld <ruediger@c-plusplus.de> writes:

> Hello,

Hi Rüdiger,

> I've written a new "Auto Update" module for magit using  the new API: 
> https://github.com/magit/magit/pull/721

Nice :-)

One remark: the following code is not sufficient:

(defun magit-filenotify-start ()
  "Start watching for changes to the source tree using filenotify.
This can only be called from a magit status buffer."
  (unless file-notify-support
    (error "Support for `file-notify' required."))
  ...

There could be support even if `file-notify-support' is nil. This
variable reflects only, whether a respective library has been linked to
Emacs, working for local files. `file-notify' could run for remote
files, even if it is not supported locally.

Maybe we shall spend a new function

(defun file-notify-supported-p (file)
  "Returns non-nil if filesystem pertaining to FILE could be watched."
  ...

Comments?

> Best regards,
> Rüdiger

Best regards, Michael.

Re: filenotify.el

[Rüdiger Sonderfeld]

Hi Michael,

On Wednesday 17 July 2013 16:54:34 Michael Albinus wrote:
> Maybe we shall spend a new function
> 
> (defun file-notify-supported-p (file)
>   "Returns non-nil if filesystem pertaining to FILE could be watched."
>   ...
> 
> Comments?

Sounds like a good idea.

Regards
Rüdiger

Re: filenotify.el

[Michael Albinus]

Rüdiger Sonderfeld <ruediger@c-plusplus.de> writes:

> Hi Michael,

Hi Rüdiger,

>> Maybe we shall spend a new function
>> 
>> (defun file-notify-supported-p (file)
>>   "Returns non-nil if filesystem pertaining to FILE could be watched."
>>   ...
>> 
>> Comments?
>
> Sounds like a good idea.

Committed to trunk as r113447.

> Regards
> Rüdiger

Best regards, Michael.

Re: filenotify.el

[Davis Herring]

> (defun magit-filenotify-start ()
>   "Start watching for changes to the source tree using filenotify.
> This can only be called from a magit status buffer."
>   (unless file-notify-support
>     (error "Support for `file-notify' required."))
>   ...
> 
> There could be support even if `file-notify-support' is nil. This
> variable reflects only, whether a respective library has been linked to
> Emacs, working for local files. `file-notify' could run for remote
> files, even if it is not supported locally.
> 
> Maybe we shall spend a new function
> 
> (defun file-notify-supported-p (file)
>   "Returns non-nil if filesystem pertaining to FILE could be watched."
>   ...
> 
> Comments?

Why not just let the error from the underlying notification request
appear?  (This is the usual bit about "don't call stat(2) before
open(2); just deal with failures from the latter".)

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

Re: filenotify.el

[Michael Albinus]

Davis Herring <herring@lanl.gov> writes:

> Why not just let the error from the underlying notification request
> appear?  (This is the usual bit about "don't call stat(2) before
> open(2); just deal with failures from the latter".)

That would be possible, yes. But I hope to improve
file-notify-supported-p, that it could also return a correct answer in
case the underlying library does not support the filesystem in question.

Example: mounted directories. (file-notify-supported-p "/mnt/dir") will
return t when either gfilenotify or inotify are linked to Emacs. And
file-notify-add-watch won't fail in either case.

But likely, in the inotify case you won't see events, because it does
not support mounted directories. In the gfilenotify case it shall work,
because glib's implementation uses polling, when other mechanisms fail.

file-notify-supported-p is not such clever yet. But users of
filenotify.el shall apply this function already, for the future when
file-notify-supported-p improves.

> Davis

Best regards, Michael.

Re: filenotify.el

[Stefan Monnier]

>> Why not just let the error from the underlying notification request
>> appear?  (This is the usual bit about "don't call stat(2) before
>> open(2); just deal with failures from the latter".)
> That would be possible, yes. But I hope to improve
> file-notify-supported-p, that it could also return a correct answer in
> case the underlying library does not support the filesystem in question.

But if you can do that for file-notify-supported-p, presumably you can
also arrange for file-notify-add-watch to fail in those cases.
I agree with Davis that in general it's better to run
file-notify-add-watch and handle any error it might signal, than to
use a function like file-notify-supported-p.

The only case where file-notify-supported-p makes sense is when you want
to know if file-notify-add-watch would work before you know whether you
want to call file-notify-add-watch.  Can't think of too many situations
where this would be the case.


        Stefan

Re: filenotify.el

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> The only case where file-notify-supported-p makes sense is when you want
> to know if file-notify-add-watch would work before you know whether you
> want to call file-notify-add-watch.  Can't think of too many situations
> where this would be the case.

It is more simple to implement for some Tramp file name handlers. For
example, in the smb case I have mapped file-notify-supported-p to
ignore. But of course I could implement it differently, let
file-notify-add-watch return with a file-notify-error in that case.

I have no heavy preference for or against file-notify-supported-p.

>         Stefan

Best regards, Michael.

Re: filenotify.el

[Stefan Monnier]

> It is more simple to implement for some Tramp file name handlers. For
> example, in the smb case I have mapped file-notify-supported-p to
> ignore. But of course I could implement it differently, let
> file-notify-add-watch return with a file-notify-error in that case.

I think it's not an either/or situation: it's important for
file-notify-add-watch to return an error if it is not supported for that
file, regardless of the existence of file-notify-supported-p.


        Stefan

Re: filenotify.el

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> It is more simple to implement for some Tramp file name handlers. For
>> example, in the smb case I have mapped file-notify-supported-p to
>> ignore. But of course I could implement it differently, let
>> file-notify-add-watch return with a file-notify-error in that case.
>
> I think it's not an either/or situation: it's important for
> file-notify-add-watch to return an error if it is not supported for that
> file, regardless of the existence of file-notify-supported-p.

I haven't meant to let behave file-notify-add-watch to return an error
just because of this. Of course, it shall return file-notify-error
anyway (marker to myself: we still must change it in w32notify.c). This
shall be checked in corresponding ert tests (next marker to myself).

Btw, file-notify-supported-p would ease to skip ert tests, if file
notification is not available for the local or remote case.

>         Stefan

Best regards, Michael.

Re: filenotify.el

[Stefan Monnier]

> Btw, file-notify-supported-p would ease to skip ert tests, if file
> notification is not available for the local or remote case.

I don't see why.  You can just as well say that "failure for lack of
support" is an acceptable result.


        Stefan

Re: filenotify.el

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Btw, file-notify-supported-p would ease to skip ert tests, if file
>> notification is not available for the local or remote case.
>
> I don't see why.  You can just as well say that "failure for lack of
> support" is an acceptable result.

Of course. I have just said, it would be more convenient to write such
tests with file-notify-supported-p, that's all.

But maybe we should not design wrt to test cases :-)
As said, I don't care too much whether we keep file-notify-supported-p.
Your decision ...

>         Stefan

Best regards, Michael.

Re: filenotify.el

[Stefan Monnier]

> As said, I don't care too much whether we keep file-notify-supported-p.
> Your decision ...

I think we should try to live without it.  If/when it proves
necessary/useful, we can re-add it.


        Stefan

Re: filenotify.el

[Davis Herring]

>> As said, I don't care too much whether we keep file-notify-supported-p.
>> Your decision ...
> 
> I think we should try to live without it.  If/when it proves
> necessary/useful, we can re-add it.

An addendum to my own agitation: if there's a way to prevent gfilenotify
from polling (or if we add Lisp polling to filenotify.el in the future),
`file-notify-add-watch' could benefit from a don't-poll flag, either as
an additional argument or as a new accepted symbol in FLAGS.  That way
we can (probably) also avoid having a
`file-notify-supported-without-polling-p'.

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

Re: filenotify.el

[Michael Albinus]

Davis Herring <herring@lanl.gov> writes:

> An addendum to my own agitation: if there's a way to prevent gfilenotify
> from polling (or if we add Lisp polling to filenotify.el in the future),
> `file-notify-add-watch' could benefit from a don't-poll flag, either as
> an additional argument or as a new accepted symbol in FLAGS.

I'll check. There is the macro G_IS_POLL_FILE_MONITOR in gio. Maybe it
can be used to implement such a flag in gfilenotify.c.

However, I would rather implement it as a defcustom than as additional
argument to file-notify-add-watch. Then you could decide yourself,
whether you want polling, or not.

> Davis

Best regards, Michael.

Re: filenotify.el

[Michael Albinus]

Davis Herring <herring@lanl.gov> writes:

> An addendum to my own agitation: if there's a way to prevent gfilenotify
> from polling (or if we add Lisp polling to filenotify.el in the future),
> `file-notify-add-watch' could benefit from a don't-poll flag, either as
> an additional argument or as a new accepted symbol in FLAGS.  That way
> we can (probably) also avoid having a
> `file-notify-supported-without-polling-p'.

gfilenotify.c uses g_file_monitor from glib/gio. This does polling for
mounted objects only in case the object to be watched is a regular
file. For directories, polling is not supported (and the function
returns an error in case there is no native file notification mechanism
for that directory).

filenotify.el tries always to watch directories. No polling therefore,
at least for the time being. Whether we want introduce explicit polling
depends on the request from users. For now, I have no plans.

It might be different when the file/directory to be watched is located
on a remote host. Then it depends on the implementation of the
respective file handler, whether it uses polling, or not.

> Davis

Best regards, Michael.

Re: filenotify.el

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I think we should try to live without it.  If/when it proves
> necessary/useful, we can re-add it.

Done. Also fixed in test/automated/file-notify-tests.el.

>         Stefan

Best regards, Michael.

Re: filenotify.el

[Glenn Morris]

Michael Albinus wrote:

> Btw, file-notify-supported-p would ease to skip ert tests, if file
> notification is not available for the local or remote case.

The ert tests are all broken now that the variable file-notify-support
has been removed (sounds like maybe you know this already?).

Re: filenotify.el

[Michael Albinus]

Glenn Morris <rgm@gnu.org> writes:

> The ert tests are all broken now that the variable file-notify-support
> has been removed (sounds like maybe you know this already?).

Yes. I have fixed this in my local repository; but I was waiting for the
final decision.

Too much hassle just now due to other things (@work); I guess there's
time to work on it tomorrow morning. Sorry for the inconvenience.

Best regards, Michael.