replacing process sentinels and filters with hooks

replacing process sentinels and filters with hooks

[Christopher Monsanto]

Hi all,

I have been working to improve the shell and comint modes. More on
that later; the reason I am writing is to complain about--and suggest
backwards compatible changes for--the process APIs.

There are three problems.

1) Process filters and sentinels do not fit in with the rest of the
Emacs API. In every other API that I can think of that involves a user
callback, the hooks API is used.
2) A process sentinel must parse text to figure out how the process
changed. It'd be easier to use if we returned structured data that
could be taken apart by pcase.
3) It's hard for more than one interested party to interact with a process.

My proposed change is to add a new function, (process-hook-list
<proc>), which returns a list of hooks. Each hook takes one argument,
which has one of the following forms:

'(input <string>)
'(signal <signalcode> <core dump flag>)
'(exit <exitcode>)

The various cases can be handled easily with the pcase primitive.

For backwards compatibility, a process' hook list comes with one hook
already added. This hook acts as a proxy to the old filter/sentinel
interface; if the user changes the filter or sentinel, the default
hook is modified appropriately. It is important to realize filters and
sentinels through the hook interface in case the user wants to fake
signals through (run-hooks (process-hook-list ..))

Any and all feedback is welcome. Did I miss anything? Do these changes
seem reasonable? If so, I would like to have sole responsibility for
implementation--I am trying to learn the Emacs C source and this seems
like a great way to do so. My copyright assignment papers are already
in transit.

Thanks for your time,

Christopher Monsanto
chris@monsan.to  --  http://monsan.to/

Re: replacing process sentinels and filters with hooks

[Paul Eggert]

On 10/01/2012 06:14 AM, Christopher Monsanto wrote:
> Each hook takes one argument,
> which has one of the following forms:
> 
> '(input <string>)
> '(signal <signalcode> <core dump flag>)
> '(exit <exitcode>)

Overall this sounds like a nice cleanup.

One relatively-minor comment about those forms.
I take it that the (signal ...) pattern is when
waitpid reports WIFSIGNALED and the (exit ...)
pattern for WIFEXITED.  But what happens for
WIFSTOPPED and WIFCONTINUED?

Re: replacing process sentinels and filters with hooks

[Christopher Monsanto]

I don't have a good answer for that, as I have no idea (yet) how the
existing sentinel interface handles the signals. My plan was to just
translate the old sentinel messages as follows:

"finished\n" --> (exit 0)
"exited abnormally with code exitcode\n" --> (exit nonzero-exitcode)
"name-of-signal\n" --> (signal code-for-name-of-signal nil)
"name-of-signal (core dumped)\n" -> (signal code-for-name-of-signal t)


Christopher Monsanto
chris@monsan.to  --  http://monsan.to/

Re: replacing process sentinels and filters with hooks

[Stefan Monnier]

> 1) Process filters and sentinels do not fit in with the rest of the
> Emacs API. In every other API that I can think of that involves a user
> callback, the hooks API is used.

Hooks are used normally for un-planned interaction between two
unrelated packages.  OTOH process sentinels and filters are typically
tightly linked to their process, the package that creates the process
being usually the same that provides the sentinels and filters.

> 2) A process sentinel must parse text to figure out how the process
> changed.

Yes, that's ugly (and it only affects sentinels).

> 3) It's hard for more than one interested party to interact with a process.

Indeed, tho it's rarely needed.  Concrete examples would be useful for
this discussion.

> My proposed change is to add a new function, (process-hook-list
> <proc>), which returns a list of hooks.  Each hook takes one argument,
> which has one of the following forms:

A hook is a symbol/variable which can hold a list of functions.
I'd rather call it `process-callbacks'.

> '(input <string>)

Just <string> would work as well.

So your proposal can be decomposed in 3 parts:
- merge sentinels and filters.
- provide a standard way to combine sentinels and filters, which is to
  run them in sequence (for sentinels, it might be a fine default, but
  for filters, it might be more useful for each filter to be able to
  affect the <string> passed to the next filter).
- change the particular representation of state passed to process sentinels.

I agree the third part is a good change (tho not terribly important).
I'm not sure the first is useful since in my experience sentinels and
filters do different things anyway.

