GNU bug report logs - #24923
Lisp watchpoints

Previous Next

Package: emacs;

Reported by: npostavs <at> users.sourceforge.net

Date: Fri, 11 Nov 2016 03:11:02 UTC

Severity: wishlist

Tags: fixed, patch

Done: npostavs <at> users.sourceforge.net

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 24923 in the body.
You can then email your comments to 24923 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Fri, 11 Nov 2016 03:11:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to npostavs <at> users.sourceforge.net:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Fri, 11 Nov 2016 03:11:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: bug-gnu-emacs <at> gnu.org
Subject: 25.1; Lisp watchpoints
Date: Thu, 10 Nov 2016 22:10:38 -0500
[Message part 1 (text/plain, inline)]
severity: wishlist
tags: patch

Continuing from
https://lists.nongnu.org/archive/html/emacs-devel/2015-11/msg01437.html,
main code difference since then is that I use a subr instead of indexing
into a table of C functions (the reason for using an index was to avoid
GC on Ffuncall; instead I did that by checking if the function is SUBRP
and doing funcall_subr on it (a new function extracted from Ffuncall)).

I've also added tests, which turned up some interesting corner cases
regarding aliases, and also that I had completely missed
kill-local-variables.

I still need to write something up in the manual.

[v4-0001-Add-lisp-watchpoints.patch (text/plain, attachment)]
[v4-0001-Add-lisp-watchpoints.patch (text/plain, attachment)]
[v4-0002-Add-function-to-trigger-debugger-on-variable-writ.patch (text/plain, attachment)]
[v4-0003-Ensure-redisplay-using-variable-watcher.patch (text/plain, attachment)]
[v4-0004-Add-tests-for-watchpoints.patch (text/plain, attachment)]
[v4-0005-etc-NEWS-Add-entry-for-watchpoints.patch (text/plain, attachment)]

Changed bug title to 'Lisp watchpoints' from '25.1; Lisp watchpoints' Request was from npostavs <at> users.sourceforge.net to control <at> debbugs.gnu.org. (Fri, 11 Nov 2016 03:14:02 GMT) Full text and rfc822 format available.

bug No longer marked as found in versions 25.1. Request was from npostavs <at> users.sourceforge.net to control <at> debbugs.gnu.org. (Fri, 11 Nov 2016 03:17:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Fri, 11 Nov 2016 10:03:02 GMT) Full text and rfc822 format available.

Message #12 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: npostavs <at> users.sourceforge.net
Cc: 24923 <at> debbugs.gnu.org
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Fri, 11 Nov 2016 12:02:42 +0200
> From: npostavs <at> users.sourceforge.net
> Date: Thu, 10 Nov 2016 22:10:38 -0500
> 
> Continuing from
> https://lists.nongnu.org/archive/html/emacs-devel/2015-11/msg01437.html,
> main code difference since then is that I use a subr instead of indexing
> into a table of C functions (the reason for using an index was to avoid
> GC on Ffuncall; instead I did that by checking if the function is SUBRP
> and doing funcall_subr on it (a new function extracted from Ffuncall)).

Thanks.  A few comments below.

One general comment is that these patches are harder to review due to
whitespace changes TAB->spaces.  How about using some non-default
options to "git diff" when generating the patch?

> >From 0507854b885681c2e57bb1c6cb97f898417bd99c Mon Sep 17 00:00:00 2001

This changeset was attached twice, for some reason.

