Re: "after" variable watchers

[Eli Zaretskii]

> From: martin rudalics <rudalics@gmx.at>
> Date: Mon, 17 May 2021 10:27:40 +0200
> 
> I'd like to install the attached patch which expands on the existing
> variable watching mechanism in the sense that the variable whose value
> gets changed has already the new value assigned to within the body of
> the watch function.  Such a feature is useful when the watch function
> calls already existing functions which act upon the variable's new value
> in a possibly deeply nested fashion.

But the watcher function already gets the new value, so why do we need
to be able to call it after the variable's value was changed?

> Consider the following example: A user wants to set `right-margin-width'
> of a buffer and I want to decide whether that margin really fits.  The
> mechanism whether it fits would be nested in a common function that
> decides whether any decoration (fringe, scroll bar, margin) fits into
> any window showing that buffer based on the minimum sizes of that window
> and the sizes of the remaining decorations.  Passing a "this is the
> requested new value of the right margin" setting to such a function is
> awkward at the very least.

I don't think I understand the use case, and so cannot follow your
"awkward" argument.

In general, this is a debugging feature, while you seem to be
describing a use case where the watcher will be a constant part of an
implementation of some feature, is that right?

> Objections, suggestions, comments?

First, I'd like to better understand the need for this.

If indeed this could be useful enough, I think I'd prefer 2 separate
list of watchers and 2 bits to indicate which one of the 2 possible
lists of watchers should be scanned to find the watcher function.  The
way you implemented it will slow down Emacs in one of its most
sensitive places: where we bind symbols to values, because we now need
to scan the same list twice, and call Fget for each symbol to see if
it's a "before" or an "after" watcher.

A few minor comments follow.

> -@defun add-variable-watcher symbol watch-function
> +@defun add-variable-watcher symbol watch-function after

This doesn't say AFTER is an optional argument.

> -@defun remove-variable-watcher symbol watch-function
> +@defun remove-variable-watcher symbol watch-function after

Neither does this.

>  This function removes @var{watch-function} from @var{symbol}'s list of
> -watchers.
> +watchers.  The third argument @var{after} should match the same argument
> +used by the previous @code{add-variable-watcher} call.

This should be reworded to be more oriented towards the Lisp
programmer who wants to use this function.  What the text does now is
describe the implementation, from which the reader will have to glean
what that means in terms of which watchers will be removed.

> @@ -1659,55 +1671,86 @@ DEFUN ("add-variable-watcher", Fadd_variable_watcher, 
> Sadd_variable_watcher,
>  WHERE is a buffer if the buffer-local value of the variable is being
>  changed, nil otherwise.
> 
> +Third argument AFTER, if non-nil, means to call WATCH-FUNCTION after
> +SYMBOL has been set.  In that case, the second argument for
> +WATCH-FUNCTION will be the value of SYMBOL before it was set to the
> +current value.

This doesn't say what will be that old-value if the variable wasn't
bound (and neither does the manual text).

> +
>  All writes to aliases of SYMBOL will call WATCH-FUNCTION too.  */)
> -  (Lisp_Object symbol, Lisp_Object watch_function)
> +  (Lisp_Object symbol, Lisp_Object watch_function, Lisp_Object after)
>  {
> +  CHECK_SYMBOL (symbol);

I don't think I understand why you added CHECK_SYMBOL here.  There's
one such call right after that.

Thanks.