"after" variable watchers

"after" variable watchers

[martin rudalics]

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

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.

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.

Objections, suggestions, comments?

Thanks, martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: after-variable-watchers.diff --]
[-- Type: text/x-patch; name="after-variable-watchers.diff", Size: 19646 bytes --]

diff --git a/doc/lispref/variables.texi b/doc/lispref/variables.texi
index 36abc316cb..8a4ff26c13 100644
--- a/doc/lispref/variables.texi
+++ b/doc/lispref/variables.texi
@@ -842,7 +842,7 @@ Watching Variables
 The following functions may be used to manipulate and query the watch
 functions for a variable.

-@defun add-variable-watcher symbol watch-function
+@defun add-variable-watcher symbol watch-function after
 This function arranges for @var{watch-function} to be called whenever
 @var{symbol} is modified.  Modifications through aliases
 (@pxref{Variable Aliases}) will have the same effect.
@@ -858,16 +858,25 @@ Watching Variables
 @code{unlet}, @code{makunbound}, or @code{defvaralias}.  @var{where}
 is a buffer if the buffer-local value of the variable is being
 changed, @code{nil} otherwise.
+
+If the optional third argument @var{after} is non-@code{nil}, this means
+to call @var{watch-function} after changing the value of @var{symbol}.
+In that case the second argument passed to @var{watch-function} will be
+the value of @var{symbol} before it was set to the current value.
 @end defun

-@defun remove-variable-watcher symbol watch-function
+@defun remove-variable-watcher symbol watch-function after
 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.
 @end defun

-@defun get-variable-watchers symbol
+@defun get-variable-watchers symbol after
 This function returns the list of @var{symbol}'s active watcher
-functions.
+functions.  If the optional second argument @var{after} is @code{nil},
+this means to return watchers run before @var{symbol}'s value is set.
+If @var{after} is non-@code{nil}, this means to return watchers run
+after @var{symbol}'s value is set.
 @end defun

 @subsection Limitations
diff --git a/src/alloc.c b/src/alloc.c
index 76d8c7ddd1..a9a67462eb 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -7662,14 +7662,14 @@ syms_of_alloc (void)
        { .a4 = watch_gc_cons_threshold },
        4, 4, "watch_gc_cons_threshold", {0}, 0}};
   XSETSUBR (watcher, &Swatch_gc_cons_threshold.s);
-  Fadd_variable_watcher (Qgc_cons_threshold, watcher);
+  Fadd_variable_watcher (Qgc_cons_threshold, watcher, Qnil);

   static union Aligned_Lisp_Subr Swatch_gc_cons_percentage =
      {{{ PSEUDOVECTOR_FLAG | (PVEC_SUBR << PSEUDOVECTOR_AREA_BITS) },
        { .a4 = watch_gc_cons_percentage },
        4, 4, "watch_gc_cons_percentage", {0}, 0}};
   XSETSUBR (watcher, &Swatch_gc_cons_percentage.s);
-  Fadd_variable_watcher (Qgc_cons_percentage, watcher);
+  Fadd_variable_watcher (Qgc_cons_percentage, watcher, Qnil);
 }

 #ifdef HAVE_X_WINDOWS
diff --git a/src/buffer.c b/src/buffer.c
index df302db0e5..cc1f6414b8 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1045,10 +1045,9 @@ reset_buffer_local_variables (struct buffer *b, bool permanent_too)
           Lisp_Object prop = Fget (local_var, Qpermanent_local);
           Lisp_Object sym = local_var;

-          /* Watchers are run *before* modifying the var.  */
           if (XSYMBOL (local_var)->u.s.trapped_write == SYMBOL_TRAPPED_WRITE)
-            notify_variable_watchers (local_var, Qnil,
-                                      Qmakunbound, Fcurrent_buffer ());
+            notify_variable_watchers (local_var, Qnil, Qmakunbound,
+				      Fcurrent_buffer (), false);

           eassert (XSYMBOL (sym)->u.s.redirect == SYMBOL_LOCALIZED);
           /* Need not do anything if some other buffer's binding is
@@ -1076,9 +1075,9 @@ reset_buffer_local_variables (struct buffer *b, bool permanent_too)
                     for (newlist = Qnil; CONSP (list); list = XCDR (list))
                       {
                         Lisp_Object elt = XCAR (list);
-                        /* Preserve element ELT if it's t,
-                           if it is a function with a `permanent-local-hook' property,
-                           or if it's not a symbol.  */
+                        /* Preserve element ELT if it's t, if it is a
+                           function with a `permanent-local-hook'
+                           property, or if it's not a symbol.  */
                         if (! SYMBOLP (elt)
                             || EQ (elt, Qt)
                             || !NILP (Fget (elt, Qpermanent_local_hook)))
@@ -1087,8 +1086,8 @@ reset_buffer_local_variables (struct buffer *b, bool permanent_too)
                   newlist = Fnreverse (newlist);
                   if (XSYMBOL (local_var)->u.s.trapped_write
 		      == SYMBOL_TRAPPED_WRITE)
-                    notify_variable_watchers (local_var, newlist,
-                                              Qmakunbound, Fcurrent_buffer ());
+                    notify_variable_watchers (local_var, newlist, Qmakunbound,
+					      Fcurrent_buffer (), true);
                   XSETCDR (XCAR (tmp), newlist);
                   continue; /* Don't do variable write trapping twice.  */
                 }
@@ -1112,7 +1111,24 @@ reset_buffer_local_variables (struct buffer *b, bool permanent_too)
       if ((idx > 0
 	   && (permanent_too
 	       || buffer_permanent_local_flags[idx] == 0)))
-	set_per_buffer_value (b, offset, per_buffer_default (offset));
+	{
+	  Lisp_Object symbol = PER_BUFFER_SYMBOL (offset);
+	  Lisp_Object old_value = ((idx == -1 || PER_BUFFER_VALUE_P (b, idx))
+				   ? per_buffer_value (b, offset)
+				   : Qnil);
+
+	  if (SYMBOLP (symbol)
+	      && XSYMBOL (symbol)->u.s.trapped_write == SYMBOL_TRAPPED_WRITE)
+	    notify_variable_watchers (symbol, old_value, Qmakunbound,
+				      Fcurrent_buffer (), false);
+
+	  set_per_buffer_value (b, offset, per_buffer_default (offset));
+
+	  if (SYMBOLP (symbol)
+	      && XSYMBOL (symbol)->u.s.trapped_write == SYMBOL_TRAPPED_WRITE)
+	    notify_variable_watchers (symbol, old_value, Qmakunbound,
+				      Fcurrent_buffer (), true);
+	}
     }
 }