> --- a/src/data.c
> +++ b/src/data.c
> @@ -649,9 +649,10 @@ DEFUN ("makunbound", Fmakunbound, Smakunbound, 1, 1, 0,
>    (register Lisp_Object symbol)
>  {
>    CHECK_SYMBOL (symbol);
> -  if (SYMBOL_CONSTANT_P (symbol))
> +  if (XSYMBOL (symbol)->trapped_write == SYMBOL_NOWRITE)
>      xsignal1 (Qsetting_constant, symbol);
> -  Fset (symbol, Qunbound);
> +  else
> +    Fset (symbol, Qunbound);
>    return symbol;

Why was this needed?  Doesn't xsignal1 never return anymore?

> +static void
> +reset_symbol_trapped_write (Lisp_Object symbol)
> +{
> +  set_symbol_trapped_write (symbol, SYMBOL_TRAPPED_WRITE);
> +}

Suggest to find a better name for this function, like
restore_symbol_trapped_write, for example.  Calling an action that
sets an attribute "reset" makes it harder to read the code.

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

I think the doc string needs to mention that the function also affects
all of SYMBOL's aliases.

> +DEFUN ("remove-variable-watcher", Fremove_variable_watcher, Sremove_variable_watcher,
> +       2, 2, 0,
> +       doc: /* Undo the effect of `add-variable-watcher'.  */)

The doc string should mention SYMBOL.  Also, preferably have the doc
string be self-contained, since that's easy in this case.

> +  (Lisp_Object symbol, Lisp_Object watch_function)
> +{
> +  symbol = Findirect_variable (symbol);
> +  Lisp_Object watchers = Fget (symbol, Qwatchers);
> +  watchers = Fdelete (watch_function, watchers);
> +  if (NILP (watchers))
> +    {
> +      set_symbol_trapped_write (symbol, SYMBOL_UNTRAPPED_WRITE);
> +      map_obarray (Vobarray, harmonize_variable_watchers, symbol);
> +    }
> +  Fput (symbol, Qwatchers, watchers);
> +  return Qnil;

Would it be more useful for this and add-variable-watcher to return
the list of watchers instead?

> +  /* Avoid recursion.  */
> +  set_symbol_trapped_write (symbol, SYMBOL_UNTRAPPED_WRITE);
> +  ptrdiff_t count = SPECPDL_INDEX ();
> +  record_unwind_protect (reset_symbol_trapped_write, symbol);

I think record_unwind_protect should be called before setting the
attribute, for this code to be safer and more future-proof.

> +/* Value is non-zero if symbol cannot be changed through a simple set,
> +   i.e. it's a constant (e.g. nil, t, :keywords), or it has some
> +   watching functions.  */
>  
>  INLINE int
> -(SYMBOL_CONSTANT_P) (Lisp_Object sym)
> +(SYMBOL_TRAPPED_WRITE_P) (Lisp_Object sym)
>  {
> -  return lisp_h_SYMBOL_CONSTANT_P (sym);
> +  return lisp_h_SYMBOL_TRAPPED_WRITE_P (sym);
>  }
>  
>  /* Placeholder for make-docfile to process.  The actual symbol
> @@ -3289,6 +3299,12 @@ set_symbol_next (Lisp_Object sym, struct Lisp_Symbol *next)
>    XSYMBOL (sym)->next = next;
>  }
>  
> +INLINE void
> +make_symbol_constant (Lisp_Object sym)
> +{
> +  XSYMBOL (sym)->trapped_write = SYMBOL_NOWRITE;
> +}
> +

So we will have make_symbol_constant, but no SYMBOL_CONSTANT_P?  Isn't
that confusing?  Some C code may wish to know whether a symbol is
constant, not just either constant or trap-on-write; why not give them
what they want?

> +      ;; Watchpoint triggered.
> +      ((and `watchpoint (let `(,symbol ,newval . ,details) (cdr args)))
> +       (insert
> +        "--"
> +        (pcase details
> +          (`(makunbound ,_) (format "Making %s void" symbol))
> +          (`(let ,_) (format "let-binding %s to %S" symbol newval))
> +          (`(unlet ,_) (format "ending let-binding of %s" symbol))
> +          (`(set nil) (format "setting %s to %S" symbol newval))
> +          (`(set ,buffer) (format "setting %s in %s to %S"
                                     ^^^^^^^^^^^^^^^^^^^^^^^^
IMO, this should include the word "buffer", otherwise it can be
confusing.

> +          (_ (format "watchpoint triggered %S" (cdr args))))

Can you give a couple of examples of this, with %S shown explicitly?
I'm not sure whether the result will be self-explanatory.

> +;;;###autoload
> +(defun debug-watch (variable)
> +  (interactive
> +   (let* ((var-at-point (variable-at-point))
> +          (var (and (symbolp var-at-point) var-at-point))
> +          (val (completing-read
> +                (concat "Debug when setting variable"
> +                        (if var (format " (default %s): " var) ": "))

I think all the other commands/variables similar to this one are named
debug-on-SOMETHING, so perhaps this one should also have such a name.
Like debug-on-setting-variable, perhaps?

> +** New function `add-variable-watcher' can be used to execute code
                                                      ^^^^^^^^^^^^^^^
It is better to say "to call a function" here.

Thanks again for working on this.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sat, 12 Nov 2016 04:34:01 GMT) Full text and rfc822 format available.

Message #15 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24923 <at> debbugs.gnu.org
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Fri, 11 Nov 2016 23:34:33 -0500
Eli Zaretskii <eliz <at> gnu.org> writes:
> One general comment is that these patches are harder to review due to
> whitespace changes TAB->spaces.  How about using some non-default
> options to "git diff" when generating the patch?

Will do for the next one.

>
>> >From 0507854b885681c2e57bb1c6cb97f898417bd99c Mon Sep 17 00:00:00 2001
>
> This changeset was attached twice, for some reason.

Oops, I was trying to insert all the attachments at once in gnus via
some text manipulation and I forgot to delete the first one I copied
from.

>
>> --- a/src/data.c
>> +++ b/src/data.c
>> @@ -649,9 +649,10 @@ DEFUN ("makunbound", Fmakunbound, Smakunbound, 1, 1, 0,
>>    (register Lisp_Object symbol)
>>  {
>>    CHECK_SYMBOL (symbol);
>> -  if (SYMBOL_CONSTANT_P (symbol))
>> +  if (XSYMBOL (symbol)->trapped_write == SYMBOL_NOWRITE)
>>      xsignal1 (Qsetting_constant, symbol);
>> -  Fset (symbol, Qunbound);
>> +  else
>> +    Fset (symbol, Qunbound);
>>    return symbol;
>
> Why was this needed?  Doesn't xsignal1 never return anymore?

Hmm, I'm not sure why I did this.  I think this whole hunk can be
dropped (as long as SYMBOL_CONSTANT_P is preserved as suggested below).


>
>> +  (Lisp_Object symbol, Lisp_Object watch_function)
>> +{
>> +  symbol = Findirect_variable (symbol);
>> +  Lisp_Object watchers = Fget (symbol, Qwatchers);
>> +  watchers = Fdelete (watch_function, watchers);
>> +  if (NILP (watchers))
>> +    {
>> +      set_symbol_trapped_write (symbol, SYMBOL_UNTRAPPED_WRITE);
>> +      map_obarray (Vobarray, harmonize_variable_watchers, symbol);
>> +    }
>> +  Fput (symbol, Qwatchers, watchers);
>> +  return Qnil;
>
> Would it be more useful for this and add-variable-watcher to return
> the list of watchers instead?

I don't think it would be especially useful, but it's easy to do so.

>
>> +/* Value is non-zero if symbol cannot be changed through a simple set,
>> +   i.e. it's a constant (e.g. nil, t, :keywords), or it has some
>> +   watching functions.  */
>>  
>>  INLINE int
>> -(SYMBOL_CONSTANT_P) (Lisp_Object sym)
>> +(SYMBOL_TRAPPED_WRITE_P) (Lisp_Object sym)
>>  {
>> -  return lisp_h_SYMBOL_CONSTANT_P (sym);
>> +  return lisp_h_SYMBOL_TRAPPED_WRITE_P (sym);
>>  }
>>  
>>  /* Placeholder for make-docfile to process.  The actual symbol
>> @@ -3289,6 +3299,12 @@ set_symbol_next (Lisp_Object sym, struct Lisp_Symbol *next)
>>    XSYMBOL (sym)->next = next;
>>  }
>>  
>> +INLINE void
>> +make_symbol_constant (Lisp_Object sym)
>> +{
>> +  XSYMBOL (sym)->trapped_write = SYMBOL_NOWRITE;
>> +}
>> +
>
> So we will have make_symbol_constant, but no SYMBOL_CONSTANT_P?  Isn't
> that confusing?  Some C code may wish to know whether a symbol is
> constant, not just either constant or trap-on-write; why not give them
> what they want?

I suppose I took Stefan's advice to replace SYMBOL_CONSTANT_P with
SYMBOL_TRAPPED_WRITE_P too literally.  Although it's almost always the
case that C code checking if a symbol is constant wants to write to the
symbol, in which case it should be doing a 3-way switch (writable,
trapped write, or constant).  But there is one exception in Fmakunbound
(mentioned above), so I guess we may as well keep it.

>
>> +          (_ (format "watchpoint triggered %S" (cdr args))))
>
> Can you give a couple of examples of this, with %S shown explicitly?
> I'm not sure whether the result will be self-explanatory.

You mean examples of this this clause being used?  It was meant more as
a catchall in case some watch types were missed by the previous clauses.
It shouldn't really ever happen unless the debugger and watchpoint code
get out of sync.  Do you think it would be better to just signal an
error?  (although would signalling an error while the debugger is
invoked cause trouble?)

>
>> +;;;###autoload
>> +(defun debug-watch (variable)
>> +  (interactive
>> +   (let* ((var-at-point (variable-at-point))
>> +          (var (and (symbolp var-at-point) var-at-point))
>> +          (val (completing-read
>> +                (concat "Debug when setting variable"
>> +                        (if var (format " (default %s): " var) ": "))
>
> I think all the other commands/variables similar to this one are named
> debug-on-SOMETHING, so perhaps this one should also have such a name.
> Like debug-on-setting-variable, perhaps?

Hah, I initially called this debug-on-set
(https://lists.nongnu.org/archive/html/emacs-devel/2015-11/txtyjJDztIULG.txt),
and then you suggested debug-watch instead
(https://lists.nongnu.org/archive/html/emacs-devel/2015-11/msg02017.html).

An alias probably makes sense, how about debug-on-variable-change?
(since it catches some changes other than `set'.)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sat, 12 Nov 2016 07:20:01 GMT) Full text and rfc822 format available.

Message #18 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: npostavs <at> users.sourceforge.net
Cc: 24923 <at> debbugs.gnu.org
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sat, 12 Nov 2016 09:19:21 +0200
> From: npostavs <at> users.sourceforge.net
> Cc: 24923 <at> debbugs.gnu.org
> Date: Fri, 11 Nov 2016 23:34:33 -0500
> 
> >> +          (_ (format "watchpoint triggered %S" (cdr args))))
> >
> > Can you give a couple of examples of this, with %S shown explicitly?
> > I'm not sure whether the result will be self-explanatory.
> 
> You mean examples of this this clause being used?  It was meant more as
> a catchall in case some watch types were missed by the previous clauses.
> It shouldn't really ever happen unless the debugger and watchpoint code
> get out of sync.  Do you think it would be better to just signal an
> error?  (although would signalling an error while the debugger is
> invoked cause trouble?)

Either signal an error, or include something like "(please submit a
bug report)" in the text.

> >> +;;;###autoload
> >> +(defun debug-watch (variable)
> >> +  (interactive
> >> +   (let* ((var-at-point (variable-at-point))
> >> +          (var (and (symbolp var-at-point) var-at-point))
> >> +          (val (completing-read
> >> +                (concat "Debug when setting variable"
> >> +                        (if var (format " (default %s): " var) ": "))
> >
> > I think all the other commands/variables similar to this one are named
> > debug-on-SOMETHING, so perhaps this one should also have such a name.
> > Like debug-on-setting-variable, perhaps?
> 
> Hah, I initially called this debug-on-set
> (https://lists.nongnu.org/archive/html/emacs-devel/2015-11/txtyjJDztIULG.txt),
> and then you suggested debug-watch instead
> (https://lists.nongnu.org/archive/html/emacs-devel/2015-11/msg02017.html).
> 
> An alias probably makes sense, how about debug-on-variable-change?
> (since it catches some changes other than `set'.)

Fine with me, thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sun, 13 Nov 2016 00:54:02 GMT) Full text and rfc822 format available.

Message #21 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24923 <at> debbugs.gnu.org
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sat, 12 Nov 2016 19:54:01 -0500
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: npostavs <at> users.sourceforge.net
>> Cc: 24923 <at> debbugs.gnu.org
>> Date: Fri, 11 Nov 2016 23:34:33 -0500
>> 
>> >> +          (_ (format "watchpoint triggered %S" (cdr args))))
>> >
>> > Can you give a couple of examples of this, with %S shown explicitly?
>> > I'm not sure whether the result will be self-explanatory.
>> 
>> You mean examples of this this clause being used?  It was meant more as
>> a catchall in case some watch types were missed by the previous clauses.
>> It shouldn't really ever happen unless the debugger and watchpoint code
>> get out of sync.  Do you think it would be better to just signal an
>> error?  (although would signalling an error while the debugger is
>> invoked cause trouble?)
>
> Either signal an error, or include something like "(please submit a
> bug report)" in the text.

Here is the updated patch, created with -b.  I went with a call to
`error'.  And actually, I had missed a couple of watchpoint types.

[v5-b-0001-Add-lisp-watchpoints.patch (text/plain, attachment)]
[v5-b-0002-Add-function-to-trigger-debugger-on-variable-writ.patch (text/plain, attachment)]
[v5-b-0003-Ensure-redisplay-using-variable-watcher.patch (text/plain, attachment)]
[v5-b-0004-Add-tests-for-watchpoints.patch (text/plain, attachment)]
[v5-b-0005-etc-NEWS-Add-entry-for-watchpoints.patch (text/plain, attachment)]
[Message part 7 (text/plain, inline)]
For the manual, do you think I should document just the debugging
commands, or should there additionally be a section in the "Variables"
chapter about the watchpoint mechanism?

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sun, 13 Nov 2016 15:30:02 GMT) Full text and rfc822 format available.

Message #24 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: npostavs <at> users.sourceforge.net
Cc: 24923 <at> debbugs.gnu.org
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sun, 13 Nov 2016 17:29:54 +0200
> From: npostavs <at> users.sourceforge.net
> Cc: 24923 <at> debbugs.gnu.org
> Date: Sat, 12 Nov 2016 19:54:01 -0500
> 
> Here is the updated patch, created with -b.  I went with a call to
> `error'.  And actually, I had missed a couple of watchpoint types.

This LGTM, just one comment for when you actually push:

> +  else if (sym->redirect == SYMBOL_LOCALIZED &&
> +           SYMBOL_BLV (sym)->frame_local)

Our coding conventions put the logical operators at the beginning of a
line, not at EOL.

> +static void
> +harmonize_variable_watchers (Lisp_Object alias, Lisp_Object base_variable)
> +{
> +  if (!EQ (base_variable, alias) &&
> +      EQ (base_variable, Findirect_variable (alias)))

Same here.

> +  if (NILP (where) &&
> +      !EQ (operation, Qset_default) && !EQ (operation, Qmakunbound) &&
> +      !NILP (Flocal_variable_if_set_p (symbol, Fcurrent_buffer ())))

And here.

> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -626,6 +626,10 @@ two objects are 'eq' ('eql'), then the result of 'sxhash-eq'
>  consistency with the new functions.  For compatibility, 'sxhash'
>  remains as an alias to 'sxhash-equal'.
>  
> +** New function `add-variable-watcher' can be used to call a function
> +when a symbol's value is changed.  This is used to implement the new
> +debugger command `debug-watch'.
                     ^^^^^^^^^^^
This should follow the renaming.

(Hopefully, this will be followed by a suitable Edebug binding.)

> For the manual, do you think I should document just the debugging
> commands, or should there additionally be a section in the "Variables"
> chapter about the watchpoint mechanism?

Both, I think.

Thanks, I think this is a very important new feature.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sun, 20 Nov 2016 02:12:01 GMT) Full text and rfc822 format available.

Message #27 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24923 <at> debbugs.gnu.org
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sat, 19 Nov 2016 21:12:13 -0500
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>
> Our coding conventions put the logical operators at the beginning of a
> line, not at EOL.

Fixed this, and added documentation.  Also, watcher functions are now
listed in describe-variable output.

Does it make sense to mention the use of the `watchers' symbol property
in the manual?  Since I've added a `get-variable-watchers' it's now
possible to ignore the symbol property as an implementation detail.

[v6-0001-Add-lisp-watchpoints.patch (text/plain, attachment)]
[v6-0002-Show-watchpoints-when-describing-variables.patch (text/plain, attachment)]
[v6-0003-Add-function-to-trigger-debugger-on-variable-writ.patch (text/plain, attachment)]
[v6-0004-Ensure-redisplay-using-variable-watcher.patch (text/plain, attachment)]
[v6-0005-Add-tests-for-watchpoints.patch (text/plain, attachment)]
[v6-0006-Document-watchpoints.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sun, 20 Nov 2016 10:51:01 GMT) Full text and rfc822 format available.

Message #30 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: npostavs <at> users.sourceforge.net
Cc: 24923 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sun, 20 Nov 2016 11:49:49 +0100
There are a few typos in the documentation:

On Sat, 19 Nov 2016 21:12:13 -0500 npostavs <at> users.sourceforge.net wrote:

> From c318be2dc401d2f3b958ceb3b48e466a3019091e Mon Sep 17 00:00:00 2001
> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Thu, 19 Nov 2015 19:50:06 -0500
> Subject: [PATCH v6 1/6] Add lisp watchpoints
>
> This allows to call a function whenever a symbol-value is changed.
              ^^^^^^^
              calling

> From 5d87668684319bb165ed0f31f637c44cda5716a6 Mon Sep 17 00:00:00 2001
> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Sat, 21 Nov 2015 17:02:42 -0500
> Subject: [PATCH v6 4/6] Ensure redisplay using variable watcher
>
> Instead of looking up the variable name in redisplay--variables when
> setting.

Incomplete sentence? Or if it's the continuation of the Subject: line,
shouldn't it be lowercase?

> From 984109b9b204c82ce2e6482210425a70b7b7e867 Mon Sep 17 00:00:00 2001
> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Sun, 13 Dec 2015 14:47:58 -0500
> Subject: [PATCH v6 6/6] Document watchpoints
>
> * doc/lispref/debugging.texi (Variable Debugging):
> * doc/lispref/variables.texi (Watching Variables): New section.
> * etc/NEWS: Add entry for watchpoints
> ---
>  doc/lispref/debugging.texi | 32 +++++++++++++++++++++++
>  doc/lispref/variables.texi | 63 ++++++++++++++++++++++++++++++++++++++++++++++
>  etc/NEWS                   |  5 ++++
>  src/data.c                 |  9 +++++++
>  4 files changed, 109 insertions(+)
>
> diff --git a/doc/lispref/debugging.texi b/doc/lispref/debugging.texi
> index 6c0908a..8cae203 100644
> --- a/doc/lispref/debugging.texi
> +++ b/doc/lispref/debugging.texi
[...]
> @@ -290,6 +291,37 @@ Function Debugging
>  not currently set up to break on entry.
>  @end deffn
>  
> +@node Variable Debugging
> +@subsection Entering the debugger when a variable is modified
> +@cindex variable write debugging
> +@cindex debugging changes to variables
> +
> +Sometimes a problem with a function is due to a wrong setting of a
> +variable.  Setting up the debugger to trigger whenever the variable is
> +changed is quick way to find the origin of the setting.
             ^
             a

> +
> +@deffn Command debug-on-variable-change variable
> +This function arranges causes the debugger to be called whenever
                 ^^^^^^^^^^^^^^^
   either "arranges for" or "causes"

> diff --git a/doc/lispref/variables.texi b/doc/lispref/variables.texi
> index 418a416..07f787c 100644
> --- a/doc/lispref/variables.texi
> +++ b/doc/lispref/variables.texi
[...]
> @@ -765,6 +766,68 @@ Setting Variables
>  @end example
>  @end defun
>  
> +@node Watching Variables
> +@section Running a function when a variable is changed.
> +@cindex variable watchpoints
> +
> +It is sometimes useful to take some action when a variable changes its
> +value.  The watchpoint facility provides the means to do so.  Some
> +possible uses for this feature include keeping display in sync with
> +variable settings, and invoking the debugger to track down unexpected
> +changes to variables @pxref{Variable Debugging}.
> +
> +Each variable has a list of watch functions stored in its
> +@code{watchers} symbol property, @xref{Symbol Properties}.  However,
> +for efficiency reasons, the list is only consulted if symbol is marked
                                                        ^
                                                       the

> +as watched.  Therefore, the watch function list should only be
> +manipulated by the following functions, which take care of the
> +symbol's watched status in addition to the property value.
> +
> +@defun add-variable-watcher symbol watch-function
> +This function arranges for @var{watch-function} to be called whenever
> +@var{symbol} (or any of its aliases @pxref{Variable Aliases}) are
> +modified.
> +
> +It will be called with 4 arguments: (@var{symbol} @var{newval}
> +@var{operation} @var{where}).
> +
> +@var{symbol} is the variable being changed.
> +@var{newval} is the value it will be changed to.
> +@var{operation} is a symbol representing the kind of change, one of:
> +`set', `let', `unlet', `makunbound', and `defvaralias'.
> +@var{where} is a buffer if the buffer-local value of the variable
                                                                     ^
                                                                     is
> +being changed, nil otherwise.
> +@end defun
> +
> +@defun remove-variable-watch symbol watch-function
> +This function removes @var{watch-function} from @var{symbol}'s list of
> +watchers.
> +@end defun
> +
> +@defun get-variable-watchers symbol
> +This function returns the list of active watcher functions.
> +@end defun
> +
> +@subsection Limitations
> +
> +There are a couple of ways in which a variable could be modifed (or at
> +least appear to be modified) without triggering a watchpoint.
> +
> +Since the watchpoint are attached to symbols, modification to the
         ^^^^^^^^^^^^^^
          watchpoints

> +objects contained within variables (e.g., by a list modification
> +function @pxref{Modifying Lists}) is not caught by this mechanism.
> +
> +Additionally, C code can modify the value of variables directly,
> +bypassing the watchpoint mechanism.
> +
> +A minor limitation of this feature, again because it targets symbols,
> +is that only variables of dynamic scope may be watched.  This poses
> +little difficulty, since modifications to lexical variables can be
> +discovered easily by inspecting the code within the scope of the
> +variable (unlike dynamic variables which can be modified by any code
                                     ^
                                     , [comma]

> +at all, @pxref{Variable Scoping}).
> +
> +
>  @node Variable Scoping
>  @section Scoping Rules for Variable Bindings
>  @cindex scoping rule
[...]
> diff --git a/src/data.c b/src/data.c
> index ff35315..ef6b48b 100644
> --- a/src/data.c
> +++ b/src/data.c
> @@ -1428,6 +1428,15 @@ harmonize_variable_watchers (Lisp_Object alias, Lisp_Object base_variable)
>  DEFUN ("add-variable-watcher", Fadd_variable_watcher, Sadd_variable_watcher,
>         2, 2, 0,
>         doc: /* Cause WATCH-FUNCTION to be called when SYMBOL is set.
> +
> +It will be called with 4 arguments: (SYMBOL NEWVAL OPERATION WHERE).
> +SYMBOL is the variable being changed.
> +NEWVAL is the value it will be changed to.
> +OPERATION is a symbol representing the kind of change, one of: `set',
> +`let', `unlet', `makunbound', and `defvaralias'.
> +WHERE is a buffer if the buffer-local value of the variable being
                                                              ^
                                                              is

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

Steve Berman




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sun, 20 Nov 2016 14:14:01 GMT) Full text and rfc822 format available.

Message #33 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: 24923 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sun, 20 Nov 2016 09:14:02 -0500
[Message part 1 (text/plain, inline)]
Stephen Berman <stephen.berman <at> gmx.net> writes:

> There are a few typos in the documentation:

Thanks, reattaching just the fixed documentation commit.

[v6.1-0006-Document-watchpoints.patch (text/plain, attachment)]
[Message part 3 (text/plain, inline)]

>
>> From 5d87668684319bb165ed0f31f637c44cda5716a6 Mon Sep 17 00:00:00 2001
>> From: Noam Postavsky <npostavs <at> gmail.com>
>> Date: Sat, 21 Nov 2015 17:02:42 -0500
>> Subject: [PATCH v6 4/6] Ensure redisplay using variable watcher
>>
>> Instead of looking up the variable name in redisplay--variables when
>> setting.
>
> Incomplete sentence? Or if it's the continuation of the Subject: line,
> shouldn't it be lowercase?

I sort of about it as a continuation, but it looks a bit weird to make
it an official continuation when there's a blank line in between.  I
rephrased:

    This replaces checking for the variable name in redisplay--variables
    when setting it.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sun, 20 Nov 2016 15:59:02 GMT) Full text and rfc822 format available.

Message #36 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: npostavs <at> users.sourceforge.net
Cc: 24923 <at> debbugs.gnu.org
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sun, 20 Nov 2016 17:58:17 +0200
> From: npostavs <at> users.sourceforge.net
> Cc: 24923 <at> debbugs.gnu.org
> Date: Sat, 19 Nov 2016 21:12:13 -0500
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> > Our coding conventions put the logical operators at the beginning of a
> > line, not at EOL.
> 
> Fixed this, and added documentation.  Also, watcher functions are now
> listed in describe-variable output.

Thanks.

> Does it make sense to mention the use of the `watchers' symbol property
> in the manual?  Since I've added a `get-variable-watchers' it's now
> possible to ignore the symbol property as an implementation detail.

I think the property can indeed be left undocumented.

> @@ -1233,13 +1233,14 @@ DEFUN ("set", Fset, Sset, 2, 2, 0,
>     If buffer/frame-locality is an issue, WHERE specifies which context to use.
>     (nil stands for the current buffer/frame).
>  
> -   If BINDFLAG is false, then if this symbol is supposed to become
> -   local in every buffer where it is set, then we make it local.
> -   If BINDFLAG is true, we don't do that.  */
> +   If BINDFLAG is SET_INTERNAL_SET, then if this symbol is supposed to
> +   become local in every buffer where it is set, then we make it
> +   local.  If BINDFLAG is SET_INTERNAL_BIND or SET_INTERNAL_UNBIND, we
> +   don't do that.  */

What are those SET_INTERNAL_* values?  They are numbers, right?  Then
they should be described as such in the doc string.

> +(defun cancel-debug-on-variable-change (&optional variable)
> +  "Undo effect of \\[debug-on-entry] on VARIABLE.
                     ^^^^^^^^^^^^^^^^^^
Copy/paste error.

I will comment on the documentation in a separate message.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sun, 20 Nov 2016 16:12:02 GMT) Full text and rfc822 format available.

Message #39 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: npostavs <at> users.sourceforge.net
Cc: 24923 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sun, 20 Nov 2016 18:11:43 +0200
> From: npostavs <at> users.sourceforge.net
> Cc: 24923 <at> debbugs.gnu.org,  Eli Zaretskii <eliz <at> gnu.org>
> Date: Sun, 20 Nov 2016 09:14:02 -0500
> 
> > There are a few typos in the documentation:
> 
> Thanks, reattaching just the fixed documentation commit.
> diff --git a/doc/lispref/debugging.texi b/doc/lispref/debugging.texi
> index 6c0908a..c047d45 100644
> --- a/doc/lispref/debugging.texi
> +++ b/doc/lispref/debugging.texi
> @@ -69,6 +69,7 @@ Debugger
>  * Error Debugging::       Entering the debugger when an error happens.
>  * Infinite Loops::        Stopping and debugging a program that doesn't exit.
>  * Function Debugging::    Entering it when a certain function is called.
> +* Variable Debugging::    Entering it when a variable is modified.

This additional menu item should be also added to the master menu in
elisp.texi.

> +@deffn Command debug-on-variable-change variable
> +This function arranges for the debugger to be called whenever
> +@var{variable} is modified.
> +
> +It is implemented using the watchpoint mechanism, so it inherits the
> +same characteristics and limitations: all aliases of @var{variable}
> +will be watched together, only dynamic variables can be watched, and
> +changes to the objects referenced by variables are not detected.  For
> +details, see @xref{Watching Variables}.
                ^^^^^
@xref already generates "See", capitalized, so you want @ref here.

> diff --git a/doc/lispref/variables.texi b/doc/lispref/variables.texi
> index 418a416..1e0b098 100644
> --- a/doc/lispref/variables.texi
> +++ b/doc/lispref/variables.texi
> @@ -34,6 +34,7 @@ Variables
>  * Accessing Variables::         Examining values of variables whose names
>                              are known only at run time.
>  * Setting Variables::           Storing new values in variables.
> +* Watching Variables::          Running a function when a variable is changed.

Likewise, this should be added to the master menu.

> +@node Watching Variables
> +@section Running a function when a variable is changed.
> +@cindex variable watchpoints

I'd add here an index entry that begins with "watchpoint", like this:

  @cindex watchpoints for Lisp variables

That's for those who like using completion in Info-index command.

> +variable settings, and invoking the debugger to track down unexpected
> +changes to variables @pxref{Variable Debugging}.

I believe you meant to put @pxref in parentheses.

> +Each variable has a list of watch functions stored in its
> +@code{watchers} symbol property, @xref{Symbol Properties}.
                                    ^^^^^
Either "see @ref" or "@pxref", because @xref generates a capitalized
"See", and so is only appropriate at the beginning of a sentence.

> +@defun add-variable-watcher symbol watch-function
> +This function arranges for @var{watch-function} to be called whenever
> +@var{symbol} (or any of its aliases @pxref{Variable Aliases}) are
                                      ^
> +modified.

A comma is missing here.  Also, I believe "is modified" reads better
here.  Or maybe replace the parentheses with commas, then plural
should be okay, I think.

> +@defun get-variable-watchers symbol
> +This function returns the list of active watcher functions.

Please mention SYMBOL here.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sun, 20 Nov 2016 17:01:02 GMT) Full text and rfc822 format available.

Message #42 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24923 <at> debbugs.gnu.org
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sun, 20 Nov 2016 12:00:52 -0500
Eli Zaretskii <eliz <at> gnu.org> writes:
>> @@ -1233,13 +1233,14 @@ DEFUN ("set", Fset, Sset, 2, 2, 0,
>>     If buffer/frame-locality is an issue, WHERE specifies which context to use.
>>     (nil stands for the current buffer/frame).
>>  
>> -   If BINDFLAG is false, then if this symbol is supposed to become
>> -   local in every buffer where it is set, then we make it local.
>> -   If BINDFLAG is true, we don't do that.  */
>> +   If BINDFLAG is SET_INTERNAL_SET, then if this symbol is supposed to
>> +   become local in every buffer where it is set, then we make it
>> +   local.  If BINDFLAG is SET_INTERNAL_BIND or SET_INTERNAL_UNBIND, we
>> +   don't do that.  */
>
> What are those SET_INTERNAL_* values?  They are numbers, right?  Then
> they should be described as such in the doc string.

They're enum values.  Perhaps you were confused by the hunk header?
That's not a doc string, it's the comment on set_internal, a C function.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sun, 20 Nov 2016 17:26:01 GMT) Full text and rfc822 format available.

Message #45 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: npostavs <at> users.sourceforge.net
Cc: 24923 <at> debbugs.gnu.org
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sun, 20 Nov 2016 19:25:06 +0200
> From: npostavs <at> users.sourceforge.net
> Cc: 24923 <at> debbugs.gnu.org
> Date: Sun, 20 Nov 2016 12:00:52 -0500
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> >> @@ -1233,13 +1233,14 @@ DEFUN ("set", Fset, Sset, 2, 2, 0,
> >>     If buffer/frame-locality is an issue, WHERE specifies which context to use.
> >>     (nil stands for the current buffer/frame).
> >>  
> >> -   If BINDFLAG is false, then if this symbol is supposed to become
> >> -   local in every buffer where it is set, then we make it local.
> >> -   If BINDFLAG is true, we don't do that.  */
> >> +   If BINDFLAG is SET_INTERNAL_SET, then if this symbol is supposed to
> >> +   become local in every buffer where it is set, then we make it
> >> +   local.  If BINDFLAG is SET_INTERNAL_BIND or SET_INTERNAL_UNBIND, we
> >> +   don't do that.  */
> >
> > What are those SET_INTERNAL_* values?  They are numbers, right?  Then
> > they should be described as such in the doc string.
> 
> They're enum values.  Perhaps you were confused by the hunk header?
> That's not a doc string, it's the comment on set_internal, a C function.

Sorry, I thought it was a doc string.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sun, 20 Nov 2016 19:26:01 GMT) Full text and rfc822 format available.

Message #48 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24923 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sun, 20 Nov 2016 14:26:15 -0500
Eli Zaretskii <eliz <at> gnu.org> writes:

>> +changes to the objects referenced by variables are not detected.  For
>> +details, see @xref{Watching Variables}.
>                 ^^^^^
> @xref already generates "See", capitalized, so you want @ref here.
>
>> +Each variable has a list of watch functions stored in its
>> +@code{watchers} symbol property, @xref{Symbol Properties}.
>                                     ^^^^^
> Either "see @ref" or "@pxref", because @xref generates a capitalized
> "See", and so is only appropriate at the beginning of a sentence.

@xref seems to be generating lowercase "see" for me, perhaps because I'm
using makeinfo 4.13?  I'll change to @ref anyway.

>
>> +@defun add-variable-watcher symbol watch-function
>> +This function arranges for @var{watch-function} to be called whenever
>> +@var{symbol} (or any of its aliases @pxref{Variable Aliases}) are
>                                       ^
>> +modified.
>
> A comma is missing here.  Also, I believe "is modified" reads better
> here.  Or maybe replace the parentheses with commas, then plural
> should be okay, I think.

Hmm, maybe rephrasing like this:

    @defun add-variable-watcher symbol watch-function
    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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sun, 20 Nov 2016 19:37:02 GMT) Full text and rfc822 format available.

Message #51 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: npostavs <at> users.sourceforge.net
Cc: 24923 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sun, 20 Nov 2016 21:36:40 +0200
> From: npostavs <at> users.sourceforge.net
> Cc: 24923 <at> debbugs.gnu.org,  stephen.berman <at> gmx.net
> Date: Sun, 20 Nov 2016 14:26:15 -0500
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> +changes to the objects referenced by variables are not detected.  For
> >> +details, see @xref{Watching Variables}.
> >                 ^^^^^
> > @xref already generates "See", capitalized, so you want @ref here.
> >
> >> +Each variable has a list of watch functions stored in its
> >> +@code{watchers} symbol property, @xref{Symbol Properties}.
> >                                     ^^^^^
> > Either "see @ref" or "@pxref", because @xref generates a capitalized
> > "See", and so is only appropriate at the beginning of a sentence.
> 
> @xref seems to be generating lowercase "see" for me, perhaps because I'm
> using makeinfo 4.13?

Unlikely.  Are you looking at the file in Info, or as plain text?  The
former has its own ideas about how to display cross-references; I
meant what is actually in the file.

>     @defun add-variable-watcher symbol watch-function
>     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.

Fine with me, thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sun, 20 Nov 2016 20:17:01 GMT) Full text and rfc822 format available.