As for the second, maybe a better path is to try and provide generic
`add-function' and `remove-function' that can operate on any
generalized-variable that holds a function.  So you could do

  (add-function (process-filter proc) #'toto)

which would set proc's filter to a new function that runs the old
function (if any) and runs toto as well.   And then `remove-function'
would reset the sentinel to its original value.

> signals through (run-hooks (process-hook-list ..))

run-hooks takes symbols as arguments.


        Stefan

Re: replacing process sentinels and filters with hooks

[Christopher Monsanto]

My mistake, I don't think I posted this response to the list. Sorry
for double message, Stefan.

On Mon, Oct 1, 2012 at 11:34 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> 1) Process filters and sentinels do not fit in with the rest of the
>> Emacs API. In every other API that I can think of that involves a user
>> callback, the hooks API is used.
>
> Hooks are used normally for un-planned interaction between two
> unrelated packages.  OTOH process sentinels and filters are typically
> tightly linked to their process, the package that creates the process
> being usually the same that provides the sentinels and filters.
> 3) It's hard for more than one interested party to interact with a process.
>
> Indeed, tho it's rarely needed.  Concrete examples would be useful for
> this discussion.

This is not my experience working with the shell and comint packages.
For instance, comint has its own hook, comint-input-filter-functions,
for doing exactly what this proposal suggests (sans sentinels). This
hook for example lets shell check for cd/pushd when doing directory
tracking.

shell is lucky that comint doesn't use a sentinel, because it
sometimes steals the process sentinel to save the shell history on
exit. If comint adds its own sentinel (which isn't too unreasonable,
considering that comint creates the process) then shell's code is
broken; the two will fight with each other. And shell does *assume*
comint doesn't use a sentinel, because it doesn't setup any code to
call "the next sentinel".

Perhaps that's not such a big deal, because both modes are in the
Emacs tree, and we can just fix that. What *is* a big deal is that it
would break anyone else's code  to add such a sentinel to comint.
There is no way either to add a "comint-sentinel-functions" because
there is no way comint can guarantee that its sentinel is running (due
to assumptions like the one shell mode made).

In other words, it's really nasty that having or not having a
sentinel/filter is apart of the API of your program.

One could say "it's a best practice to always remember the last
sentinel". And I would respond, "well, if it is a best practice, then
why doesn't the API use hooks?"

For another example, I wrote a package similar to popwin
(https://github.com/m2ym/popwin-el) that uses the shell. On a key
press, I can open a temporary shell in a small popped window, which is
pretty handy. It is also pretty handy that when I exit the shell (by
typing exit in bash) the shell window is destroyed. But I can't know
if I have exited unless I have a sentinel. And I have to share the
sentinel with shell-write-history-on-exit.

> Just <string> would work as well.

I disagree. Ideally I'd like to use pcase to check the three
alternatives; to do this, I either have to ensure that the (input ...)
case is last, or I have to use a more complicated pattern involving
(pred stringp). And with the proposed API, you do have to check--you
cannot simply just assume that your argument is a string. So let's
make it as easy as possible to check.

> So your proposal can be decomposed in 3 parts:
> ...
> - provide a standard way to combine sentinels and filters, which is to
>   run them in sequence (for sentinels, it might be a fine default, but
>   for filters, it might be more useful for each filter to be able to
>   affect the <string> passed to the next filter).

I'll add that this can be useful for any hook that takes arguments. I
think it is a bad idea to consider process callback functions special.
If it is useful behavior to modify callback arguments, then let us be
consistent and give that ability to any type of callback, not just
processes.

> I agree the third part is a good change (tho not terribly important).

I'm interested in gradually improving the existing Emacs API. To me,
it is very important. And since I have volunteered to do all of the
work (and probably won't volunteer on things that others find
important, as there are many important things to be done) it won't eat
up any of your time, unless you decide to review a patch. In which
case I would be very grateful of your time! :)

> I'm not sure the first is useful since in my experience sentinels and
> filters do different things anyway.

I personally think it is cleaner to have them together, but if others
disagree there is probably a reason for it :) This is not an essential
part of my proposal.

> As for the second, maybe a better path is to try and provide generic
> `add-function' and `remove-function' that can operate on any
> generalized-variable that holds a function.  So you could do
>
>   (add-function (process-filter proc) #'toto)
>
> which would set proc's filter to a new function that runs the old
> function (if any) and runs toto as well.   And then `remove-function'
> would reset the sentinel to its original value.

Why have a different way of doing things? One of the reasons I like my
proposal is that it unifies concepts and adds more consistency to the
API. The slogan: "If you want have a callback, use the hook API."
Since there is no way for filters and sentinels to guarantee order
(just like hooks) I think the best route is to just add the ability
for a hook to modify the arguments that are passed to the next hook.
Although I am not too convinced that this is important,
comint-input-filter-functions doesn't do this.

> run-hooks takes symbols as arguments.

It was just an example, I am a busy person and I think I got the gist
of what I was saying across. But I'll add (off-topic) that there seems
to be no reason for run-hooks to take a symbol as an argument. The C
code just immediately dereferences it.

Re: replacing process sentinels and filters with hooks

[Christopher Monsanto]

Hi Stefan,

I think we are mostly on the same page now. When I say that "I don't
want to treat process callbacks specially" and "do we need a different
way of doing things?", I'm referring to the fact that add-function and
add-hook do similar things and it would be ideal (in my opinion) to
not have two general purpose APIs that (almost?) do the same thing.
I'm not necessarily calling for merging implementations, but if we end
up having both I think it would be nice to 1) point out the similarity
in the documentation and 2) have clear recommendations on which API to
use given the circumstances. For instance, would add-hook be (very
lightly-)deprecated in favor of add-function, since add-function is
more general than hooks? Or do we recommend the use of add-function
only when the callbacks take arguments? Or recommend add-function only
when we expect callbacks to modify arguments?

> How do you expect add-hook to work.  E.g. give an example would be an appropriate add-hook call to add a function on your process-callback?

Hooks could use an interface similar to defadvice to change their
arguments: (hook-set-arg <index> <value>) would modify the argument
being passed to the next hook. Assuming for brevity that the argument
to a process callback is simply a string representing process-input:

  (defun my-callback (input)
    (message "Saw input: %s" input)
    (hook-set-arg 0 (concat "Seen: " input)))

  ;; process-callbacks defaults to current buffer's process
  (add-hook (process-callbacks) 'my-callback)

Yes, there seems to be a bit of ugliness in this case with
process-callbacks needing to return a symbol. In this case, I just
think of a symbol as acting as a reference type in ML, so it doesn't
bother me too much I guess.

My personal preference would be for hook-set-arg, because we wouldn't
have two different APIs for callbacks that take arguments, one being
more powerful than the other. I can imagine having different APIs
might be a problem: consider the case where a library author doesn't
know about add-function so they use the hook API. A user complains
that they need the ability to modify the arguments passed to the next
hook, and now the library author has to switch APIs (breaks backward
compat) or support two different APIs (breaks maintainer's spirit).

But you probably know better than I on this one, so use your judgment :)

> Note that comint-input-filter-functions is a hook on process-send-string rather than on the process-filter, whereas comint-preoutput-filter-functions (which is the hook on the process-filter) does allow modifying the string passed to the next function on the hook.

Argh, I was thinking of comint-preoutput-filter-functions in my head.
Sorry for the confusion!

I've copied the rest of your message below, as it wasn't sent to the
list. My apologies if it was meant to be private.

On Mon, Oct 1, 2012 at 10:25 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> shell is lucky that comint doesn't use a sentinel, because it
>> sometimes steals the process sentinel to save the shell history on
>> exit.
>
> Agreed, which is why I suggest `add/remove-function' so that packages
> can share something like process-sentinel without having to fight over
> it, or even having to plan ahead.
>
>> Perhaps that's not such a big deal, because both modes are in the
>> Emacs tree, and we can just fix that.
>
> No, it is important.
>
>>> - provide a standard way to combine sentinels and filters, which is to
>>> run them in sequence (for sentinels, it might be a fine default, but
>>> for filters, it might be more useful for each filter to be able to
>>> affect the <string> passed to the next filter).
>> I'll add that this can be useful for any hook that takes arguments.
>
> Agreed.
>
>> I think it is a bad idea to consider process callback functions special.
>
> I do not want to treat process callback functions specially.
>
>> If it is useful behavior to modify callback arguments, then let us be
>> consistent and give that ability to any type of callback, not just
>> processes.
>
> That's what I want out of add/remove-function.  Maybe it can even be
> used on (symbol-function f) as a lightweight alternative to defadvice.
>
>>> I'm not sure the first is useful since in my experience sentinels and
>>> filters do different things anyway.
>> I personally think it is cleaner to have them together, but if others
>> disagree there is probably a reason for it :) This is not an essential
>> part of my proposal.
>
> I'm not dead-set against merging them, but unless it simplifies
> implementation, I don't see much benefit, so I'm leaning towards keeping
> the separation.
>
>>> (add-function (process-filter proc) #'toto)
>> Why have a different way of doing things?
>
> Different from what?
>
>> API. The slogan: "If you want have a callback, use the hook API."
>
> Oh, so you want to use add-hook instead of a new add-function?
> The problem with that is that add-hook is written specifically for
> symbols/variables and I don't see how to make it work for
> process-callbacks, unless process-callbacks returns a symbol (itself
> holding a list of function), which I think is not very elegant.
>
>> Since there is no way for filters and sentinels to guarantee order
>> (just like hooks) I think the best route is to just add the ability
>> for a hook to modify the arguments that are passed to the next hook.
>
> In add/remove/run-hook, this is determined by the kind of hook, i.e. how
> the hook is run (run-hooks vs run-hook-with-args vs
> run-hook-with-args-until-success with with-wrapper-hook).
>
> Indeed for a new add-function, the choice would be made in add-function
> instead.
>
>> Although I am not too convinced that this is important,
>> comint-input-filter-functions doesn't do this.
>
> But comint-input-filter-functions is comint's hook for
> process-send-string, whereas comint-preoutput-filter-functions (the hook
> corresponding to the process-filter) does have the ability to modify the
> string passed to the next function.
>
>>> run-hooks takes symbols as arguments.
>> It was just an example, I am a busy person and I think I got the gist
>> of what I was saying across. But I'll add (off-topic) that there seems
>> to be no reason for run-hooks to take a symbol as an argument.  The C
>> code just immediately dereferences it.
>
> It's slightly more complex than that (it dereferences it potentially
> twice: once for the buffer-local value and once for the global value),
> but the real problem comes in add/remove-hook where receiving a list is
> not good enough since that list can be nil in which case there's no
> updatable field on which to add/remove the function (hence the need for
> an additional indirection, such as making function-callbacks return
> a symbol).
>
>
>         Stefan

Re: replacing process sentinels and filters with hooks

[Stefan Monnier]

> way of doing things?", I'm referring to the fact that add-function and
> add-hook do similar things and it would be ideal (in my opinion) to
> not have two general purpose APIs that (almost?) do the same thing.

I agree that having both add-hook and add-function is not ideal.
But I think the ideal solution doesn't exist because of constraints
imposed by backward compatibility.

> Hooks could use an interface similar to defadvice to change their
> arguments: (hook-set-arg <index> <value>) would modify the argument
> being passed to the next hook.

Hmm... that sounds ugly.  Because we also want to let a function prevent
the subsequent functions from being run.  IOW we want to be able to have
the equivalent of an `around' advice.

I think something along the lines of what with-wrapper-hook does is
a good model (i.e. (add-function (process-filter proc) fun) would take
a `fun' which expects 3 args: the proc, the string, and a
"keep-running-the-hook" function which expects 2 args (the proc and the
string)).
Then we can also define

  (defmacro add-function-before (place fun)
    `(add-function ,place (lambda (fallback-fun &rest args)
                            (apply ,fun args)
                            (apply fallback-fun args))))

Which is more like `add-hook'; and

  (defmacro add-function-after (place fun)
    `(add-function ,place (lambda (fallback-fun &rest args)
                            (apply fallback-fun args)
                            (apply ,fun args))))

which is a bit like "add-hook with the append arg set".

> Yes, there seems to be a bit of ugliness in this case with
> process-callbacks needing to return a symbol.

Yes, that's one ugliness.  The other problem with it is that it means
that it doesn't solve the problem for other "objects holding a function"
such as symbol-function, or syntax-propertize-function.

> In this case, I just think of a symbol as acting as a reference type
> in ML, so it doesn't bother me too much I guess.

As a functional programmer, that's very much the way I think about
it, indeed.  But the advantage of add-function (assuming we can write
it) is that it can work for anything that holds a function.  E.g. we
wouldn't even need to change process-filter (I'd still consider
changing process-sentinel to clean up its string argument).
I.e. not only we can preserve backward compatibility, but we don't even
need to obsolete anything.

> I've copied the rest of your message below, as it wasn't sent to the
> list.  My apologies if it was meant to be private.

Thank you for that (I actually did send it to the list "by hand" via
`resend' after noticing, IIRC).


        Stefan

Re: replacing process sentinels and filters with hooks

[Stefan Monnier]

> I think something along the lines of what with-wrapper-hook does is
> a good model (i.e. (add-function (process-filter proc) fun) would take
> a `fun' which expects 3 args: the proc, the string, and a
> "keep-running-the-hook" function which expects 2 args (the proc and the
> string)).

This is now implemented on the Emacs trunk.  It's called `add-function' and
takes an additional argument, for convenience, which says whether to add the
function before, after, or around the existing function.

It's the basis of the new advice mechanism (whose main entry point
is `advice-add').


        Stefan

Re: replacing process sentinels and filters with hooks

[Stephen J. Turnbull]

Christopher Monsanto writes:

 > I think we are mostly on the same page now. When I say that "I don't
 > want to treat process callbacks specially" and "do we need a different
 > way of doing things?", I'm referring to the fact that add-function and
 > add-hook do similar things

Not really.  The reason why `add-hook' isn't defined as an alias for
`add-to-list' is that hook values are very varied: they can be a list
(including nil), a function, or void, and (independently of the above,
except for void I suppose) they can be global, local, or both.  (I
suspect that the global portion of a hook's value is actually the
shared tail of all the local parts, which would make manipulating hook
values a bit fussy.)

The reason why `add-function' won't be defined as an alias for
`add-to-list' is that it just plain wouldn't work at all (instant
wrong-type-argument, as the values it's intended to operate on are
functions, not lists).

In fact, `add-function' isn't much like `add-hook' at all.  It is far
more similar to what Python expresses with decorator syntax (hm...),
or to `defadvice' itself.

 > For instance, would add-hook be (very lightly-)deprecated in favor
 > of add-function, since add-function is more general than hooks?

No point in deprecation.  `add-function' is not more general, it's
different.  `add-hook' operates on lists, `add-function' on "places"
that hold functions.  Not to mention that `add-function' isn't really
defined yet.

I suppose you could change the APIs for sentinels and filters to be
lists, with basically the same rules as hooks.

 > Hooks could use an interface similar to defadvice to change their
 > arguments: (hook-set-arg <index> <value>) would modify the argument
 > being passed to the next hook.

This is a completely different story from `defadvice'.  Concretely,
why would you want to do that?  Most hook functions don't take
arguments at all.  The ones that do, I can't really see why you would
want to give an altered argument to a random function whose purpose
you don't know.  If you *do* know some function that "needs" edited
args, what's wrong with

(defun decorate (fun)
   `(lambda (&rest args) (apply ,fun (munge args))))

(maplist (lambda (l)
           (if (eq (car l) victim)
               (setcar l (decorate (car l)))))
         whatever-hook)

?

 > My personal preference would be for hook-set-arg, because we wouldn't
 > have two different APIs for callbacks that take arguments, one being
 > more powerful than the other. I can imagine having different APIs
 > might be a problem: consider the case where a library author doesn't
 > know about add-function so they use the hook API.

I think anybody who finds a need to know about something that can
modify other functions' arguments probably is capable of finding the
answer.  `hook-set-arg' is the kind of thing I constitutionally
disagree with, anyway.  Side effects should be avoided when possible.

 > A user complains that they need the ability to modify the arguments
 > passed to the next hook,

"Next hook" or "all remaining hooks"?  Suppose some other code
prepends a new function?  What do you think the user might propose to
do about that?

 > and now the library author has to switch APIs (breaks backward
 > compat) or support two different APIs (breaks maintainer's spirit).

This is inherently fragile, as implied above.  The user already can do
that, anyway:

;; Famous Last Words Department: Untested but should work.
(defun replacement-hook (&rest args)
  (apply 'run-hook-with-args 'user-hook args)
(setq user-hook original-hook)
(setq original-hook nil)
(add-hook 'original-hook #'replacement-hook)

For process sentinels and filters, I can see a relatively simple,
hooklike API.  Perhaps even the hook API, although processes have
their own locality, different from buffers.

I don't really see the need to alter arguments of a function (unless,
like defadvice, you're going to call it yourself, or the function
itself runs a hook to prepare its arguments).  I don't deny that such
needs might exist, but I think it makes sense to leave it up to the
users who find such needs until we have some idea of what kinds of
there are requirements (and how to satisfy them).