diff --git a/src/data.c b/src/data.c
index d547f5da5e..bf0722bc65 100644
--- a/src/data.c
+++ b/src/data.c
@@ -1478,6 +1478,8 @@ set_internal (Lisp_Object symbol, Lisp_Object newval, Lisp_Object where,

   CHECK_SYMBOL (symbol);
   struct Lisp_Symbol *sym = XSYMBOL (symbol);
+  Lisp_Object old_value = Qunbound;
+
   switch (sym->u.s.trapped_write)
     {
     case SYMBOL_NOWRITE:
@@ -1489,13 +1491,13 @@ set_internal (Lisp_Object symbol, Lisp_Object newval, Lisp_Object where,
         return;

     case SYMBOL_TRAPPED_WRITE:
+      old_value = find_symbol_value (symbol);
       /* Setting due to thread-switching doesn't count.  */
       if (bindflag != SET_INTERNAL_THREAD_SWITCH)
-        notify_variable_watchers (symbol, voide? Qnil : newval,
-                                  (bindflag == SET_INTERNAL_BIND? Qlet :
-                                   bindflag == SET_INTERNAL_UNBIND? Qunlet :
-                                   voide? Qmakunbound : Qset),
-                                  where);
+        notify_variable_watchers (symbol, voide ? Qnil : newval,
+				  ((bindflag == SET_INTERNAL_BIND) ? Qlet :
+				   (bindflag == SET_INTERNAL_UNBIND) ? Qunlet :
+				   voide ? Qmakunbound : Qset), where, false);
       break;

     case SYMBOL_UNTRAPPED_WRITE:
@@ -1508,7 +1510,7 @@ set_internal (Lisp_Object symbol, Lisp_Object newval, Lisp_Object where,
   switch (sym->u.s.redirect)
     {
     case SYMBOL_VARALIAS: sym = indirect_variable (sym); goto start;
-    case SYMBOL_PLAINVAL: SET_SYMBOL_VAL (sym , newval); return;
+    case SYMBOL_PLAINVAL: SET_SYMBOL_VAL (sym, newval); goto stop;
     case SYMBOL_LOCALIZED:
       {
 	struct Lisp_Buffer_Local_Value *blv = SYMBOL_BLV (sym);
@@ -1619,6 +1621,15 @@ set_internal (Lisp_Object symbol, Lisp_Object newval, Lisp_Object where,
       }
     default: emacs_abort ();
     }
+
+ stop:
+  if (sym->u.s.trapped_write == SYMBOL_TRAPPED_WRITE
+      && bindflag != SET_INTERNAL_THREAD_SWITCH)
+    notify_variable_watchers (symbol, EQ (old_value, Qunbound) ? Qnil : old_value,
+			      ((bindflag == SET_INTERNAL_BIND) ? Qlet :
+			       (bindflag == SET_INTERNAL_UNBIND) ? Qunlet :
+			       voide ? Qmakunbound : Qset),
+			      where, true);
   return;
 }

@@ -1626,6 +1637,7 @@ set_internal (Lisp_Object symbol, Lisp_Object newval, Lisp_Object where,
 set_symbol_trapped_write (Lisp_Object symbol, enum symbol_trapped_write trap)
 {
   struct Lisp_Symbol *sym = XSYMBOL (symbol);
+
   if (sym->u.s.trapped_write == SYMBOL_NOWRITE)
     xsignal1 (Qtrapping_constant, symbol);
   sym->u.s.trapped_write = trap;
@@ -1647,7 +1659,7 @@ harmonize_variable_watchers (Lisp_Object alias, Lisp_Object base_variable)
 }

 DEFUN ("add-variable-watcher", Fadd_variable_watcher, Sadd_variable_watcher,
-       2, 2, 0,
+       2, 3, 0,
        doc: /* Cause WATCH-FUNCTION to be called when SYMBOL is about to be set.

 It will be called with 4 arguments: (SYMBOL NEWVAL OPERATION WHERE).
@@ -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.
+
 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);
+
   symbol = Findirect_variable (symbol);
   CHECK_SYMBOL (symbol);
   set_symbol_trapped_write (symbol, SYMBOL_TRAPPED_WRITE);
   map_obarray (Vobarray, harmonize_variable_watchers, symbol);

-  Lisp_Object watchers = Fget (symbol, Qwatchers);
-  Lisp_Object member = Fmember (watch_function, watchers);
-  if (NILP (member))
-    Fput (symbol, Qwatchers, Fcons (watch_function, watchers));
+  if (NILP (after))
+    {
+      Lisp_Object watchers = Fget (symbol, Qwatchers_before);
+
+      if (NILP (Fmember (watch_function, watchers)))
+	Fput (symbol, Qwatchers_before, Fcons (watch_function, watchers));
+    }
+  else
+    {
+      Lisp_Object watchers = Fget (symbol, Qwatchers_after);
+
+      if (NILP (Fmember (watch_function, watchers)))
+	Fput (symbol, Qwatchers_after, Fcons (watch_function, watchers));
+    }
+
   return Qnil;
 }

 DEFUN ("remove-variable-watcher", Fremove_variable_watcher, Sremove_variable_watcher,
-       2, 2, 0,
+       2, 3, 0,
        doc: /* Undo the effect of `add-variable-watcher'.
 Remove WATCH-FUNCTION from the list of functions to be called when
 SYMBOL (or its aliases) are set.  */)
-  (Lisp_Object symbol, Lisp_Object watch_function)
+  (Lisp_Object symbol, Lisp_Object watch_function, Lisp_Object after)
 {
   symbol = Findirect_variable (symbol);
-  Lisp_Object watchers = Fget (symbol, Qwatchers);
-  watchers = Fdelete (watch_function, watchers);
-  if (NILP (watchers))
+
+  Lisp_Object watchers_before = Fget (symbol, Qwatchers_before);
+  Lisp_Object watchers_after = Fget (symbol, Qwatchers_after);
+
+  if (NILP (after))
+    {
+      watchers_before = Fdelete (watch_function, watchers_before);
+      Fput (symbol, Qwatchers_before, watchers_before);
+    }
+  else
+    {
+      watchers_after = Fdelete (watch_function, watchers_after);
+      Fput (symbol, Qwatchers_after, watchers_after);
+    }
+
+  if (NILP (watchers_before) && NILP (watchers_after))
     {
       set_symbol_trapped_write (symbol, SYMBOL_UNTRAPPED_WRITE);
       map_obarray (Vobarray, harmonize_variable_watchers, symbol);
     }
-  Fput (symbol, Qwatchers, watchers);
+
   return Qnil;
 }

 DEFUN ("get-variable-watchers", Fget_variable_watchers, Sget_variable_watchers,
-       1, 1, 0,
-       doc: /* Return a list of SYMBOL's active watchers.  */)
-  (Lisp_Object symbol)
+       1, 2, 0,
+       doc: /* Return a list of SYMBOL's active watchers.
+Optional second argument AFTER nil means to return watchers run before
+SYMBOL's value is set.  AFTER non-nil means to return watchers run after
+SYMBOL's value is set.  */)
+  (Lisp_Object symbol, Lisp_Object after)
 {
-  return (SYMBOL_TRAPPED_WRITE_P (symbol) == SYMBOL_TRAPPED_WRITE)
-    ? Fget (Findirect_variable (symbol), Qwatchers)
-    : Qnil;
+  return (Fget (Findirect_variable (symbol),
+		NILP (after) ? Qwatchers_before : Qwatchers_after));
 }

 void
-notify_variable_watchers (Lisp_Object symbol,
-                          Lisp_Object newval,
-                          Lisp_Object operation,
-                          Lisp_Object where)
+notify_variable_watchers (Lisp_Object symbol, Lisp_Object newval,
+			  Lisp_Object operation, Lisp_Object where, bool after)
 {
   symbol = Findirect_variable (symbol);

@@ -1719,22 +1762,23 @@ notify_variable_watchers (Lisp_Object symbol,
   if (NILP (where)
       && !EQ (operation, Qset_default) && !EQ (operation, Qmakunbound)
       && !NILP (Flocal_variable_if_set_p (symbol, Fcurrent_buffer ())))
-    {
-      XSETBUFFER (where, current_buffer);
-    }
+    XSETBUFFER (where, current_buffer);

   if (EQ (operation, Qset_default))
     operation = Qset;

-  for (Lisp_Object watchers = Fget (symbol, Qwatchers);
+  for (Lisp_Object watchers
+	 = Fget (symbol, after ? Qwatchers_after : Qwatchers_before);
        CONSP (watchers);
        watchers = XCDR (watchers))
     {
       Lisp_Object watcher = XCAR (watchers);
+
       /* Call subr directly to avoid gc.  */
       if (SUBRP (watcher))
         {
           Lisp_Object args[] = { symbol, newval, operation, where };
+
           funcall_subr (XSUBR (watcher), ARRAYELTS (args), args);
         }
       else
@@ -1828,6 +1872,8 @@ set_default_internal (Lisp_Object symbol, Lisp_Object value,
 {
   CHECK_SYMBOL (symbol);
   struct Lisp_Symbol *sym = XSYMBOL (symbol);
+  Lisp_Object old_value = Qunbound;;
+
   switch (sym->u.s.trapped_write)
     {
     case SYMBOL_NOWRITE:
@@ -1839,11 +1885,12 @@ set_default_internal (Lisp_Object symbol, Lisp_Object value,
         return;

     case SYMBOL_TRAPPED_WRITE:
+      old_value = find_symbol_value (symbol);
       /* Don't notify here if we're going to call Fset anyway.  */
       if (sym->u.s.redirect != SYMBOL_PLAINVAL
           /* Setting due to thread switching doesn't count.  */
           && bindflag != SET_INTERNAL_THREAD_SWITCH)
-        notify_variable_watchers (symbol, value, Qset_default, Qnil);
+        notify_variable_watchers (symbol, value, Qset_default, Qnil, false);
       break;

     case SYMBOL_UNTRAPPED_WRITE:
@@ -1867,7 +1914,7 @@ set_default_internal (Lisp_Object symbol, Lisp_Object value,
 	/* If the default binding is now loaded, set the REALVALUE slot too.  */
 	if (blv->fwd.fwdptr && EQ (blv->defcell, blv->valcell))
 	  store_symval_forwarding (blv->fwd, value, NULL);
-        return;
+        goto stop;
       }
     case SYMBOL_FORWARDED:
       {
@@ -1906,10 +1953,16 @@ set_default_internal (Lisp_Object symbol, Lisp_Object value,
 	  }
 	else
           set_internal (symbol, value, Qnil, bindflag);
-        return;
+        goto stop;
       }
     default: emacs_abort ();
     }
+
+ stop:
+  if (sym->u.s.trapped_write == SYMBOL_TRAPPED_WRITE
+      && sym->u.s.redirect != SYMBOL_PLAINVAL
+      && bindflag != SET_INTERNAL_THREAD_SWITCH)
+    notify_variable_watchers (symbol, old_value, Qset_default, Qnil, true);
 }

 DEFUN ("set-default", Fset_default, Sset_default, 2, 2, 0,
@@ -2142,6 +2195,9 @@ DEFUN ("kill-local-variable", Fkill_local_variable, Skill_local_variable,
   struct Lisp_Symbol *sym;

   CHECK_SYMBOL (variable);
+
+  Lisp_Object old_value = find_symbol_value (variable);
+
   sym = XSYMBOL (variable);

  start:
@@ -2173,7 +2229,8 @@ DEFUN ("kill-local-variable", Fkill_local_variable, Skill_local_variable,
     }

   if (sym->u.s.trapped_write == SYMBOL_TRAPPED_WRITE)
-    notify_variable_watchers (variable, Qnil, Qmakunbound, Fcurrent_buffer ());
+    notify_variable_watchers (variable, Qnil, Qmakunbound,
+			      Fcurrent_buffer (), false);

   /* Get rid of this buffer's alist element, if any.  */
   XSETSYMBOL (variable, sym);	/* Propagate variable indirection.  */
@@ -2192,6 +2249,10 @@ DEFUN ("kill-local-variable", Fkill_local_variable, Skill_local_variable,
       swap_in_global_binding (sym);
   }

+  if (sym->u.s.trapped_write == SYMBOL_TRAPPED_WRITE)
+    notify_variable_watchers (variable, old_value, Qmakunbound,
+			      Fcurrent_buffer (), true);
+
   return variable;
 }

@@ -4195,7 +4256,8 @@ #define PUT_ERROR(sym, tail, msg)			\
   Vmost_negative_fixnum = make_fixnum (MOST_NEGATIVE_FIXNUM);
   make_symbol_constant (intern_c_string ("most-negative-fixnum"));

-  DEFSYM (Qwatchers, "watchers");
+  DEFSYM (Qwatchers_before, "watchers-before");
+  DEFSYM (Qwatchers_after, "watchers-after");
   DEFSYM (Qmakunbound, "makunbound");
   DEFSYM (Qunlet, "unlet");
   DEFSYM (Qset, "set");
diff --git a/src/eval.c b/src/eval.c
index aeedcc50cc..6d69d54ef2 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -603,6 +603,7 @@ DEFUN ("defvaralias", Fdefvaralias, Sdefvaralias, 2, 3, 0,
   (Lisp_Object new_alias, Lisp_Object base_variable, Lisp_Object docstring)
 {
   struct Lisp_Symbol *sym;
+  Lisp_Object old_value = Qnil;

   CHECK_SYMBOL (new_alias);
   CHECK_SYMBOL (base_variable);
@@ -634,7 +635,7 @@ DEFUN ("defvaralias", Fdefvaralias, Sdefvaralias, 2, 3, 0,
     set_internal (base_variable, find_symbol_value (new_alias),
                   Qnil, SET_INTERNAL_BIND);
   else if (!NILP (Fboundp (new_alias))
-           && !EQ (find_symbol_value (new_alias),
+           && !EQ (old_value = find_symbol_value (new_alias),
                    find_symbol_value (base_variable)))
     call2 (intern ("display-warning"),
            list3 (Qdefvaralias, intern ("losing-value"), new_alias),
@@ -653,8 +654,8 @@ DEFUN ("defvaralias", Fdefvaralias, Sdefvaralias, 2, 3, 0,
   }

   if (sym->u.s.trapped_write == SYMBOL_TRAPPED_WRITE)
-    notify_variable_watchers (new_alias, base_variable, Qdefvaralias, Qnil);
-
+    notify_variable_watchers (new_alias, base_variable,
+			      Qdefvaralias, Qnil, false);
   sym->u.s.declared_special = true;
   XSYMBOL (base_variable)->u.s.declared_special = true;
   sym->u.s.redirect = SYMBOL_VARALIAS;
@@ -664,6 +665,9 @@ DEFUN ("defvaralias", Fdefvaralias, Sdefvaralias, 2, 3, 0,
   /* Even if docstring is nil: remove old docstring.  */
   Fput (new_alias, Qvariable_documentation, docstring);

+  if (sym->u.s.trapped_write == SYMBOL_TRAPPED_WRITE)
+    notify_variable_watchers (new_alias, old_value, Qdefvaralias, Qnil, true);
+
   return base_variable;
 }

diff --git a/src/lisp.h b/src/lisp.h
index f83c55f827..ba7edb86a3 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3535,7 +3535,7 @@ modiff_to_integer (modiff_count a)
 /* Defined in data.c.  */
 extern AVOID wrong_choice (Lisp_Object, Lisp_Object);
 extern void notify_variable_watchers (Lisp_Object, Lisp_Object,
-				      Lisp_Object, Lisp_Object);
+				      Lisp_Object, Lisp_Object, bool);
 extern Lisp_Object indirect_function (Lisp_Object);
 extern Lisp_Object find_symbol_value (Lisp_Object);
 enum Arith_Comparison {

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.

Re: "after" variable watchers

[martin rudalics]

 > 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?

Because knowing the new value in the watch function is of no practical
use.  I'd have to pass it down to all functions called directly or
indirectly by the watch function that make use of that value.

 >> 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.

I have a function called window_updeco_window that decides for each
window which of its decorations can be displayed and which decorations
must be suppressed because the window is too small.  Each aspect like
the width of the scroll bar or its right margin has a nominal and a
realized value.

The nominal value is the one set by default or by the user in a frame-,
buffer- or window-local way.  That value changes only when it is set by
the user.

The realized value is the one as it will appear on display.  It is
calculated from the nominal value and constraints imposed by the actual
size of the window, the window's character sizes and minimum body sizes.
If, for example, the nominal width of a scroll bar is too large, that
scroll bar is not displayed.  If the nominal width of the right margin
is too large, that margin will be either shrunk or not displayed at all.

window_updeco_window is called indirectly from all places that change a
window's decorations, font or size.  In order to have it DTRT when a
user wants to change a buffer's right margin width, for example, I would
have to pass the new value from the variable watcher function to
window_updeco_window and from there to the function that calculates the
realized width of the right margin from that of its nominal width.

That final client would then have to perform the sanity check whether
the value passed to it is really a number (or, for example, the value
for the vertical scroll bar type is really one of 'right', 'left' etc.).
Checks we would then have do perform twice because they would be
immediately afterwards be done in exactly the same way when setting the
buffer-local value.  Been there, tried that, didn't like it at all.

 > 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?

Right.  In the mold of your `add-variable-watcher' at the end of
frame.el.

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

First, it's useful for having a change in a buffer's decorations take
effect immediately.  So we get rid of things like

   Setting this variable does not take effect until a new buffer is displayed
   in a window.  To make the change take effect, call ‘set-window-buffer’.

We also achieve a more consistent handling of the values of decorations.
Think of recalculating a specific window's decorations from a newly set
`scroll-bar-width' frame parameter and a buffer-local `scroll-bar-width'
that has not taken effect yet.

And finally we would get rid of the present mixture of errors thrown at
the user and that of changes that are silently ignored whenever a
decoration does not fit.  A user then can set the nominal width of the
right margin to some arbitrary number.  When the window is large enough
to accommodate it, it will be displayed that way.  When the window gets
too small, it will be temporarily clipped or skipped.

 > 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.

Right.  So you mean to write a separate notify_after_variable_watchers
function, say?  Good idea.

 >> +
 >>   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.

There wasn't at the time I wrote that about a year ago and for some
reason the new instance got merged successfully.

Many thanks for the comments, martin

Re: "after" variable watchers

[Eli Zaretskii]

> Cc: npostavs@gmail.com, emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Mon, 17 May 2021 18:40:09 +0200
> 
>  > 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?
> 
> Because knowing the new value in the watch function is of no practical
> use.  I'd have to pass it down to all functions called directly or
> indirectly by the watch function that make use of that value.

Sorry, I don't understand.  I really don't, not even after reading
your explanations.  (Assuming I understood the explanations: they seem
to mix what you intend to do with this "after-watcher" and what you
would have needed with the current watcher machinery, so it's
difficult to figure out which is which.)

Perhaps taking a single example and explaining how would this work
with today's watchers and with the feature you want to install could
help me understand.

>  > 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?
> 
> Right.  In the mold of your `add-variable-watcher' at the end of
> frame.el.

So if I remove those, you will retract this proposal?

And note that the analogy is incomplete at best: the watchers in
frame.el watch _variables_ that we used to have forever, so making
them modifiable only via accessor functions would be a breaking
change.  Whereas you are talking about stuff which is done by
functions already.

>  > First, I'd like to better understand the need for this.
> 
> First, it's useful for having a change in a buffer's decorations take
> effect immediately.  So we get rid of things like
> 
>    Setting this variable does not take effect until a new buffer is displayed
>    in a window.  To make the change take effect, call ‘set-window-buffer’.

I think this could be a step in the wrong direction.  It might be
easier for users, but it makes it harder for us to develop Emacs in
the long run, for example if we want more threading.  That's because
all those global and buffer-local variables are part of the huge
global state we have, and that gets in the way of some features.

> And finally we would get rid of the present mixture of errors thrown at
> the user and that of changes that are silently ignored whenever a
> decoration does not fit.  A user then can set the nominal width of the
> right margin to some arbitrary number.  When the window is large enough
> to accommodate it, it will be displayed that way.  When the window gets
> too small, it will be temporarily clipped or skipped.

So we get silent fixes for all of them?  Not sure this is an
improvement.  I think we already had complaints about silently
"fixing" the decorations as we see fit.

Re: "after" variable watchers

[martin rudalics]

 > Perhaps taking a single example and explaining how would this work
 > with today's watchers and with the feature you want to install could
 > help me understand.

Maybe the following helps: I propose a function to calculate the desired
width of a window W's left fringe from, in this order

(1) The desired width of W's left fringe as set by `set-window-fringes'
     and stored in a slot w->nominal_left_fringe_width where a value of
     -1 stands for "take the width from W's buffer",

(2) the value of `left-fringe-width' of W's buffer where anything but a
     non-negative number means to "take the width from W's frame", and

(3) the value of f->left_fringe_width of W's frame as installed by
     gui_set_left_fringe - a non-negative integer.

This is the function:

/** Return configured width of W's left fringe.  */
static int
window_config_left_fringe_width (struct window *w)
{
   int width = (WINDOW_NON_GRAPHICAL_OR_PSEUDO_P (w)
	       ? 0 : w->nominal_left_fringe_width);

   if (width < 0)
     {
       Lisp_Object value = WINDOW_BUFFER_LOCAL_VALUE (Qleft_fringe_width, w);

       width = (RANGED_FIXNUMP (0, value, INT_MAX)
	       ? XFIXNAT (value)
	       : FRAME_LEFT_FRINGE_WIDTH (XFRAME (WINDOW_FRAME (w))));
     }

   return width;
}

window_config_left_fringe_width would be run, among many others,
whenever we

(i) call `set-window-fringes' for W, change `left-fringe-width' of W's
     buffer or change the 'left-fringe' parameter of W's frame, and

(ii) change the size of W.

The value produced by this function is stored in w->left_fringe_width
provided it fits into that window.  Otherwise, a value of 0 is stored.
(In the latter case, all remaining horizontal decorations of W will get
a width of 0 too.) The value stored is called the "realized" value of
W's left fringe width.  The realized value is used to display the
window, to determine its box width and the horizontal position where the
text area starts.

The value produced by this function is _also_ used to determine the
minimum width of W which is needed to tell whether W can be split or
resized and also to tell the window manager what W's frame's minimum
width should be.

Please tell me now if any of the reasoning I've done so far is wrong in
your opinion.  In that case we could try to find a different solution
for such a function or abolish the idea.  Otherwise, please read on.

Now suppose I assign a new value to `left-fringe-width' of W's buffer.

- If I do not watch `left-fringe-width' at all, the new value will be
   picked up by redisplay the next time I call this function for some
   other reason, for example, when resizing W, unless it is preceded by a
   `set-window-buffer'.  Such an effect could be surprising at least.  It
   may lead to inconsistent behavior when W shall be split or resized.

- If I use a normal "before" watcher, the effect would probably be seen
   with the next redisplay.  Still there's no guarantee that this would
   happen, so a surprising effect can still not be excluded.

So I would either have to explicitly pass the new value of
`left-fringe-width' down to window_config_left_fringe_width, check there
whether it is sane (which amounts to do twice everything the
gui_set_... functions do) and apply it then or, use what I called an
"after" variable watcher which installs the new value before running
window_config_left_fringe_width.

 >> Right.  In the mold of your `add-variable-watcher' at the end of
 >> frame.el.
 >
 > So if I remove those, you will retract this proposal?

It would be a pity to remove them.  And obviously you don't have to
remove any of those if you want me to retract my proposal.

 > And note that the analogy is incomplete at best: the watchers in
 > frame.el watch _variables_ that we used to have forever, so making
 > them modifiable only via accessor functions would be a breaking
 > change.  Whereas you are talking about stuff which is done by
 > functions already.

My window.el has

(mapc (lambda (var)
         (add-variable-watcher var (symbol-function 'window-update-decorations-for-variable) t))
       '(window-scale-body
         window-drop-decorations
         face-remapping-alist
         line-spacing
         min-body-width
         min-body-height
         left-fringe-width
         right-fringe-width
         fringes-outside-margins
         vertical-scroll-bar
         horizontal-scroll-bar
         scroll-bar-width
         scroll-bar-height
         left-margin-width
         right-margin-width
         mode-line-format
         header-line-format
         tab-line-format))

where most are variables that we used to have for quite some time and
some figure here as well as in your list.

 >> First, it's useful for having a change in a buffer's decorations take
 >> effect immediately.  So we get rid of things like
 >>
 >>     Setting this variable does not take effect until a new buffer is displayed
 >>     in a window.  To make the change take effect, call ‘set-window-buffer’.
 >
 > I think this could be a step in the wrong direction.  It might be
 > easier for users, but it makes it harder for us to develop Emacs in
 > the long run, for example if we want more threading.  That's because
 > all those global and buffer-local variables are part of the huge
 > global state we have, and that gets in the way of some features.

Can you explain how my proposal could be related to threading and what
kind of features it might break there?

 >> And finally we would get rid of the present mixture of errors thrown at
 >> the user and that of changes that are silently ignored whenever a
 >> decoration does not fit.  A user then can set the nominal width of the
 >> right margin to some arbitrary number.  When the window is large enough
 >> to accommodate it, it will be displayed that way.  When the window gets
 >> too small, it will be temporarily clipped or skipped.
 >
 > So we get silent fixes for all of them?  Not sure this is an
 > improvement.  I think we already had complaints about silently
 > "fixing" the decorations as we see fit.

Your last statement is partially ironic and not really nice, in
particular because it inverts the state of affairs.

Emacs 27 explicitly states in set_window_fringes to

       	 Check dimensions of new fringes.  Make changes only if they
	 fit the window's dimensions.

So Emacs 27 silently ignores the value of `left-fringe-width' whenever
it does not fit and keeps on ignoring it when the window gets enlarged
sufficiently to show them afterwards.

With my proposal, no settings are ever ignored or fixed silently.
Decorations will grow and shrink together with their windows and frames.
IMO this is better than, for example, showing the left margin instead of
the window's text area when a window has become too narrow.

martin

Re: "after" variable watchers

[Eli Zaretskii]

> Cc: npostavs@gmail.com, emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 18 May 2021 17:10:24 +0200
> 
> (1) The desired width of W's left fringe as set by `set-window-fringes'
>      and stored in a slot w->nominal_left_fringe_width where a value of
>      -1 stands for "take the width from W's buffer",

Side note: could we please NOT call these "nominal" values?  "Nominal"
means "standard", whereas these aren't.  How about "desired" or
"requested"?

> (2) the value of `left-fringe-width' of W's buffer where anything but a
>      non-negative number means to "take the width from W's frame", and
> 
> (3) the value of f->left_fringe_width of W's frame as installed by
>      gui_set_left_fringe - a non-negative integer.
> 
> This is the function:
> 
> /** Return configured width of W's left fringe.  */
> static int
> window_config_left_fringe_width (struct window *w)
> {
>    int width = (WINDOW_NON_GRAPHICAL_OR_PSEUDO_P (w)
> 	       ? 0 : w->nominal_left_fringe_width);
> 
>    if (width < 0)
>      {
>        Lisp_Object value = WINDOW_BUFFER_LOCAL_VALUE (Qleft_fringe_width, w);
> 
>        width = (RANGED_FIXNUMP (0, value, INT_MAX)
> 	       ? XFIXNAT (value)
> 	       : FRAME_LEFT_FRINGE_WIDTH (XFRAME (WINDOW_FRAME (w))));
>      }
> 
>    return width;
> }
> 
> window_config_left_fringe_width would be run, among many others,
> whenever we
> 
> (i) call `set-window-fringes' for W, change `left-fringe-width' of W's
>      buffer or change the 'left-fringe' parameter of W's frame, and
> 
> (ii) change the size of W.
> 
> The value produced by this function is stored in w->left_fringe_width
> provided it fits into that window.  Otherwise, a value of 0 is stored.
> (In the latter case, all remaining horizontal decorations of W will get
> a width of 0 too.) The value stored is called the "realized" value of
> W's left fringe width.  The realized value is used to display the
> window, to determine its box width and the horizontal position where the
> text area starts.
> 
> The value produced by this function is _also_ used to determine the
> minimum width of W which is needed to tell whether W can be split or
> resized and also to tell the window manager what W's frame's minimum
> width should be.
> 
> Please tell me now if any of the reasoning I've done so far is wrong in
> your opinion.  In that case we could try to find a different solution
> for such a function or abolish the idea.  Otherwise, please read on.
> 
> Now suppose I assign a new value to `left-fringe-width' of W's buffer.
> 
> - If I do not watch `left-fringe-width' at all, the new value will be
>    picked up by redisplay the next time I call this function for some
>    other reason, for example, when resizing W, unless it is preceded by a
>    `set-window-buffer'.  Such an effect could be surprising at least.  It
>    may lead to inconsistent behavior when W shall be split or resized.

Presumably, we are now at this situation?  If so, what are the
practical problems with what we have now?  We had this since many
years ago, and I'm not aware of any significant problems with what we
have.  So what are the reasons for wanting such a significant new
infrastructure, if we don't have serious problems with what we have?

> - If I use a normal "before" watcher, the effect would probably be seen
>    with the next redisplay.  Still there's no guarantee that this would
>    happen, so a surprising effect can still not be excluded.
> 
> So I would either have to explicitly pass the new value of
> `left-fringe-width' down to window_config_left_fringe_width, check there
> whether it is sane (which amounts to do twice everything the
> gui_set_... functions do) and apply it then or, use what I called an
> "after" variable watcher which installs the new value before running
> window_config_left_fringe_width.

If your proposed "after" watcher will calculate a "sanitized" value of
w->left_fringe_width and store it in the window's structure, then why
does it have to run "after" the change? the effect of the change will
not be shown until the next redisplay cycle anyway.

>  > And note that the analogy is incomplete at best: the watchers in
>  > frame.el watch _variables_ that we used to have forever, so making
>  > them modifiable only via accessor functions would be a breaking
>  > change.  Whereas you are talking about stuff which is done by
>  > functions already.
> 
> My window.el has
> 
> (mapc (lambda (var)
>          (add-variable-watcher var (symbol-function 'window-update-decorations-for-variable) t))
>        '(window-scale-body
>          window-drop-decorations
>          face-remapping-alist
>          line-spacing
>          min-body-width
>          min-body-height
>          left-fringe-width
>          right-fringe-width
>          fringes-outside-margins
>          vertical-scroll-bar
>          horizontal-scroll-bar
>          scroll-bar-width
>          scroll-bar-height
>          left-margin-width
>          right-margin-width
>          mode-line-format
>          header-line-format
>          tab-line-format))
> 
> where most are variables that we used to have for quite some time and
> some figure here as well as in your list.

line-spacing and face-remapping-alist don't really belong there, do
they?  Likewise mode-line-format and header-line-format, AFAIR.
AFAIK, the changes in those are immediately shown by the next
redisplay.  So why are they in the above list?

As for the rest: once again, what problems do we have now with changes
to them?

Note that changes to the variables listed in frame.el do only one
thing: set a flag in the buffer's structure telling the display code
this buffer needs to be redisplayed.  By contrast, you want to run
complex functions when the variables are modified, and actually affect
the values stored as result.  That is unheard of in Emacs; it is
almost like you are asking for having changes in some variables to
automatically run a function.

>  > I think this could be a step in the wrong direction.  It might be
>  > easier for users, but it makes it harder for us to develop Emacs in
>  > the long run, for example if we want more threading.  That's because
>  > all those global and buffer-local variables are part of the huge
>  > global state we have, and that gets in the way of some features.
> 
> Can you explain how my proposal could be related to threading and what
> kind of features it might break there?

Reducing the global state means we need to access variables via
accessor functions.  Your suggestion goes in the exact opposite
direction: you want a change in a variable's value to have an
immediate effect, which would mean we'd need more variables to be
thread-local, or risk causing changes in the wrong buffer/thread.

>  >> And finally we would get rid of the present mixture of errors thrown at
>  >> the user and that of changes that are silently ignored whenever a
>  >> decoration does not fit.  A user then can set the nominal width of the
>  >> right margin to some arbitrary number.  When the window is large enough
>  >> to accommodate it, it will be displayed that way.  When the window gets
>  >> too small, it will be temporarily clipped or skipped.
>  >
>  > So we get silent fixes for all of them?  Not sure this is an
>  > improvement.  I think we already had complaints about silently
>  > "fixing" the decorations as we see fit.
> 
> Your last statement is partially ironic and not really nice, in
> particular because it inverts the state of affairs.

There's no irony there, none at all.  As for not being nice, I
apologize if this comes that way, but I assure you nothing like that
was ever intended.

> Emacs 27 explicitly states in set_window_fringes to
> 
>        	 Check dimensions of new fringes.  Make changes only if they
> 	 fit the window's dimensions.
> 
> So Emacs 27 silently ignores the value of `left-fringe-width' whenever
> it does not fit and keeps on ignoring it when the window gets enlarged
> sufficiently to show them afterwards.

Ignoring invalid changes, when this is documented, is not a
catastrophe.  Maybe I don't understand your plan, but it sounds to me
like you want to extend such silent "editing" of set values to many
more widow-related attributes; left-fringe-width was just an example,
right? and setting it to zero when unfit was also just an example,
right?

> With my proposal, no settings are ever ignored or fixed silently.
> Decorations will grow and shrink together with their windows and frames.
> IMO this is better than, for example, showing the left margin instead of
> the window's text area when a window has become too narrow.

I don't know if it's better.  I'm mainly worried that we are trying to
invent significant infrastructure to fix problems that I don't think I
understand sufficiently well, and in fact am not aware they even
exist.  So maybe we should make a step back and describe and discuss
those problems first.

Re: "after" variable watchers

[martin rudalics]

 >> (1) The desired width of W's left fringe as set by `set-window-fringes'
 >>       and stored in a slot w->nominal_left_fringe_width where a value of
 >>       -1 stands for "take the width from W's buffer",
 >
 > Side note: could we please NOT call these "nominal" values?  "Nominal"
 > means "standard", whereas these aren't.  How about "desired" or
 > "requested"?

I've been using "nominal" in the sense expressed by Wikipedia as "From a
philosophical viewpoint, nominal value represents an accepted condition,
which is a goal or an approximation, as opposed to the real value, which
is always present.".  That sentence expresses well what I'm trying to do
here IMHO.

 >> Now suppose I assign a new value to `left-fringe-width' of W's buffer.
 >>
 >> - If I do not watch `left-fringe-width' at all, the new value will be
 >>     picked up by redisplay the next time I call this function for some
 >>     other reason, for example, when resizing W, unless it is preceded by a
 >>     `set-window-buffer'.  Such an effect could be surprising at least.  It
 >>     may lead to inconsistent behavior when W shall be split or resized.
 >
 > Presumably, we are now at this situation?

Not really.  Capturing the present situation would require to:

- Forget earlier requests to set a specific window's fringe width.  The
   value we currently store for a window's fringe width is agnostic wrt
   from where it was obtained from and thus cannot be restored from any
   nominal value once it has been configured.

- Prevent applying `left-fringe-width' until set_window_buffer is run.

 > If so, what are the
 > practical problems with what we have now?  We had this since many
 > years ago, and I'm not aware of any significant problems with what we
 > have.  So what are the reasons for wanting such a significant new
 > infrastructure, if we don't have serious problems with what we have?

What we have now is not what we had since "many" years ago.  A window
margin would shrink when dragging the divider between the window and its
neighbor and become sticky after that in Emacs 24.  Try with Emacs 24,
set in *scratch* a `left-margin-width' of 40, do C-x 3 and drag the
divider between the mode lines back and forth.  Eventually, the margins
in both windows will have dropped to 4 columns.

Note that if, in Emacs 24, such a margin is shrunk due to shrinking its
frame, it may disappear entirely.  Try with a left margin width of 40
again, do C-x 3 and drag the frame's right edge to shrink the windows
and expand it again.  The behavior of Emacs 24 in this regard is rather
unpredictable.

Note also that in either case it's technically impossible to re-enlarge
margins after they have shrunk because, as mentioned earlier, the origin
of the configured value has been lost.

With Emacs 25, splitting a window with large margins was inhibited and
this became a constant source of troubles so we added the `min-margins'
crutches which, to give reasonable results, have to be accompanied by an
external application to handle margins widths after a window has been
resized.  So while the behavior of Emacs 25 became more predictable, it
also introduced restrictions people didn't like and still awaits a
solution for handling margins "reasonably" by default.

 > If your proposed "after" watcher will calculate a "sanitized" value of
 > w->left_fringe_width and store it in the window's structure, then why
 > does it have to run "after" the change? the effect of the change will
 > not be shown until the next redisplay cycle anyway.

But other parts of our code may want to know the size of the fringes
before that.  Think of someone displaying a buffer in a window, setting
up some decorations and trying to fit that window to its buffer.  If
that function used the old sizes of decorations, things may go wrong
when redisplay decides to change sizes after that.  Similar reasoning
applies to all other functions that try to split or resize windows.

 >> (mapc (lambda (var)
 >>           (add-variable-watcher var (symbol-function 'window-update-decorations-for-variable) t))
 >>         '(window-scale-body
 >>           window-drop-decorations
 >>           face-remapping-alist
 >>           line-spacing
 >>           min-body-width
 >>           min-body-height
 >>           left-fringe-width
 >>           right-fringe-width
 >>           fringes-outside-margins
 >>           vertical-scroll-bar
 >>           horizontal-scroll-bar
 >>           scroll-bar-width
 >>           scroll-bar-height
 >>           left-margin-width
 >>           right-margin-width
 >>           mode-line-format
 >>           header-line-format
 >>           tab-line-format))
 >>
 >> where most are variables that we used to have for quite some time and
 >> some figure here as well as in your list.
 >
 > line-spacing and face-remapping-alist don't really belong there, do
 > they?

They do if we want to make windows sensitive to any text scaling of
their buffers.  The new variable `window-scale-body' tells whether to
interpret the specified minimum body height (a number of lines) of a
window in terms of that window's character height - which may include
any line spacing - or in the line height of the window's frame.  This
should fix a number of bugs where people complained that Emacs always
uses the frame's line height in such a case.  BTW, the way to fix the
behavior here is extremely hairy and much more controversial than what
we are discussing in the present thread.

 > Likewise mode-line-format and header-line-format, AFAIR.
 > AFAIK, the changes in those are immediately shown by the next
 > redisplay.  So why are they in the above list?

When a window gets very small, I first want to remove its horizontal
scroll bar, then its tab and header lines and keep its mode line visible
as long as possible.  The redisplay code then will be told to not
redisplay the former.  But maybe you're right here and we can get rid of
these.

 > As for the rest: once again, what problems do we have now with changes
 > to them?
 >
 > Note that changes to the variables listed in frame.el do only one
 > thing: set a flag in the buffer's structure telling the display code
 > this buffer needs to be redisplayed.

.. and at that time take their new values into account ...

 > By contrast, you want to run
 > complex functions when the variables are modified, and actually affect
 > the values stored as result.  That is unheard of in Emacs; it is
 > almost like you are asking for having changes in some variables to
 > automatically run a function.

Right.  But I do not run a Lisp function.  Though I see no harm in
running a Lisp function from a variable watcher either.

 > Reducing the global state means we need to access variables via
 > accessor functions.  Your suggestion goes in the exact opposite
 > direction: you want a change in a variable's value to have an
 > immediate effect, which would mean we'd need more variables to be
 > thread-local, or risk causing changes in the wrong buffer/thread.

I have no idea how to impede having "a change in a variable's value have
an immediate effect".  But if you came up with an example how setting a
variable could affect threading detrimentally, I might be able to
understand.

 > Ignoring invalid changes, when this is documented, is not a
 > catastrophe.

What makes a change invalid?  A fringe width for one and the same buffer
may fit into one window showing that buffer and not fit into another
window showing it.  That's what Emacs 27 does and I won't change that.
The only thing that I intend to change is to make that fringe reappear
in a window whenever that window gets large enough to show it.  So we
would actually do less "silent editing" than with Emacs 27 or at least
not more.

 > Maybe I don't understand your plan, but it sounds to me
 > like you want to extend such silent "editing" of set values to many
 > more widow-related attributes; left-fringe-width was just an example,
 > right?

Right.

 > and setting it to zero when unfit was also just an example,
 > right?

The width of a fringe would always be either the requested value when it
fits or zero when it does not fit.  Window margins OTOH would shrink and
expand dynamically.

 >> With my proposal, no settings are ever ignored or fixed silently.
 >> Decorations will grow and shrink together with their windows and frames.
 >> IMO this is better than, for example, showing the left margin instead of
 >> the window's text area when a window has become too narrow.
 >
 > I don't know if it's better.  I'm mainly worried that we are trying to
 > invent significant infrastructure to fix problems that I don't think I

Why do you think that my change to variable watchers would "invent
significant infrastructure".  It hardly does anything that is not
already there.  If an application wants to use a variable's new value
and react to it immediately, it can do that already now, albeit in a
less comfortable way.

 > understand sufficiently well, and in fact am not aware they even
 > exist.  So maybe we should make a step back and describe and discuss
 > those problems first.

Not processing buffer local variables immediately has surprising effects
because it depends on whether `set-window-buffer' gets executed for that
buffer or not.  With emacs -Q consider

(setq left-margin-width 20)
C-x <right>
C-x <left>

or instead of C-x <left>  do C-x k.

It also makes `display-buffer' inconsistent.  If the buffer is already
shown in a window, it does not get the new margin width, otherwise it
will.  Finally, it may render the results of `window-state-put' and
`set-window-configuration' incompatible since the former uses
`set-window-buffer' while the latter doesn't.

All these imply that whenever users set one of these variables, they
should call `set-window-buffer' for all windows showing the buffer right
away.  Just that if the window is too small and the margins don't fit,
users still will not see them later even if they make enough room for
them.  And if they fit and a user shrinks the window, the margins might
continue to occupy the entire display area of that window.

martin

Re: "after" variable watchers

[Eli Zaretskii]

> Cc: npostavs@gmail.com, emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Mon, 24 May 2021 10:47:29 +0200
> 
>  >> (1) The desired width of W's left fringe as set by `set-window-fringes'
>  >>       and stored in a slot w->nominal_left_fringe_width where a value of
>  >>       -1 stands for "take the width from W's buffer",
>  >
>  > Side note: could we please NOT call these "nominal" values?  "Nominal"
>  > means "standard", whereas these aren't.  How about "desired" or
>  > "requested"?
> 
> I've been using "nominal" in the sense expressed by Wikipedia as "From a
> philosophical viewpoint, nominal value represents an accepted condition,
> which is a goal or an approximation, as opposed to the real value, which
> is always present.".  That sentence expresses well what I'm trying to do
> here IMHO.

Well, I'm asking you to use a different terminology, because it always
trips me, Wikipedia notwithstanding.

More to the point: the more I think about what you want to do the less
I like it.  So I'm going to stop thinking and just say that I don't
think we should do it.  I understand the motivation, but I think it's
a slippery slope towards much more complex and hard to maintain code,
with a goal that I believe cannot be reached, only approximated.  We
will complicate the heck out of the window-resizing functions, and in
the end will not be able to promise that any change will be
immediately visible through the accessors and variables.  E.g., what
happens if redisplay decides to resize the mini-window -- you end up
with at least one window whose dimensions are not what was before.

We don't make such promises now, and with frames AFAIU we cannot even
try promising that (because WMs get in the way).  So why complicate
Emacs so much for such ephemeral reasons?  I say let's document that
not every change is immediately reflected, until the next redisplay,
and move on to more important developments.

Having said that, you are the expert on this, so if you decide to do
this anyway, I won't fight you or forbid doing that.  I just think we
shouldn't do this.

Re: "after" variable watchers

[martin rudalics]

 > Having said that, you are the expert on this, so if you decide to do
 > this anyway, I won't fight you or forbid doing that.  I just think we
 > shouldn't do this.

So I'll abandon any solution based on variable watchers.  Whether this
permits me to install the remaining changes I planned is a question that
I cannot answer yet.

Thanks, martin

Re: "after" variable watchers

[Stefan Monnier]

> window_updeco_window is called indirectly from all places that change a
> window's decorations, font or size.

Would it make sense to call it more lazily, e.g. as part of redisplay
(basically, the watchpoints would just set some dirty bits and then at
the beginning of redisplay you'd then run `window_updeco_window` on
those windows with the dirty bit set)?


        Stefan

Re: "after" variable watchers

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  npostavs@gmail.com,  emacs-devel@gnu.org
> Date: Mon, 17 May 2021 14:36:28 -0400
> 
> > window_updeco_window is called indirectly from all places that change a
> > window's decorations, font or size.
> 
> Would it make sense to call it more lazily, e.g. as part of redisplay
> (basically, the watchpoints would just set some dirty bits and then at
> the beginning of redisplay you'd then run `window_updeco_window` on
> those windows with the dirty bit set)?

You don't need a watcher to set flags on windows.  Each change in the
dimensions of windows and their decorations is already preformed by
functions.

Re: "after" variable watchers

[Stefan Monnier]

>> > window_updeco_window is called indirectly from all places that change a
>> > window's decorations, font or size.
>> 
>> Would it make sense to call it more lazily, e.g. as part of redisplay
>> (basically, the watchpoints would just set some dirty bits and then at
>> the beginning of redisplay you'd then run `window_updeco_window` on
>> those windows with the dirty bit set)?
>
> You don't need a watcher to set flags on windows.  Each change in the
> dimensions of windows and their decorations is already preformed by
> functions.

You're asking for a watcher, right?
I's telling you to make this watcher set a dirty flag rather than
immediately reacting to that variable value's change.


        Stefan

Re: "after" variable watchers

[Stefan Monnier]

> You're asking for a watcher, right?

[ Sorry, wrong person: you're not, he is, but I hope what I meant was
  still clear enough.  ]


        Stefan

Re: "after" variable watchers

[martin rudalics]

 >> window_updeco_window is called indirectly from all places that change a
 >> window's decorations, font or size.
 >
 > Would it make sense to call it more lazily, e.g. as part of redisplay
 > (basically, the watchpoints would just set some dirty bits and then at
 > the beginning of redisplay you'd then run `window_updeco_window` on
 > those windows with the dirty bit set)?

That was my first approach given our current implementation of watch
points.  I spent a couple of weeks on it but it didn't really work out.
Between the time a variable is set and the time the setting is applied,
the "realized" values would be inaccurate.  These values are, however,
needed in too many places before we start redisplaying a window.

I do not want to exclude that such a lazy approach could work.  But it
would require more profound changes to the way window sizes and
decorations are processed than the approach based on "after" variable
watchers.

martin

Re: "after" variable watchers

[Stefan Monnier]

martin rudalics [2021-05-18 17:10:47] wrote:
>>> window_updeco_window is called indirectly from all places that change a
>>> window's decorations, font or size.
>>
>> Would it make sense to call it more lazily, e.g. as part of redisplay
>> (basically, the watchpoints would just set some dirty bits and then at
>> the beginning of redisplay you'd then run `window_updeco_window` on
>> those windows with the dirty bit set)?
>
> That was my first approach given our current implementation of watch
> points.  I spent a couple of weeks on it but it didn't really work out.
> Between the time a variable is set and the time the setting is applied,
> the "realized" values would be inaccurate.  These values are, however,
> needed in too many places before we start redisplaying a window.

What was the problem you encountered when you tried to add:

    if (<dirty>) window_updeco_window (...);

to the getters of the realized values?

> I do not want to exclude that such a lazy approach could work.  But it
> would require more profound changes to the way window sizes and
> decorations are processed than the approach based on "after" variable
> watchers.

You obviously know more about this code, so I can't say much more.
I was just pointing in the direction of using dirty "bits" (it's
actually often better to represent dirty state via things like our
buffer ticks than via actual `dirty` bits) because it's the standard
solution for such problems: experience has usually shown that decoupling
the setting of vars from the execution of the consequences usually leads
to a much more solid design where it's much easier to deal with problems
of inf-loops or (in)efficiency.


        Stefan

Re: "after" variable watchers

[martin rudalics]

 > What was the problem you encountered when you tried to add:
 >
 >      if (<dirty>) window_updeco_window (...);
 >
 > to the getters of the realized values?

There are too many implicit "realized" values to make this work nicely.
Think of macros like WINDOW_BODY_PIXEL_WIDTH, WINDOW_BOX_LEFT_EDGE_X,
WINDOW_BOX_RIGHT_EDGE_X, WINDOW_SCROLL_BAR_X or functions like
window_box_height in addition to the things window_updeco_window sets
directly.

 > You obviously know more about this code, so I can't say much more.
 > I was just pointing in the direction of using dirty "bits" (it's
 > actually often better to represent dirty state via things like our
 > buffer ticks than via actual `dirty` bits) because it's the standard
 > solution for such problems: experience has usually shown that decoupling
 > the setting of vars from the execution of the consequences usually leads
 > to a much more solid design where it's much easier to deal with problems
 > of inf-loops or (in)efficiency.

Right.  But note that I implicitly call window_updeco_window also
whenever I change a window's size, for example, from adjust_frame_size
after a frame got resized.  A `window-body-size' or `window-min-size'
call after that would then require to run window_updeco_window too.
Identifying all sorts of such getters is not a very reliable process.
(BTW, identifying all sorts of setters is not that easy either -
face-remap.el alone contains six calls for this.)

I do use a quite extended mechanism for identifying a "dirty" state when
trying to catch changes in windows (see run_window_change_functions) so
I think that I'm not allergic to such a solution.  But in the case at
hand I simply failed to integrate it well in the existing framework.

martin

Re: "after" variable watchers

[Eli Zaretskii]

> Cc: Eli Zaretskii <eliz@gnu.org>, npostavs@gmail.com, emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 18 May 2021 19:01:34 +0200
> 
>  > What was the problem you encountered when you tried to add:
>  >
>  >      if (<dirty>) window_updeco_window (...);
>  >
>  > to the getters of the realized values?
> 
> There are too many implicit "realized" values to make this work nicely.
> Think of macros like WINDOW_BODY_PIXEL_WIDTH, WINDOW_BOX_LEFT_EDGE_X,
> WINDOW_BOX_RIGHT_EDGE_X, WINDOW_SCROLL_BAR_X or functions like
> window_box_height in addition to the things window_updeco_window sets
> directly.

I don't think I understand this reasoning.  Changes in these values
are only visible as result of the next redisplay cycle, no?  So in
effect these values "wait" for the next redisplay anyway, right?

> Right.  But note that I implicitly call window_updeco_window also
> whenever I change a window's size, for example, from adjust_frame_size
> after a frame got resized.

I think Stefan asks why do we need to do that.  Why not wait for when
these values are needed by redisplay, and calculate them only then?

Re: "after" variable watchers

[martin rudalics]

 >> There are too many implicit "realized" values to make this work nicely.
 >> Think of macros like WINDOW_BODY_PIXEL_WIDTH, WINDOW_BOX_LEFT_EDGE_X,
 >> WINDOW_BOX_RIGHT_EDGE_X, WINDOW_SCROLL_BAR_X or functions like
 >> window_box_height in addition to the things window_updeco_window sets
 >> directly.
 >
 > I don't think I understand this reasoning.  Changes in these values
 > are only visible as result of the next redisplay cycle, no?  So in
 > effect these values "wait" for the next redisplay anyway, right?

window_box_height or its moral equivalents like `window-inside-edges'
should be accessible before that, so a user or an application can make
layout decisions immediately after it has set a decoration.  Currently,
it can do that when invoking `set-window-fringes' or setting the
`left-fringe' parameter of the window's frame.  It cannot do that when
setting `left-fringe-width' of the window's buffer unless it also does a
`set-window-buffer' right away.

 >> Right.  But note that I implicitly call window_updeco_window also
 >> whenever I change a window's size, for example, from adjust_frame_size
 >> after a frame got resized.
 >
 > I think Stefan asks why do we need to do that.  Why not wait for when
 > these values are needed by redisplay, and calculate them only then?

If a specified value does not fit, we should be able react in different
ways:

- When an application asks for a window size that is not large enough to
   accommodate its decorations, we should be able to reject that request
   and signal an error.

- When the WM shrinks our frame so that a particular window is not large
   enough to include all of its decorations, we have to comply and do
   something reasonable to make that window display its contents orderly.

The display engine is not able to distinguish these cases.  And even if
it were, how should it react in the first case?

martin

Re: "after" variable watchers

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org, monnier@iro.umontreal.ca, npostavs@gmail.com
> From: martin rudalics <rudalics@gmx.at>
> Date: Mon, 24 May 2021 10:48:29 +0200
> 
>  > these values are needed by redisplay, and calculate them only then?
> 
> If a specified value does not fit, we should be able react in different
> ways:
> 
> - When an application asks for a window size that is not large enough to
>    accommodate its decorations, we should be able to reject that request
>    and signal an error.
> 
> - When the WM shrinks our frame so that a particular window is not large
>    enough to include all of its decorations, we have to comply and do
>    something reasonable to make that window display its contents orderly.
> 
> The display engine is not able to distinguish these cases.  And even if
> it were, how should it react in the first case?

I believe the idea was to run the same function(s) you intend to call
from the watcher at redisplay time.  So if your functions are able to
figure this out, so will redisplay.

Re: "after" variable watchers

[Matt Armstrong]

martin rudalics <rudalics@gmx.at> writes:

> 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.
>
> 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.
>
> Objections, suggestions, comments?

I haven't seen vary many truly useful designs based on such a mechanism.
It would be good to understand why.

I think the basic problem is that the "deeply nested fashion" you
mention is often quite complex, and many times ends up depending on
other variables that are not yet modified.

Consider your example: what if the new value for `right-margin-width'
makes sense only after some other variables are also changed.  Say,
variables controlling fringe or scroll bar width, etc.  If those
variables are set by code after `right-margin-width' then the "valid
right-margin-width" predicate would reject the value even though it will
soon be valid.

What we end up with is a situation where not only the final state must
be valid (from the point of view of the watch function), but all
transient states too.  I have personally found systems like this to be
difficult to deal with.

The solution I most often I see is designs based on "dirty" state, where
some point in the future code is triggered to validate state state.  For
that I think the current watch mechanism is sufficient to track which
variables have changed?

Re: "after" variable watchers

[martin rudalics]

 > I haven't seen vary many truly useful designs based on such a mechanism.
 > It would be good to understand why.
 >
 > I think the basic problem is that the "deeply nested fashion" you
 > mention is often quite complex, and many times ends up depending on
 > other variables that are not yet modified.
 >
 > Consider your example: what if the new value for `right-margin-width'
 > makes sense only after some other variables are also changed.  Say,
 > variables controlling fringe or scroll bar width, etc.  If those
 > variables are set by code after `right-margin-width' then the "valid
 > right-margin-width" predicate would reject the value even though it will
 > soon be valid.

No sane value will be ever rejected any more.  That's one of the major
clues of the new design.  Rather, the value will be stored as "nominal"
internally and get "realized" whenever that's possible.

 > What we end up with is a situation where not only the final state must
 > be valid (from the point of view of the watch function), but all
 > transient states too.

Right, if "valid" means "sane".  A `right-margin-width' of 500 columns
will be valid even if will never fit on your frame.

 > I have personally found systems like this to be
 > difficult to deal with.

A user will not have to care about transient and final states.  Every
state must be valid and none is the final one.  After you're through
with the changes cited above you may want to resize your frame and you
will end up in the next transient state.

 > The solution I most often I see is designs based on "dirty" state, where
 > some point in the future code is triggered to validate state state.  For
 > that I think the current watch mechanism is sufficient to track which
 > variables have changed?

Please read my answer to Eli.  Maybe it clarifies things a bit.  In
fact, the "after" variable watcher is needed to produce a "valid
transient state" which can be either displayed immediately or kept
pending until the next transient state has been produced.

martin