Message #54 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24923 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sun, 20 Nov 2016 15:16:49 -0500
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> @xref seems to be generating lowercase "see" for me, perhaps because I'm
>> using makeinfo 4.13?
>
> Unlikely.  Are you looking at the file in Info, or as plain text?  The
> former has its own ideas about how to display cross-references; I
> meant what is actually in the file.

I was looking at the file in Info-mode.  Looking in fundamental-mode, it
seems that @xref generates an uppercase "*Note", where @ref generates
"*note".

Anyway, here is the final(?) patchset:

[v7-0001-Add-lisp-watchpoints.patch (text/plain, attachment)]
[v7-0002-Show-watchpoints-when-describing-variables.patch (text/plain, attachment)]
[v7-0003-Add-function-to-trigger-debugger-on-variable-writ.patch (text/plain, attachment)]
[v7-0004-Ensure-redisplay-using-variable-watcher.patch (text/plain, attachment)]
[v7-0005-Add-tests-for-watchpoints.patch (text/plain, attachment)]
[v7-0006-Document-watchpoints.patch (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Mon, 21 Nov 2016 17:32:02 GMT) Full text and rfc822 format available.

Message #57 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: npostavs <at> users.sourceforge.net
Cc: 24923 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Mon, 21 Nov 2016 19:31:03 +0200
> From: npostavs <at> users.sourceforge.net
> Cc: 24923 <at> debbugs.gnu.org,  stephen.berman <at> gmx.net
> Date: Sun, 20 Nov 2016 15:16:49 -0500
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> >> 
> >> @xref seems to be generating lowercase "see" for me, perhaps because I'm
> >> using makeinfo 4.13?
> >
> > Unlikely.  Are you looking at the file in Info, or as plain text?  The
> > former has its own ideas about how to display cross-references; I
> > meant what is actually in the file.
> 
> I was looking at the file in Info-mode.  Looking in fundamental-mode, it
> seems that @xref generates an uppercase "*Note", where @ref generates
> "*note".

Yes.  @xref produces "See" in the printed output (PDF etc.).

> Anyway, here is the final(?) patchset:

LGTM, thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sat, 03 Dec 2016 01:47:02 GMT) Full text and rfc822 format available.

