From: Michael Albinus <michael.albinus@gmx.de>
To: "Rüdiger Sonderfeld" <ruediger@c-plusplus.de>
Cc: emacs-devel@gnu.org
Subject: Re: filenotify.el
Date: Tue, 25 Jun 2013 21:00:43 +0200 [thread overview]
Message-ID: <87vc52yq04.fsf@gmx.de> (raw)
In-Reply-To: <1383316.4Bc292dOgY@descartes> ("Rüdiger Sonderfeld"'s message of "Tue, 25 Jun 2013 18:25 +0200")
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.
next prev parent reply other threads:[~2013-06-25 19:00 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-25 12:02 filenotify.el Michael Albinus
2013-06-25 16:25 ` filenotify.el Rüdiger Sonderfeld
2013-06-25 19:00 ` Michael Albinus [this message]
2013-06-25 19:20 ` filenotify.el Eli Zaretskii
2013-06-25 19:27 ` filenotify.el Michael Albinus
2013-06-25 20:14 ` filenotify.el Eli Zaretskii
2013-06-26 6:06 ` filenotify.el Michael Albinus
2013-06-26 0:11 ` filenotify.el Stefan Monnier
2013-06-26 6:21 ` filenotify.el Michael Albinus
2013-06-26 13:04 ` filenotify.el Stefan Monnier
2013-06-26 14:15 ` filenotify.el Michael Albinus
2013-06-26 14:28 ` filenotify.el Stefan Monnier
2013-06-26 14:37 ` filenotify.el Michael Albinus
2013-06-26 15:57 ` filenotify.el Eli Zaretskii
2013-06-26 19:59 ` filenotify.el Stefan Monnier
2013-06-26 20:28 ` filenotify.el Michael Albinus
2013-06-26 0:45 ` filenotify.el Rüdiger Sonderfeld
2013-06-26 6:40 ` filenotify.el Michael Albinus
2013-06-26 7:50 ` About `<prefix>--' (was Re: filenotify.el) Stephen Berman
2013-06-26 13:06 ` Stefan Monnier
2013-06-26 22:36 ` Stephen Berman
2013-06-27 1:39 ` Stefan Monnier
2013-06-27 23:19 ` Josh
2013-06-28 3:00 ` Xue Fuqiao
2013-06-28 22:54 ` Stefan Monnier
2013-06-28 23:50 ` Xue Fuqiao
2013-06-29 2:06 ` Stefan Monnier
2013-06-28 21:59 ` Richard Stallman
2013-06-30 18:50 ` Josh
2013-06-27 12:18 ` filenotify.el (2) Michael Albinus
2013-07-04 9:54 ` Michael Albinus
2013-07-17 13:52 ` filenotify.el Rüdiger Sonderfeld
2013-07-17 14:54 ` filenotify.el Michael Albinus
2013-07-17 16:21 ` filenotify.el Rüdiger Sonderfeld
2013-07-18 10:10 ` filenotify.el Michael Albinus
2013-07-22 18:17 ` filenotify.el Davis Herring
2013-07-22 18:28 ` filenotify.el Michael Albinus
2013-07-22 19:00 ` filenotify.el Stefan Monnier
2013-07-23 6:57 ` filenotify.el Michael Albinus
2013-07-23 13:08 ` filenotify.el Stefan Monnier
2013-07-23 13:34 ` filenotify.el Michael Albinus
2013-07-23 13:57 ` filenotify.el Stefan Monnier
2013-07-23 14:13 ` filenotify.el Michael Albinus
2013-07-23 14:23 ` filenotify.el Stefan Monnier
2013-07-23 15:31 ` filenotify.el Davis Herring
2013-07-24 9:14 ` filenotify.el Michael Albinus
2013-08-02 7:10 ` filenotify.el Michael Albinus
2013-07-24 14:01 ` filenotify.el Michael Albinus
2013-07-23 16:10 ` filenotify.el Glenn Morris
2013-07-23 16:58 ` filenotify.el Michael Albinus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87vc52yq04.fsf@gmx.de \
--to=michael.albinus@gmx.de \
--cc=emacs-devel@gnu.org \
--cc=ruediger@c-plusplus.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.