Message #60 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24923 <at> debbugs.gnu.org, stephen.berman <at> gmx.net
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Fri, 02 Dec 2016 20:47:37 -0500
tags 24923 fixed
close 24923 
quit

Eli Zaretskii <eliz <at> gnu.org> writes:
>
>> Anyway, here is the final(?) patchset:
>
> LGTM, thanks.

Pushed as 88fefc3 (56c8178, e7cd98b, d3faef9, cfd2b9e, 459a234, 2272131).





Added tag(s) fixed. Request was from npostavs <at> users.sourceforge.net to control <at> debbugs.gnu.org. (Sat, 03 Dec 2016 01:47:02 GMT) Full text and rfc822 format available.

bug closed, send any further explanations to 24923 <at> debbugs.gnu.org and npostavs <at> users.sourceforge.net Request was from npostavs <at> users.sourceforge.net to control <at> debbugs.gnu.org. (Sat, 03 Dec 2016 01:47:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sat, 03 Dec 2016 03:50:01 GMT) Full text and rfc822 format available.

Message #67 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Clément Pit--Claudel <clement.pit <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Fri, 2 Dec 2016 22:49:21 -0500
[Message part 1 (text/plain, inline)]
On 2016-12-02 20:47, npostavs <at> users.sourceforge.net wrote:
> tags 24923 fixed
> close 24923 
> quit
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>>> Anyway, here is the final(?) patchset:
>>
>> LGTM, thanks.
> 
> Pushed as 88fefc3 (56c8178, e7cd98b, d3faef9, cfd2b9e, 459a234, 2272131).

This is incredibly cool!  Thanks so much for working on this :)

[signature.asc (application/pgp-signature, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sat, 03 Dec 2016 03:51:01 GMT) Full text and rfc822 format available.

Message #70 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Clément Pit--Claudel <clement.pit <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Fri, 2 Dec 2016 22:50:40 -0500
[Message part 1 (text/plain, inline)]
On 2016-12-02 20:47, npostavs <at> users.sourceforge.net wrote:
> Pushed as 88fefc3 (56c8178, e7cd98b, d3faef9, cfd2b9e, 459a234, 2272131)

I wonder: would it make sense to extend defcustom to use this, too? Wouldn't it be great if I could tag my defcustom variables in some way, and then setq would automatically invoke the corresponding defcustom setter?

[signature.asc (application/pgp-signature, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sat, 03 Dec 2016 05:02:02 GMT) Full text and rfc822 format available.

Message #73 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Colascione <dancol <at> dancol.org>
To: Clément Pit--Claudel <clement.pit <at> gmail.com>
Cc: 24923 <at> debbugs.gnu.org
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Fri, 02 Dec 2016 21:01:06 -0800
On Fri, Dec 02 2016, Clément Pit--Claudel wrote:
> On 2016-12-02 20:47, npostavs <at> users.sourceforge.net wrote:
>> Pushed as 88fefc3 (56c8178, e7cd98b, d3faef9, cfd2b9e, 459a234, 2272131)
>
> I wonder: would it make sense to extend defcustom to use this, too?
> Wouldn't it be great if I could tag my defcustom variables in some
> way, and then setq would automatically invoke the corresponding
> defcustom setter?

setf maybe. setq should not be magical.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#24923; Package emacs. (Sat, 03 Dec 2016 14:11:02 GMT) Full text and rfc822 format available.

Message #76 received at 24923 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Daniel Colascione <dancol <at> dancol.org>
Cc: 24923 <at> debbugs.gnu.org,
 Clément Pit--Claudel <clement.pit <at> gmail.com>
Subject: Re: bug#24923: 25.1; Lisp watchpoints
Date: Sat, 03 Dec 2016 09:11:10 -0500
Daniel Colascione <dancol <at> dancol.org> writes:

> On Fri, Dec 02 2016, Clément Pit--Claudel wrote:
>> On 2016-12-02 20:47, npostavs <at> users.sourceforge.net wrote:
>>> Pushed as 88fefc3 (56c8178, e7cd98b, d3faef9, cfd2b9e, 459a234, 2272131)
>>
>> I wonder: would it make sense to extend defcustom to use this, too?
>> Wouldn't it be great if I could tag my defcustom variables in some
>> way, and then setq would automatically invoke the corresponding
>> defcustom setter?
>
> setf maybe. setq should not be magical.

There were indeed some concerns about that kind usage when this feature
was first proposed, e.g.:

- http://lists.gnu.org/archive/html/emacs-devel/2015-01/msg01064.html

    I like the idea of such hooks (which I've always thought of as
    "watchers" rather than hooks), actually, but only for purposes such
    as debugging.

    If we want to use such hooks for purposes such as "automatically
    recompute values of dependent vars", then I think the right way is to
    introduce a new layer which checks&runs these hooks, using the "raw"
    `setq' underneath.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 01 Jan 2017 12:24:10 GMT) Full text and rfc822 format available.

This bug report was last modified 8 years and 166 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.