Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Alan Mackenzie]

Hello, Emacs.

A few weeks ago, I was attempting to edebug a program which itself
scanned through a buffer B.  That buffer was also displayed in a window
(or possibly two windows).  Each time the program went into edebug, the
point in B got corrupted.

Setting edebug-save-displayed-buffer-points didn't help.

I now understand what was going wrong.  Without
edebug-save-displayed-buffer-points, B's point got set to a window point
from a random window displaying B, caused by a set-window-configuration
call from edebug.

With edebug-save-displayed-buffer-points set, the function
edebug-get-displayed-buffer points was recording window-points for each
displayed window, but after the recursive edit was writing these
window-points back into the buffer points.  This is surely a bug.  At the
least, there is a race condition if two windows display the same buffer.

In neither of the above scenarios does edebug do anything to restore the
buffer points after the recursive edit.  This seems to be a bad thing.

I propose that edebug should get an option to preserve buffer points,
alongside the existing options which preserve window points.

One downside of this would be the runtime involved in the set-buffer
calls needed to call `point' and `goto-char'.  This could make edebug
quite sluggish.  So, why don't we introduce new functions buffer-point
and set-buffer-point?  These would enable edebug to record and restore
these points economically.  Why don't these functions exist already?

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Eli Zaretskii]

> Date: Mon, 31 Oct 2022 11:43:15 +0000
> From: Alan Mackenzie <acm@muc.de>
> 
> A few weeks ago, I was attempting to edebug a program which itself
> scanned through a buffer B.  That buffer was also displayed in a window
> (or possibly two windows).  Each time the program went into edebug, the
> point in B got corrupted.
> 
> Setting edebug-save-displayed-buffer-points didn't help.

Did you try switch-to-buffer-preserve-window-point?

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Alan Mackenzie]

Hello, Eli.

On Mon, Oct 31, 2022 at 15:16:28 +0200, Eli Zaretskii wrote:
> > Date: Mon, 31 Oct 2022 11:43:15 +0000
> > From: Alan Mackenzie <acm@muc.de>

> > A few weeks ago, I was attempting to edebug a program which itself
> > scanned through a buffer B.  That buffer was also displayed in a window
> > (or possibly two windows).  Each time the program went into edebug, the
> > point in B got corrupted.

> > Setting edebug-save-displayed-buffer-points didn't help.

> Did you try switch-to-buffer-preserve-window-point?

I wasn't aware of that variable.  Thanks!  It's quite something to get
your head around.

I don't think it's going to help me, since it's about preserving a
window point.  My problem in edebug is about preserving the _buffer_
point over the recursive edit.

Anyhow, I proposed buffer-point and set-buffer-point.  They would be a
lot faster than set-buffer followed by point and goto-char.  Here is my
first version of these.  What do you think?



diff --git a/src/buffer.c b/src/buffer.c
index b67b989326..34b7b4442f 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1453,6 +1453,48 @@ returns the symbol `autosaved'.  */)
     return Qnil;
 }
 
+DEFUN ("buffer-point", Fbuffer_point, Sbuffer_point, 1, 1, 0,
+       doc: /* Return the buffer point of BUFFER-OR-NAME.
+The argument may be a buffer or the name of an existing buffer.  */)
+  (Lisp_Object buffer_or_name)
+{
+  Lisp_Object buffer;
+  struct buffer *b;
+
+  buffer = Fget_buffer (buffer_or_name);
+  if (NILP (buffer))
+    nsberror (buffer_or_name);
+  b = XBUFFER (buffer);
+  return (make_fixnum (b->pt));
+}
+
+DEFUN ("set-buffer-point", Fset_buffer_point, Sset_buffer_point, 2, 2, 0,
+       doc: /* Set the buffer point of BUFFER-OR-NAME to POS.
+BUFFER-OR-NAME is a buffer or the name of one.  POS is a buffer
+position, either a number or a marker.  If POS is outside the current
+visible region, the buffer point is set to the nearest place in it.
+Return the buffer point actually set.  */)
+  (Lisp_Object buffer_or_name, Lisp_Object pos)
+{
+  Lisp_Object buffer;
+  struct buffer *b;
+  ptrdiff_t p;
+
+  buffer = Fget_buffer (buffer_or_name);
+  if (NILP (buffer))
+    nsberror (buffer_or_name);
+  b = XBUFFER (buffer);
+
+  CHECK_FIXNUM_COERCE_MARKER (pos);
+  p = XFIXNUM (pos);
+  if (p < b->begv) p = b->begv;
+  if (p > b->zv) p = b->zv;
+
+  SET_PT (p);
+  return make_fixnum (p);
+}
+
+
 DEFUN ("force-mode-line-update", Fforce_mode_line_update,
        Sforce_mode_line_update, 0, 1, 0,
        doc: /* Force redisplay of the current buffer's mode line and header line.
@@ -5898,6 +5942,8 @@ There is no reason to change that value except for debugging purposes.  */);
   defsubr (&Sbuffer_local_value);
   defsubr (&Sbuffer_local_variables);
   defsubr (&Sbuffer_modified_p);
+  defsubr (&Sbuffer_point);
+  defsubr (&Sset_buffer_point);
   defsubr (&Sforce_mode_line_update);
   defsubr (&Sset_buffer_modified_p);
   defsubr (&Sbuffer_modified_tick);


-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Eli Zaretskii]

> Date: Mon, 31 Oct 2022 14:32:12 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> Anyhow, I proposed buffer-point and set-buffer-point.  They would be a
> lot faster than set-buffer followed by point and goto-char.  Here is my
> first version of these.  What do you think?

I'm not sure performance in a debugger is a reason good enough to add
2 more primitives.  The fact that we didn't need them until now should
tell us something, no?

Stefan, Lars, WDYT?

Anyway, a couple of minor comments:

> +DEFUN ("buffer-point", Fbuffer_point, Sbuffer_point, 1, 1, 0,
> +       doc: /* Return the buffer point of BUFFER-OR-NAME.
> +The argument may be a buffer or the name of an existing buffer.  */)
> +  (Lisp_Object buffer_or_name)

Why not an optional argument to 'point'?  And why in buffer.c and not
in editfns.c?

> +  return (make_fixnum (b->pt));

Please never-ever use b->pt etc. directly.  We have BUF_PT and other
macros for that, and for a good reason.

> +  CHECK_FIXNUM_COERCE_MARKER (pos);
> +  p = XFIXNUM (pos);

This is sub-optimal: a marker holds both character and byte position,
and you lose it here.  Ignoring the byte position is only justified if
the marker belongs to the wrong buffer.

> +  if (p < b->begv) p = b->begv;
> +  if (p > b->zv) p = b->zv;

We have clip_to_bounds.  And again, always use BUF_BEGV and BUF_ZV,
not literal references to members of struct buffer.

> +  SET_PT (p);

We have SET_BUF_PT_BOTH.

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Alan Mackenzie]

Hello, Eli.

On Mon, Oct 31, 2022 at 16:50:52 +0200, Eli Zaretskii wrote:
> > Date: Mon, 31 Oct 2022 14:32:12 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > Anyhow, I proposed buffer-point and set-buffer-point.  They would be a
> > lot faster than set-buffer followed by point and goto-char.  Here is my
> > first version of these.  What do you think?

> I'm not sure performance in a debugger is a reason good enough to add
> 2 more primitives.  The fact that we didn't need them until now should
> tell us something, no?

Well, I timed it.  With 207 buffers, creating an alist of (buffer .
buffere-point) with my new function was 17 times as fast as using
with-current-buffer and point.  Restoring the buffer-points was 8 times
as fast with my new function.  It's probably moot, though, since the
"slow" restoration only took 0.00137 seconds for all 207 buffers.

But on the other hand, these two functions feel like they ought to exist.
They could save a lot of clumsy programming with swapping the buffer,
just to get or set point.

> Stefan, Lars, WDYT?

> Anyway, a couple of minor comments:

> > +DEFUN ("buffer-point", Fbuffer_point, Sbuffer_point, 1, 1, 0,
> > +       doc: /* Return the buffer point of BUFFER-OR-NAME.
> > +The argument may be a buffer or the name of an existing buffer.  */)
> > +  (Lisp_Object buffer_or_name)

> Why not an optional argument to 'point'?  And why in buffer.c and not
> in editfns.c?

I'm not sure what you mean by an optional argument, here.

buffer.c was the first file that sprang to mind.  It could easily be
somewhere else, though.

> > +  return (make_fixnum (b->pt));

> Please never-ever use b->pt etc. directly.  We have BUF_PT and other
> macros for that, and for a good reason.

BUF_PT and friends work specifically on current_buffer.  The whole idea
of the new functions is to avoid having to switch buffers.

> > +  CHECK_FIXNUM_COERCE_MARKER (pos);
> > +  p = XFIXNUM (pos);

> This is sub-optimal: a marker holds both character and byte position,
> and you lose it here.  Ignoring the byte position is only justified if
> the marker belongs to the wrong buffer.

And I need to check the marker's in the correct buffer, too.  But thanks
for the tip!

> > +  if (p < b->begv) p = b->begv;
> > +  if (p > b->zv) p = b->zv;

> We have clip_to_bounds.  And again, always use BUF_BEGV and BUF_ZV,
> not literal references to members of struct buffer.

I knew there was some sort of function, but couldn't think of the name,
and couldn't find a way to search for it.  ;-(

> > +  SET_PT (p);

> We have SET_BUF_PT_BOTH.

OK.  That SET_PT was a mistake; it sets point in the current buffer, not
the buffer that was the argument.  But I take the point (no pun
intended).  If we've already got point_byte, there's no point calculating
it all over again.

Thanks!

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Stefan Monnier]

[ Duh, sorry I saw this message too late.  ]

> Well, I timed it.  With 207 buffers, creating an alist of (buffer .
> buffer-point) with my new function was 17 times as fast as using
> with-current-buffer and point.  Restoring the buffer-points was
> 8 times as fast with my new function.

I actually expected worse than 17x.

> It's probably moot, though, since the "slow" restoration only took
> 0.00137 seconds for all 207 buffers.

That's the main issue for me, yes.  I mean, I regularly have more than
200 buffers in my Emacs sessions, but AFAICT the rest of the work
performed by Edebug when it interrupts the execution of the main program
will dwarf this anyway.

I think the better way forward is to define `edebug--buffer-point` and
`edebug--set-buffer-point` functions in `debug.el` for now.  If at some
point we can see that it's useful in other packages we can move those
definitions elsewhere and remove their `edebug--` prefix.  And if at
some point they show up as a significant contribution in a CPU profile,
*then* we can move them to C.

> But on the other hand, these two functions feel like they ought to exist.
> They could save a lot of clumsy programming with swapping the buffer,
> just to get or set point.

    (with-current-buffer BUF (point))
and
    (with-current-buffer BUF (goto-char POS))

aren't that terribly verbose.


>> Why not an optional argument to 'point'?  And why in buffer.c and not
>> in editfns.c?
> I'm not sure what you mean by an optional argument, here.

IIUC he's suggesting to redefine `point` to take an optional argument
(and merge it with `buffer-point`).

>> Please never-ever use b->pt etc. directly.  We have BUF_PT and other
>> macros for that, and for a good reason.
> BUF_PT and friends work specifically on current_buffer.

No, that's `PT`.  `BUF_PT` takes a buffer argument.


        Stefan

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Eli Zaretskii]

> Date: Mon, 31 Oct 2022 15:46:07 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > I'm not sure performance in a debugger is a reason good enough to add
> > 2 more primitives.  The fact that we didn't need them until now should
> > tell us something, no?
> 
> Well, I timed it.  With 207 buffers, creating an alist of (buffer .
> buffere-point) with my new function was 17 times as fast as using
> with-current-buffer and point.

17 times faster doesn't yet tell how important is the speedup, because
you give no absolute numbers, and they are what's important here.

> But on the other hand, these two functions feel like they ought to exist.
> They could save a lot of clumsy programming with swapping the buffer,
> just to get or set point.

There's nothing clumsy with what we did, and the fact that we did
manage without them speaks volumes.

> > > +DEFUN ("buffer-point", Fbuffer_point, Sbuffer_point, 1, 1, 0,
> > > +       doc: /* Return the buffer point of BUFFER-OR-NAME.
> > > +The argument may be a buffer or the name of an existing buffer.  */)
> > > +  (Lisp_Object buffer_or_name)
> 
> > Why not an optional argument to 'point'?  And why in buffer.c and not
> > in editfns.c?
> 
> I'm not sure what you mean by an optional argument, here.

I mean (point &optional buffer), of course, what else could I mean?

> > > +  return (make_fixnum (b->pt));
> 
> > Please never-ever use b->pt etc. directly.  We have BUF_PT and other
> > macros for that, and for a good reason.
> 
> BUF_PT and friends work specifically on current_buffer.

No, they don't:

  /* Position of point in buffer.  */
  INLINE ptrdiff_t
  BUF_PT (struct buffer *buf)
  {
    return (buf == current_buffer ? PT
	    : NILP (BVAR (buf, pt_marker)) ? buf->pt
	    : marker_position (BVAR (buf, pt_marker)));
  }

> The whole idea of the new functions is to avoid having to switch
> buffers.

We do this from C in a gazillion of places.

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Alan Mackenzie]

Hello, Eli.

On Mon, Oct 31, 2022 at 19:55:40 +0200, Eli Zaretskii wrote:
> > Date: Mon, 31 Oct 2022 15:46:07 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > I'm not sure performance in a debugger is a reason good enough to add
> > > 2 more primitives.  The fact that we didn't need them until now should
> > > tell us something, no?

> > Well, I timed it.  With 207 buffers, creating an alist of (buffer .
> > buffere-point) with my new function was 17 times as fast as using
> > with-current-buffer and point.

> 17 times faster doesn't yet tell how important is the speedup, because
> you give no absolute numbers, and they are what's important here.

I think I did.  To quote:
> > It's probably moot, though, since the "slow" restoration only took
> > 0.00137 seconds for all 207 buffers.

> > But on the other hand, these two functions feel like they ought to exist.
> > They could save a lot of clumsy programming with swapping the buffer,
> > just to get or set point.

> There's nothing clumsy with what we did, and the fact that we did
> manage without them speaks volumes.

OK.

> > > > +DEFUN ("buffer-point", Fbuffer_point, Sbuffer_point, 1, 1, 0,
> > > > +       doc: /* Return the buffer point of BUFFER-OR-NAME.
> > > > +The argument may be a buffer or the name of an existing buffer.  */)
> > > > +  (Lisp_Object buffer_or_name)

> > > Why not an optional argument to 'point'?  And why in buffer.c and not
> > > in editfns.c?

> > I'm not sure what you mean by an optional argument, here.

> I mean (point &optional buffer), of course, what else could I mean?

OK.

> > > > +  return (make_fixnum (b->pt));

> > > Please never-ever use b->pt etc. directly.  We have BUF_PT and other
> > > macros for that, and for a good reason.

> > BUF_PT and friends work specifically on current_buffer.

> No, they don't:

Apologies.  I got that wrong.

>   /* Position of point in buffer.  */
>   INLINE ptrdiff_t
>   BUF_PT (struct buffer *buf)
>   {
>     return (buf == current_buffer ? PT
> 	    : NILP (BVAR (buf, pt_marker)) ? buf->pt
> 	    : marker_position (BVAR (buf, pt_marker)));
>   }

> > The whole idea of the new functions is to avoid having to switch
> > buffers.

> We do this from C in a gazillion of places.

OK.  I now think these new functions aren't really needed, mainly because
the current way, though much slower, is fast enough.  I still think they
would be a neater way of getting/setting a buffer point, but it's not a
big thing.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Eli Zaretskii]

> Date: Mon, 31 Oct 2022 20:46:12 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > 17 times faster doesn't yet tell how important is the speedup, because
> > you give no absolute numbers, and they are what's important here.
> 
> I think I did.  To quote:
> > > It's probably moot, though, since the "slow" restoration only took
> > > 0.00137 seconds for all 207 buffers.

So if this means that the faster method gains you 1.3 msec for 200
buffers, I think such a difference is negligible in the context of
debugging, where code always runs much slower than normally.

> OK.  I now think these new functions aren't really needed, mainly because
> the current way, though much slower, is fast enough.  I still think they
> would be a neater way of getting/setting a buffer point, but it's not a
> big thing.

OK, thanks.

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Stefan Monnier]

>> Anyhow, I proposed buffer-point and set-buffer-point.  They would be a
>> lot faster than set-buffer followed by point and goto-char.  Here is my
>> first version of these.  What do you think?
>
> I'm not sure performance in a debugger is a reason good enough to add
> 2 more primitives.  The fact that we didn't need them until now should
> tell us something, no?
> Stefan, Lars, WDYT?

I'd be interested to see numbers showing that it's indeed "too costly"
to just use `set-buffer`, before we add such primitives.

> Anyway, a couple of minor comments:

AFAIK `get-buffer` will return non-nil when passed a killed buffer, so
I think those two functions currently would misbehave when passed
a killed buffer.

>> +DEFUN ("buffer-point", Fbuffer_point, Sbuffer_point, 1, 1, 0,
>> +       doc: /* Return the buffer point of BUFFER-OR-NAME.
>> +The argument may be a buffer or the name of an existing buffer.  */)
>> +  (Lisp_Object buffer_or_name)
>
> Why not an optional argument to 'point'?

That would be a lot more invasive, so hard to justify for
a functionality that we're not even sure we want to have.

> And why in buffer.c and not in editfns.c?

[ Truth be told, I don't really understand how these are split :-(  ]

>> +  CHECK_FIXNUM_COERCE_MARKER (pos);
>> +  p = XFIXNUM (pos);
>
> This is sub-optimal: a marker holds both character and byte position,
> and you lose it here.  Ignoring the byte position is only justified if
> the marker belongs to the wrong buffer.

I suspect the performance impact is negligible compared to what it would
cost using `set-buffer`.  Since we're not convinced the cost of
`set-buffer` is high enough to justify `set-buffer-point`, I wouldn't
worry about optimizing the case where the arg is a marker.


        Stefan

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Alan Mackenzie <acm@muc.de>,  emacs-devel@gnu.org
> Date: Mon, 31 Oct 2022 13:19:34 -0400
> 
> >> +  CHECK_FIXNUM_COERCE_MARKER (pos);
> >> +  p = XFIXNUM (pos);
> >
> > This is sub-optimal: a marker holds both character and byte position,
> > and you lose it here.  Ignoring the byte position is only justified if
> > the marker belongs to the wrong buffer.
> 
> I suspect the performance impact is negligible

Are you talking about using character and byte position, or are you
talking about something else?  The performance impact of char_to_byte
is not negligible at all!

> compared to what it would
> cost using `set-buffer`.  Since we're not convinced the cost of
> `set-buffer` is high enough to justify `set-buffer-point`, I wouldn't
> worry about optimizing the case where the arg is a marker.

My remarks were general, not necessarily related to this particular
patch.  The principle should always be if you already have a byte
position, use it!

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Stefan Monnier]

>> >> +  CHECK_FIXNUM_COERCE_MARKER (pos);
>> >> +  p = XFIXNUM (pos);
>> >
>> > This is sub-optimal: a marker holds both character and byte position,
>> > and you lose it here.  Ignoring the byte position is only justified if
>> > the marker belongs to the wrong buffer.
>> I suspect the performance impact is negligible
> Are you talking about using character and byte position,

Yes, tho only specifically in this `set-buffer-point` case.

> The performance impact of char_to_byte is not negligible at all!

Agreed.  My comment was based on the fact that I don't expect
`set-buffer-point` to be called very many times.

> My remarks were general, not necessarily related to this particular
> patch.  The principle should always be if you already have a byte
> position, use it!

I didn't mean to imply otherwise.


        Stefan

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Stefan Monnier]

> A few weeks ago, I was attempting to edebug a program which itself
> scanned through a buffer B.  That buffer was also displayed in a window
> (or possibly two windows).  Each time the program went into edebug, the
> point in B got corrupted.

Re-reading this email I now realize that I've seen similar problems
without Edebug.  Maybe we should try and make sure redisplay itself
(rather than, or in addition, I'm not sure) preserves buffer points?


        Stefan

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Mon, 31 Oct 2022 13:21:57 -0400
> 
> Maybe we should try and make sure redisplay itself (rather than, or
> in addition, I'm not sure) preserves buffer points?

We do try and make sure it does.  There are quite a few places in the
code where we jump through many hoops to make sure this is true.

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Stefan Monnier]

Eli Zaretskii [2022-10-31 20:10:43] wrote:
>> Maybe we should try and make sure redisplay itself (rather than, or
>> in addition, I'm not sure) preserves buffer points?
>
> We do try and make sure it does.  There are quite a few places in the
> code where we jump through many hoops to make sure this is true.

Hmm... maybe we should investigate exactly why the buffer's point is
modified in Alan's case.  Maybe it should indeed be Edebug's role to
save&restore those, but maybe what he's seeing is just a plain
bug elsewhere.


        Stefan

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: acm@muc.de,  emacs-devel@gnu.org
> Date: Mon, 31 Oct 2022 19:14:26 -0400
> 
> Eli Zaretskii [2022-10-31 20:10:43] wrote:
> >> Maybe we should try and make sure redisplay itself (rather than, or
> >> in addition, I'm not sure) preserves buffer points?
> >
> > We do try and make sure it does.  There are quite a few places in the
> > code where we jump through many hoops to make sure this is true.
> 
> Hmm... maybe we should investigate exactly why the buffer's point is
> modified in Alan's case.

Yes, we should, I agree.

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Alan Mackenzie]

Hello, Emacs.

On Mon, Oct 31, 2022 at 11:43:15 +0000, Alan Mackenzie wrote:
> A few weeks ago, I was attempting to edebug a program which itself
> scanned through a buffer B.  That buffer was also displayed in a window
> (or possibly two windows).  Each time the program went into edebug, the
> point in B got corrupted.

> Setting edebug-save-displayed-buffer-points didn't help.

> I now understand what was going wrong.  Without
> edebug-save-displayed-buffer-points, B's point got set to a window point
> from a random window displaying B, caused by a set-window-configuration
> call from edebug.

> With edebug-save-displayed-buffer-points set, the function
> edebug-get-displayed-buffer points was recording window-points for each
> displayed window, but after the recursive edit was writing these
> window-points back into the buffer points.  This is surely a bug.  At the
> least, there is a race condition if two windows display the same buffer.

> In neither of the above scenarios does edebug do anything to restore the
> buffer points after the recursive edit.  This seems to be a bad thing.

> I propose that edebug should get an option to preserve buffer points,
> alongside the existing options which preserve window points.

[ .... ]

I now have a patch which aims to fix the above problem.  To use it,
select a window containing the buffer the function under test will be
scanning (or otherwise manipulating).  Type C-x X F to add that buffer's
name to edebug-save-buffer-points.

During the current or future edebug session, this buffer will now be
protected from having its buffer point corrupted by edebug's other
mechanisms which manipulate window-points.

In fact, I'm not sure that edebug-save-displayed-buffer-points isn't
obsolete - it dates from 1994 (or earlier), a time before there were
frames or window configurations.  It seems to me its function is covered
by the later edebug-save-windows.

Anyhow, here's the patch.  What do you think?



diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index 67704bdb51..c4099164bc 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -157,6 +157,23 @@ edebug-save-displayed-buffer-points
 need it."
   :type 'boolean)
 
+(defcustom edebug-save-buffer-points nil
+  "If non-nil save and restore the buffer points in some buffers.
+
+Saving and restoring the buffer point in a buffer is needed if you
+are debugging code which sets point in that buffer, particularly if
+there is also a window displaying that buffer.  Otherwise the buffer
+point (being used by the program) will get overwritten by the
+window point.
+
+If the value is a list of buffer names (recommended), only those
+buffers will have their buffer points restored.  Otherwise, t means
+restore all buffers\\=' points, and nil means none.
+
+A buffer\\='s name can be added to or removed from this list
+dynamically by the key binding \\[edebug-toggle-save-buffer-points]."
+  :type '(choice boolean (repeat string)))
+
 (defcustom edebug-initial-mode 'step
   "Initial execution mode for Edebug, if non-nil.
 If this variable is non-nil, it specifies the initial execution mode
@@ -401,6 +420,32 @@ edebug-set-buffer-points
 		(goto-char (cdr buf-point))))
 	    buffer-points)))
 
+(defun edebug-get-all-buffer-points ()
+  ;; Return an alist of (BUFFER . BUFFER-POINT) for all buffers.
+  (let (bp-list)
+    (dolist (buf (buffer-list))
+      (with-current-buffer buf
+        (push (cons buf (point)) bp-list)))
+    bp-list))
+
+(defun edebug-restore-buffer-points (buffer-points)
+  ;; Restore (some) buffer-points from the argument, an alist with
+  ;; elements of the form (BUFFER . BUFFER-POINT).
+  (cond
+   ((eq edebug-save-buffer-points t)
+    (dolist (elt buffer-points)
+      (unless (eq (car elt) (current-buffer))
+        (with-current-buffer (car elt)
+          (goto-char (cdr elt))))))
+   ((listp edebug-save-buffer-points)
+    (dolist (buf edebug-save-buffer-points)
+      (let ((elt (assq buf buffer-points)))
+        (when elt                       ; ?Always non-nil
+          (unless (eq (car elt) (current-buffer))
+            (with-current-buffer (car elt)
+              (goto-char (cdr elt))))))))
+   (t nil)))
+
 (defun edebug-current-windows (which-windows)
   ;; Get either a full window configuration or some window information.
   (if (listp which-windows)
@@ -2600,6 +2647,8 @@ edebug--display-1
         (edebug-outside-mark (mark t))
 	edebug-outside-windows		; Window or screen configuration.
 	edebug-buffer-points
+        edebug-all-buffer-points
+
 
 	edebug-eval-buffer             ; Declared here so we can kill it below.
 	(eval-result-list (and edebug-eval-list
@@ -2627,7 +2678,9 @@ edebug--display-1
 	      (setq edebug-outside-windows
 		    (edebug-current-windows edebug-save-windows)))
 
-	  (if edebug-save-displayed-buffer-points
+          (setq edebug-all-buffer-points (edebug-get-all-buffer-points))
+
+          (if edebug-save-displayed-buffer-points
 	      (setq edebug-buffer-points (edebug-get-displayed-buffer-points)))
 
 	  ;; First move the edebug buffer point to edebug-point
@@ -2759,6 +2814,11 @@ edebug--display-1
                     (if edebug-save-displayed-buffer-points
                         (edebug-set-buffer-points edebug-buffer-points))
 
+                    ;; Restore (non-displayed) buffer points.
+                    (if edebug-save-buffer-points
+                        (edebug-restore-buffer-points
+                         edebug-all-buffer-points))
+
                     ;; Unrestore trace window's window-point.
                     (if edebug-trace-window
                         (set-window-start edebug-trace-window
@@ -3013,6 +3075,26 @@ edebug-toggle-save-windows
       (edebug-toggle-save-selected-window)
     (edebug-toggle-save-all-windows)))
 
+(defun edebug-toggle-save-buffer-points (arg)
+  "Toggle the saving and restoring of a buffer\\='s buffer point.
+That buffer is the one in the selected window.  With a prefix
+argument, switch the setting on or off for all buffers."
+  (interactive "P")
+  (let* ((cur-win-buf (window-buffer (selected-window)))
+         (win-buf-name (buffer-name cur-win-buf)))
+    (if arg
+        (setq edebug-save-buffer-points (not edebug-save-buffer-points))
+      (cond
+       ((eq t edebug-save-buffer-points)
+        ;; Save all buffers except the one in the current window.
+        (setq edebug-save-buffer-points
+              (mapcar #'buffer-name
+                      (delq cur-win-buf (buffer-list)))))
+       ((memq win-buf-name edebug-save-buffer-points)
+        (setq edebug-save-buffer-points
+              (delq (buffer-name cur-win-buf) edebug-save-buffer-points)))
+       (t (push win-buf-name edebug-save-buffer-points))))))
+
 (defun edebug-where ()
   "Show the debug windows and where we stopped in the program."
   (interactive)
@@ -3850,6 +3934,7 @@ edebug-mode-map
   "p"       #'edebug-bounce-point
   "P"       #'edebug-view-outside        ; same as v
   "W"       #'edebug-toggle-save-windows
+  ;; "F"       #'edebug-toggle-save-buffer-points
 
   ;; misc
   "?"       #'edebug-help
@@ -3904,6 +3991,7 @@ edebug-global-map
   ;; views
   "w"   #'edebug-where
   "W"   #'edebug-toggle-save-windows
+  "F"   #'edebug-toggle-save-buffer-points
 
   ;; quitting
   "q"   #'top-level


> -- 
> Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Eli Zaretskii]

> Date: Mon, 31 Oct 2022 21:25:08 +0000
> From: Alan Mackenzie <acm@muc.de>
> 
> +(defcustom edebug-save-buffer-points nil
> +  "If non-nil save and restore the buffer points in some buffers.
> +
> +Saving and restoring the buffer point in a buffer is needed if you
> +are debugging code which sets point in that buffer, particularly if
> +there is also a window displaying that buffer.  Otherwise the buffer
> +point (being used by the program) will get overwritten by the
> +window point.
> +
> +If the value is a list of buffer names (recommended), only those
> +buffers will have their buffer points restored.  Otherwise, t means
> +restore all buffers\\=' points, and nil means none.

If we indeed need such an option, why shouldn't it be Edebug's
business to automatically keep point in all buffers that are displayed
in some window?  It doesn't strike me as the best UI to burden the
user with that task.

And like Stefan, I think we still need to understand better what
exactly happens here and why.  I don't think I understood that from
your original description.

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Eli.

On Tue, Nov 01, 2022 at 08:45:42 +0200, Eli Zaretskii wrote:
> > Date: Mon, 31 Oct 2022 21:25:08 +0000
> > From: Alan Mackenzie <acm@muc.de>

> > +(defcustom edebug-save-buffer-points nil
> > +  "If non-nil save and restore the buffer points in some buffers.
> > +
> > +Saving and restoring the buffer point in a buffer is needed if you
> > +are debugging code which sets point in that buffer, particularly if
> > +there is also a window displaying that buffer.  Otherwise the buffer
> > +point (being used by the program) will get overwritten by the
> > +window point.
> > +
> > +If the value is a list of buffer names (recommended), only those
> > +buffers will have their buffer points restored.  Otherwise, t means
> > +restore all buffers\\=' points, and nil means none.

> If we indeed need such an option, why shouldn't it be Edebug's
> business to automatically keep point in all buffers that are displayed
> in some window?  It doesn't strike me as the best UI to burden the
> user with that task.

It would be intolerable for users.  Say during an edebug session, the
user makes some notes in buffer my-notes.txt.  Execute an instruction,
then go back to my-notes.txt.  Point has been "restored" to before the
new notes.  That would happen in every buffer.

> And like Stefan, I think we still need to understand better what
> exactly happens here and why.  I don't think I understood that from
> your original description.

Sorry, I had some difficulty getting from my original problem to a
reproducible test case.  That's here:

With test-edebug.el being
#########################################################################
(defun test-edebug ()
  (let ((A "*scratch*") (B "emacs.README"))
    (set-buffer A)
    (set-buffer B)
    (goto-char (point-max))
    (insert "(2022-11-01)\n")
    ;; B's buffer-point is at point-max.

    (set-buffer A)
    (set-buffer B)
    ;; B's buffer-point is no longer at point-max.
    (insert "(2022-11-01)a\n")))
#########################################################################
,
(i) Emacs -Q.
(ii) On a single frame, arrange buffers *scratch*, test-edebug.el, and
  some other substantial buffer, that I call emacs.README.
(iii) Put point in emacs.README somewhere other than point-max.
(iv) Instrument test-edebug for edebug with C-u C-M-x.
(v) M-: (test-edebug).
(vi) Step through test-edebug using the space key.
(vii) Note that the second text insertion happens where point was in the
  window, not at point-max.  This is the bug.

My patch from yesterday evening, though not in its final form, appears
to solve the bug, providing the user does C-x X F in emacs.README.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> Date: Tue, 1 Nov 2022 11:41:02 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > > +If the value is a list of buffer names (recommended), only those
> > > +buffers will have their buffer points restored.  Otherwise, t means
> > > +restore all buffers\\=' points, and nil means none.
> 
> > If we indeed need such an option, why shouldn't it be Edebug's
> > business to automatically keep point in all buffers that are displayed
> > in some window?  It doesn't strike me as the best UI to burden the
> > user with that task.
> 
> It would be intolerable for users.  Say during an edebug session, the
> user makes some notes in buffer my-notes.txt.  Execute an instruction,
> then go back to my-notes.txt.  Point has been "restored" to before the
> new notes.  That would happen in every buffer.

??? Why would point be restored to the value before the last move of
point?  The program you are debugging doesn't affect that buffer with
notes, does it?

Maybe I don't understand what your patch does, then.  I thought it was
supposed to _prevent_ such moves from happening.

> With test-edebug.el being
> #########################################################################
> (defun test-edebug ()
>   (let ((A "*scratch*") (B "emacs.README"))
>     (set-buffer A)
>     (set-buffer B)
>     (goto-char (point-max))
>     (insert "(2022-11-01)\n")
>     ;; B's buffer-point is at point-max.
> 
>     (set-buffer A)
>     (set-buffer B)
>     ;; B's buffer-point is no longer at point-max.
>     (insert "(2022-11-01)a\n")))
> #########################################################################
> ,
> (i) Emacs -Q.
> (ii) On a single frame, arrange buffers *scratch*, test-edebug.el, and
>   some other substantial buffer, that I call emacs.README.
> (iii) Put point in emacs.README somewhere other than point-max.
> (iv) Instrument test-edebug for edebug with C-u C-M-x.
> (v) M-: (test-edebug).
> (vi) Step through test-edebug using the space key.
> (vii) Note that the second text insertion happens where point was in the
>   window, not at point-max.  This is the bug.

I cannot reproduce this: for me, the insertion is at point-max.  Maybe
your recipe description is incomplete?

But in any case, I didn't ask _what_ happens, I asked _why_?  IOW, I
presumed that you understood why Edebug moves point, and asked for a
detailed description of the code involved and the reason it gets
executed in this scenario.

Thanks.

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Eli.

On Tue, Nov 01, 2022 at 13:53:09 +0200, Eli Zaretskii wrote:
> > Date: Tue, 1 Nov 2022 11:41:02 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > > +If the value is a list of buffer names (recommended), only those
> > > > +buffers will have their buffer points restored.  Otherwise, t means
> > > > +restore all buffers\\=' points, and nil means none.

> > > If we indeed need such an option, why shouldn't it be Edebug's
> > > business to automatically keep point in all buffers that are displayed
> > > in some window?  It doesn't strike me as the best UI to burden the
> > > user with that task.

> > It would be intolerable for users.  Say during an edebug session, the
> > user makes some notes in buffer my-notes.txt.  Execute an instruction,
> > then go back to my-notes.txt.  Point has been "restored" to before the
> > new notes.  That would happen in every buffer.

> ??? Why would point be restored to the value before the last move of
> point?  The program you are debugging doesn't affect that buffer with
> notes, does it?

Probably not, but it might.  The point is, edebug can't know [*] which
buffers have been modified by edebug itself, and which have been modified
by the user.  Some might have been modified by both.
[*] Without significant enhancement to edebug.

> Maybe I don't understand what your patch does, then.  I thought it was
> supposed to _prevent_ such moves from happening.

In the blank line in test-edebug.el, the patch prevents emacs.README's
buffer-point being set to the window-point of the window in which the
buffer is displayed.

> > With test-edebug.el being
> > #########################################################################
> >  1 (defun test-edebug ()
> >  2   (let ((A "*scratch*") (B "emacs.README"))
> >  3    (set-buffer A)
> >  4    (set-buffer B)
> >  5    (goto-char (point-max))
> >  6    (insert "(2022-11-01)\n")
> >  7    ;; B's buffer-point is at point-max.
> >  8
> >  9    (set-buffer A)
> > 10    (set-buffer B)
> > 11    ;; B's buffer-point is no longer at point-max.
> > 12   (insert "(2022-11-01)a\n")))
> > #########################################################################
> > ,
> > (i) Emacs -Q.
> > (ii) On a single frame, arrange buffers *scratch*, test-edebug.el, and
> >   some other substantial buffer, that I call emacs.README.
> > (iii) Put point in emacs.README somewhere other than point-max.
> > (iv) Instrument test-edebug for edebug with C-u C-M-x.
> > (v) M-: (test-edebug).
> > (vi) Step through test-edebug using the space key.
> > (vii) Note that the second text insertion happens where point was in the
> >   window, not at point-max.  This is the bug.

> I cannot reproduce this: for me, the insertion is at point-max.  Maybe
> your recipe description is incomplete?

Apologies.  I neglected to mention that window-saving (toggled by "W" in
an edebug session) must be enabled to trigger the bug.  Indeed, I wasn't
fully aware of this.

> But in any case, I didn't ask _what_ happens, I asked _why_?  IOW, I
> presumed that you understood why Edebug moves point, and asked for a
> detailed description of the code involved and the reason it gets
> executed in this scenario.

OK, here goes!

It's all in the function edebug--display-1, which in the master copy of
edebug.el starts at L2573.  At L2628, e--display-1 calls
edebug-current-windows, which calls current-window-configuration, saving
the configuration in edebug-outside-windows.

The recursive edit takes place at L2730, in which the user types a space
to execute one instruction.  Suppose that was L9 from test-edebug.el (see
above).  The current buffer is now A (*scratch*).

At L2754, the function calls edebug-set-windows which calls
(set-window-configuration edebug-outside-windows).

The program advances one instruction and calls edebug--display-1 again.
It calls .... which calls current-window-configuration, which stores the
window-point of buffer B (emacs.README) which is no longer the current
buffer.

In the recursive edit, the user again types space which advances over
L10.

At L2754 again, ... which calls set-window-configuration.  This time B
was not the current buffer stored by current-window-configuration, so B's
BUFFER-POINT GETS RESTORED TO ITS STORED WINDOW-POINT.  This happens at
src/window.c function Fset_window_configuration at L7270.

This restored point in buffer B is where the second `insert' wrongly
takes effect.

> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> Date: Tue, 1 Nov 2022 13:42:03 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > ??? Why would point be restored to the value before the last move of
> > point?  The program you are debugging doesn't affect that buffer with
> > notes, does it?
> 
> Probably not, but it might.  The point is, edebug can't know [*] which
> buffers have been modified by edebug itself, and which have been modified
> by the user.  Some might have been modified by both.
> [*] Without significant enhancement to edebug.

What do you mean by "buffers modified by edebug itself"?  AFAIK,
Edebug doesn't modify any buffers except the one where you are
stepping, where it moves the overlay arrow and point.

> > > With test-edebug.el being
> > > #########################################################################
> > >  1 (defun test-edebug ()
> > >  2   (let ((A "*scratch*") (B "emacs.README"))
> > >  3    (set-buffer A)
> > >  4    (set-buffer B)
> > >  5    (goto-char (point-max))
> > >  6    (insert "(2022-11-01)\n")
> > >  7    ;; B's buffer-point is at point-max.
> > >  8
> > >  9    (set-buffer A)
> > > 10    (set-buffer B)
> > > 11    ;; B's buffer-point is no longer at point-max.
> > > 12   (insert "(2022-11-01)a\n")))
> > > #########################################################################
> > > ,
> > > (i) Emacs -Q.
> > > (ii) On a single frame, arrange buffers *scratch*, test-edebug.el, and
> > >   some other substantial buffer, that I call emacs.README.
> > > (iii) Put point in emacs.README somewhere other than point-max.
> > > (iv) Instrument test-edebug for edebug with C-u C-M-x.
> > > (v) M-: (test-edebug).
> > > (vi) Step through test-edebug using the space key.
> > > (vii) Note that the second text insertion happens where point was in the
> > >   window, not at point-max.  This is the bug.
> 
> > I cannot reproduce this: for me, the insertion is at point-max.  Maybe
> > your recipe description is incomplete?
> 
> Apologies.  I neglected to mention that window-saving (toggled by "W" in
> an edebug session) must be enabled to trigger the bug.  Indeed, I wasn't
> fully aware of this.

If the problem happens only when edebug-save-displayed-buffer-points
is non-nil, then maybe we should step back a notch and ask why you set
that option?  It is nil by default.  What doesn't work for you if you
keep it at its nil value?

AFAIU, this variable exists for some very special situations (which
ones exactly I admit I don't have a clear idea), and it could be that
your use case is not one of them.

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Eli.

On Tue, Nov 01, 2022 at 16:42:30 +0200, Eli Zaretskii wrote:
> > Date: Tue, 1 Nov 2022 13:42:03 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > ??? Why would point be restored to the value before the last move of
> > > point?  The program you are debugging doesn't affect that buffer with
> > > notes, does it?

> > Probably not, but it might.  The point is, edebug can't know [*] which
> > buffers have been modified by edebug itself, and which have been modified
> > by the user.  Some might have been modified by both.
> > [*] Without significant enhancement to edebug.

> What do you mean by "buffers modified by edebug itself"?  AFAIK,
> Edebug doesn't modify any buffers except the one where you are
> stepping, where it moves the overlay arrow and point.

Sorry, can we pretend I didn't write that last paragraph, please?  It's
nonsense.

Going back to your question three paragraphs ago, which I haven't
answered yet, point in the notes buffer would get restored by the window
configuration stuff we're discussing below.  At least it would if it's
displayed in the selected frame.

> > > > With test-edebug.el being
> > > > #########################################################################
> > > >  1 (defun test-edebug ()
> > > >  2   (let ((A "*scratch*") (B "emacs.README"))
> > > >  3    (set-buffer A)
> > > >  4    (set-buffer B)
> > > >  5    (goto-char (point-max))
> > > >  6    (insert "(2022-11-01)\n")
> > > >  7    ;; B's buffer-point is at point-max.
> > > >  8
> > > >  9    (set-buffer A)
> > > > 10    (set-buffer B)
> > > > 11    ;; B's buffer-point is no longer at point-max.
> > > > 12   (insert "(2022-11-01)a\n")))
> > > > #########################################################################
> > > > ,
> > > > (i) Emacs -Q.
> > > > (ii) On a single frame, arrange buffers *scratch*, test-edebug.el, and
> > > >   some other substantial buffer, that I call emacs.README.
> > > > (iii) Put point in emacs.README somewhere other than point-max.
> > > > (iv) Instrument test-edebug for edebug with C-u C-M-x.
> > > > (v) M-: (test-edebug).
> > > > (vi) Step through test-edebug using the space key.
> > > > (vii) Note that the second text insertion happens where point was in the
> > > >   window, not at point-max.  This is the bug.

> > > I cannot reproduce this: for me, the insertion is at point-max.  Maybe
> > > your recipe description is incomplete?

> > Apologies.  I neglected to mention that window-saving (toggled by "W" in
> > an edebug session) must be enabled to trigger the bug.  Indeed, I wasn't
> > fully aware of this.

> If the problem happens only when edebug-save-displayed-buffer-points
> is non-nil, then maybe we should step back a notch and ask why you set
> that option?  It is nil by default.

It is t by default, and has been since Richard S. created or amended the
declaration in 1997.  It frequently annoys me when I go into edebug (but
not enough for me actually to customise it to nil).

> What doesn't work for you if you keep it at its nil value?

Before I got into the habit of typing W to set it to nil, I seem to
remember that sometimes my .el buffer would scroll back to its previous
position when I typed (edebug's) space, sometimes it wouldn't.  I never
worked out why.

Might you have customised it to nil in your own configuration?

> AFAIU, this variable exists for some very special situations (which
> ones exactly I admit I don't have a clear idea), and it could be that
> your use case is not one of them.

As I say, I just find it annoying.  But t is the default value for it,
for reasons probably long lost.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> Date: Tue, 1 Nov 2022 17:06:38 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > If the problem happens only when edebug-save-displayed-buffer-points
> > is non-nil, then maybe we should step back a notch and ask why you set
> > that option?  It is nil by default.
> 
> It is t by default, and has been since Richard S. created or amended the
> declaration in 1997.

It isn't here:

  (defcustom edebug-save-displayed-buffer-points nil
    "If non-nil, save and restore point in all displayed buffers.

What am I missing?

> Might you have customised it to nil in your own configuration?

No.

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Eli.

On Tue, Nov 01, 2022 at 19:12:35 +0200, Eli Zaretskii wrote:
> > Date: Tue, 1 Nov 2022 17:06:38 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > If the problem happens only when edebug-save-displayed-buffer-points
> > > is non-nil, then maybe we should step back a notch and ask why you set
> > > that option?  It is nil by default.

> > It is t by default, and has been since Richard S. created or amended the
> > declaration in 1997.

> It isn't here:

>   (defcustom edebug-save-displayed-buffer-points nil
>     "If non-nil, save and restore point in all displayed buffers.

> What am I missing?

The troublesome behaviour is controlled by edebug-save-windows, not
edebug-save-displayed-buffer-points.  edebug-save-windows is enabled by
default.  Sorry for not reading your post more carefully.

> > Might you have customised it to nil in your own configuration?

> No.

OK.  Then I think we now agree that edebug-save-windows is t by default,
this is the variable toggled by the W key sequence, and it is the
variable which causes the undesired corruption of point when stepping
through test-edebug.el.

I don't know why edebug-save-windows is t by default.

In the meantime, the bug I have reported is real, even if it only
triggers occasionally.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> Date: Tue, 1 Nov 2022 17:24:26 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> >   (defcustom edebug-save-displayed-buffer-points nil
> >     "If non-nil, save and restore point in all displayed buffers.
> 
> > What am I missing?
> 
> The troublesome behaviour is controlled by edebug-save-windows, not
> edebug-save-displayed-buffer-points.  edebug-save-windows is enabled by
> default.  Sorry for not reading your post more carefully.

This now gets me back to the inability to reproduce the problem with
your recipe.  If that depends on edebug-save-windows, not on
edebug-save-displayed-buffer-points, and since edebug-save-windows is
t by default, why wasn't I able to reproduce the problem?

Anyway, the documentation of edebug-save-windows says:


   -- User Option: edebug-save-windows
       If this is non-‘nil’, Edebug saves and restores the window
       configuration.  That takes some time, so if your program does not
       care what happens to the window configurations, it is better to set
       this variable to ‘nil’.

       If the value is a list, only the listed windows are saved and
       restored.

So I'm now asking whether setting edebug-save-windows to nil would
have solved your problem, and if so, whether we really need some
bugfix and a new varaiable?

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Eli.

On Tue, Nov 01, 2022 at 19:57:11 +0200, Eli Zaretskii wrote:
> > Date: Tue, 1 Nov 2022 17:24:26 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > >   (defcustom edebug-save-displayed-buffer-points nil
> > >     "If non-nil, save and restore point in all displayed buffers.

> > > What am I missing?

> > The troublesome behaviour is controlled by edebug-save-windows, not
> > edebug-save-displayed-buffer-points.  edebug-save-windows is enabled by
> > default.  Sorry for not reading your post more carefully.

> This now gets me back to the inability to reproduce the problem with
> your recipe.  If that depends on edebug-save-windows, not on
> edebug-save-displayed-buffer-points, and since edebug-save-windows is
> t by default, why wasn't I able to reproduce the problem?

I don't know.  Have you tried again?

> Anyway, the documentation of edebug-save-windows says:


>    -- User Option: edebug-save-windows
>        If this is non-‘nil’, Edebug saves and restores the window
>        configuration.  That takes some time, so if your program does not
>        care what happens to the window configurations, it is better to set
>        this variable to ‘nil’.

>        If the value is a list, only the listed windows are saved and
>        restored.

> So I'm now asking whether setting edebug-save-windows to nil would
> have solved your problem, and if so, whether we really need some
> bugfix and a new varaiable?

Well, it seems at the moment that my problem was caused by
set-window-configuration, in that not only does it restore the stored
window configuration, it also overwrites the buffer-points for all but
the current buffer.  That is the mechanism of the corruption of the
buffer-points, which I detailed earlier.

Why does set-window-configuration overwrite the buffer-points?  The
window configuration does not contain them.  The code just assumes that
the buffer-point should be set to the window point.  Of course, we have
a race condition if a buffer is displayed in several windows.  So this
would appear to be a bug, the root cause of the bug in this thread.

Maybe set-window-configuration should be amended not to write the
buffer-points?  That might cause problems in other areas, though.  The
window configuration is one of the few areas where the documentation is
poor enough that you need to read the C source to find out what it's
really doing.

To come back to your two questions, I honestly don't know if setting
edebug-save-windows to nil would have prevented the problem.  I think
so, but I'm not sure.  The original bug is an arduous bug to set up.

Whether we need a bugfix, I would say definitely yes.  There is no way
that a user faced with this corruption of a buffer-point would somehow
say "Ah, window configurations!  That's what's going wrong".  Maybe we
could fix it by documenting the precise situation in the Elisp manual,
possibly combined with making edebug-save-windows nil by default.

Or maybe the patch to the code is a safer, more direct fix.  After all,
edebug is the system debugger, the tool of last resort.  It should not
fail at all.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Stefan Monnier]

> Why does set-window-configuration overwrite the buffer-points?  The
> window configuration does not contain them.  The code just assumes that
> the buffer-point should be set to the window point.  Of course, we have
> a race condition if a buffer is displayed in several windows.  So this
> would appear to be a bug, the root cause of the bug in this thread.

This suggests the patch below, right?
I note that this only changes the buffer-point for `current-buffer`, not
for all the buffers displayed in the window-config, right?
There's still a "race condition", of course.

> Maybe set-window-configuration should be amended not to write the
> buffer-points?  That might cause problems in other areas, though.  The
> window configuration is one of the few areas where the documentation is
> poor enough that you need to read the C source to find out what it's
> really doing.

Yup.  We could start by providing some way to tell
`set-window-configuration` not to change buffer-points (and use that in
Edebug)?  This way we fix the problem for Edebug without risking
changes elsewhere?

We can try and run out own Emacs with the patch installed, to see if we
notice any regression.  If we do, that might help us understand what we
should do.  If we don't, maybe it's hint that it was really just a bug.


        Stefan


diff --git a/src/window.c b/src/window.c
index b858d145415..382d3cbdc6a 100644
--- a/src/window.c
+++ b/src/window.c
@@ -7270,12 +7270,6 @@ DEFUN ("set-window-configuration", Fset_window_configuration,
 	      set_marker_restricted (w->start, p->start, w->contents);
 	      set_marker_restricted (w->pointm, p->pointm, w->contents);
 	      set_marker_restricted (w->old_pointm, p->old_pointm, w->contents);
-	      /* As documented in Fcurrent_window_configuration, don't
-		 restore the location of point in the buffer which was
-		 current when the window configuration was recorded.  */
-	      if (!EQ (p->buffer, new_current_buffer)
-		  && XBUFFER (p->buffer) == current_buffer)
-		Fgoto_char (w->pointm);
 	    }
 	  else if (BUFFERP (w->contents) && BUFFER_LIVE_P (XBUFFER (w->contents)))
 	    /* Keep window's old buffer; make sure the markers are real.  */

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Stefan.

On Tue, Nov 01, 2022 at 15:47:45 -0400, Stefan Monnier wrote:
> > Why does set-window-configuration overwrite the buffer-points?  The
> > window configuration does not contain them.  The code just assumes that
> > the buffer-point should be set to the window point.  Of course, we have
> > a race condition if a buffer is displayed in several windows.  So this
> > would appear to be a bug, the root cause of the bug in this thread.

> This suggests the patch below, right?
> I note that this only changes the buffer-point for `current-buffer`, not
> for all the buffers displayed in the window-config, right?

Not quite.  It changes the buffer-point for every buffer except the
"current buffer".

> There's still a "race condition", of course.

> > Maybe set-window-configuration should be amended not to write the
> > buffer-points?  That might cause problems in other areas, though.  The
> > window configuration is one of the few areas where the documentation is
> > poor enough that you need to read the C source to find out what it's
> > really doing.

> Yup.  We could start by providing some way to tell
> `set-window-configuration` not to change buffer-points (and use that in
> Edebug)?  This way we fix the problem for Edebug without risking
> changes elsewhere?

An &optional parameter, you mean?  I'd thought of that, but it feels
ugly.

> We can try and run out own Emacs with the patch installed, to see if we
> notice any regression.

Indeed, yes.  I count 89 occurrences of '(set-window-configuration' in
our Lisp sources.  That's not really a comfortable number to check by
looking at the code.  :-(

> If we do, that might help us understand what we should do.  If we
> don't, maybe it's hint that it was really just a bug.

It feels like a bug for the reasons already given, and the fact that the
"current buffer"'s buffer-point is spared.  If there's something wrong
with overwriting that buffer's buffer-point, why is it OK for all the
other buffers in windows in the window-configuration?

I'm going to try out your patch and see if it, by itself, fixes the bug,
now that I've got a reproducible test case.

<...later...>

I've tried it, and the patch doesn't fix the bug.  :-(  Something else
is clearly overwriting the buffer-point.

>         Stefan


> diff --git a/src/window.c b/src/window.c
> index b858d145415..382d3cbdc6a 100644
> --- a/src/window.c
> +++ b/src/window.c
> @@ -7270,12 +7270,6 @@ DEFUN ("set-window-configuration", Fset_window_configuration,
>  	      set_marker_restricted (w->start, p->start, w->contents);
>  	      set_marker_restricted (w->pointm, p->pointm, w->contents);
>  	      set_marker_restricted (w->old_pointm, p->old_pointm, w->contents);
> -	      /* As documented in Fcurrent_window_configuration, don't
> -		 restore the location of point in the buffer which was
> -		 current when the window configuration was recorded.  */
> -	      if (!EQ (p->buffer, new_current_buffer)
> -		  && XBUFFER (p->buffer) == current_buffer)
> -		Fgoto_char (w->pointm);
>  	    }
>  	  else if (BUFFERP (w->contents) && BUFFER_LIVE_P (XBUFFER (w->contents)))
>  	    /* Keep window's old buffer; make sure the markers are real.  */

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Stefan Monnier]

>> > Why does set-window-configuration overwrite the buffer-points?
>> > The window configuration does not contain them.  The code just
>> > assumes that the buffer-point should be set to the window point.
>> > Of course, we have a race condition if a buffer is displayed in
>> > several windows.  So this would appear to be a bug, the root cause
>> > of the bug in this thread.
>
>> This suggests the patch below, right?
>> I note that this only changes the buffer-point for `current-buffer`,
>> not for all the buffers displayed in the window-config, right?
>
> Not quite.  It changes the buffer-point for every buffer except the
> "current buffer".

Really?  My reading of the code:

	      /* As documented in Fcurrent_window_configuration, don't
		 restore the location of point in the buffer which was
		 current when the window configuration was recorded.  */
	      if (!EQ (p->buffer, new_current_buffer)
		  && XBUFFER (p->buffer) == current_buffer)
		Fgoto_char (w->pointm);

is that it's done only for the current buffer and only if it's different
from the "to be current buffer".

Am I missing something?

>> Yup.  We could start by providing some way to tell
>> `set-window-configuration` not to change buffer-points (and use that in
>> Edebug)?  This way we fix the problem for Edebug without risking
>> changes elsewhere?
> An &optional parameter, you mean?  I'd thought of that, but it feels
> ugly.

Agreed.  Especially since I get the feeling that it's just a plain bug.

That piece of code dates back to the initial revision of window.c back
in 1991, tho.  That function had some serious bugs in the handling of
buffer points which stayed unnoticed for years (I remember the one
I fixed with in 2005 with e67a1dea3e622d61024b2dc17c36831d048bb271), so
I wouldn't be surprised that this one is also a mistake.

> I've tried it, and the patch doesn't fix the bug.  :-(

Why didn't you say so at the beginning of your message?
Now I look like fool!  :-)


        Stefan

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Stefan.

On Tue, Nov 01, 2022 at 17:51:45 -0400, Stefan Monnier wrote:
> >> > Why does set-window-configuration overwrite the buffer-points?
> >> > The window configuration does not contain them.  The code just
> >> > assumes that the buffer-point should be set to the window point.
> >> > Of course, we have a race condition if a buffer is displayed in
> >> > several windows.  So this would appear to be a bug, the root cause
> >> > of the bug in this thread.

> >> This suggests the patch below, right?
> >> I note that this only changes the buffer-point for `current-buffer`,
> >> not for all the buffers displayed in the window-config, right?

> > Not quite.  It changes the buffer-point for every buffer except the
> > "current buffer".

> Really?  My reading of the code:

> 	      /* As documented in Fcurrent_window_configuration, don't
> 		 restore the location of point in the buffer which was
> 		 current when the window configuration was recorded.  */
> 	      if (!EQ (p->buffer, new_current_buffer)
> 		  && XBUFFER (p->buffer) == current_buffer)
> 		Fgoto_char (w->pointm);

> is that it's done only for the current buffer and only if it's different
> from the "to be current buffer".

> Am I missing something?

Hmm.  I spent a great deal of yesterday asserting false things, then
apologising for them.  The above was the last such false thing, for which
I also apologise.

I think I was influenced by the doc string of
current-window-configuration, which seems to imply what I wrote above.

> >> Yup.  We could start by providing some way to tell
> >> `set-window-configuration` not to change buffer-points (and use that in
> >> Edebug)?  This way we fix the problem for Edebug without risking
> >> changes elsewhere?
> > An &optional parameter, you mean?  I'd thought of that, but it feels
> > ugly.

> Agreed.  Especially since I get the feeling that it's just a plain bug.

> That piece of code dates back to the initial revision of window.c back
> in 1991, tho.  That function had some serious bugs in the handling of
> buffer points which stayed unnoticed for years (I remember the one
> I fixed with in 2005 with e67a1dea3e622d61024b2dc17c36831d048bb271), so
> I wouldn't be surprised that this one is also a mistake.

> > I've tried it, and the patch doesn't fix the bug.  :-(

> Why didn't you say so at the beginning of your message?
> Now I look like fool!  :-)

The fundamental problem is a conceptual one:- so many bits of code
wrongly take the liberty of writing to buffer point when they have no
business doing so.  The only code which should transfer window point to
buffer point is the command loop (or maybe redisplay) when the window is
about to become the one that the user will edit in.  Or something like
that.  There's clearly been a lot of confusion about window/buffer point
over the decades which shows in the number of places such changes in
buffer point occur, and the bugs which have sometimes resulted, like the
one you cite above.

However, even I can see that it is impracticable to try to fix this now.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Stefan Monnier]

>> Really?  My reading of the code:
>
>> 	      /* As documented in Fcurrent_window_configuration, don't
>> 		 restore the location of point in the buffer which was
>> 		 current when the window configuration was recorded.  */
>> 	      if (!EQ (p->buffer, new_current_buffer)
>> 		  && XBUFFER (p->buffer) == current_buffer)
>> 		Fgoto_char (w->pointm);
>
>> is that it's done only for the current buffer and only if it's different
>> from the "to be current buffer".
>
>> Am I missing something?
>
> Hmm.  I spent a great deal of yesterday asserting false things, then
> apologising for them.  The above was the last such false thing, for which
> I also apologise.

If we had to apologize every time we misread/misunderstood code, we'd
never get anywhere :-)

> There's clearly been a lot of confusion about window/buffer point over
> the decades which shows in the number of places such changes in buffer
> point occur, and the bugs which have sometimes resulted, like the one
> you cite above.

This is arguably one of the most subtle/delicate/complex aspect of
ELisp's semantics, indeed.


        Stefan

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Wed, 02 Nov 2022 09:12:57 -0400
> 
> > There's clearly been a lot of confusion about window/buffer point over
> > the decades which shows in the number of places such changes in buffer
> > point occur, and the bugs which have sometimes resulted, like the one
> > you cite above.
> 
> This is arguably one of the most subtle/delicate/complex aspect of
> ELisp's semantics, indeed.

Indeed.  And of Emacs in general.  And if we really want to have a
serious discussion of those aspects, we must bring Martin Rudalics on
board.

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Tue, 01 Nov 2022 15:47:45 -0400
> 
> > Why does set-window-configuration overwrite the buffer-points?  The
> > window configuration does not contain them.  The code just assumes that
> > the buffer-point should be set to the window point.  Of course, we have
> > a race condition if a buffer is displayed in several windows.  So this
> > would appear to be a bug, the root cause of the bug in this thread.
> 
> This suggests the patch below, right?

I hope not.  We should not change behavior of set-window-configuration
easily, certainly not due to this problem.  I'm against this patch,
sorry.

Re: Edebug corrupting point in buffers.

[Stefan Monnier]

>> > Why does set-window-configuration overwrite the buffer-points?  The
>> > window configuration does not contain them.  The code just assumes that
>> > the buffer-point should be set to the window point.  Of course, we have
>> > a race condition if a buffer is displayed in several windows.  So this
>> > would appear to be a bug, the root cause of the bug in this thread.
>> 
>> This suggests the patch below, right?
>
> I hope not.  We should not change behavior of set-window-configuration
> easily, certainly not due to this problem.  I'm against this patch,
> sorry.

I am against it as well.  I was mostly using it to ask confirm I had
correctly understood the piece of code we were talking about.


        Stefan

Re: Edebug corrupting point in buffers.

[Juri Linkov]

> Maybe we could fix it by documenting the precise situation in the
> Elisp manual, possibly combined with making edebug-save-windows nil
> by default.

I see the same problem even when edebug-save-windows is nil:

0. emacs -Q
1. paste:

(with-temp-buffer
  (save-excursion (insert "a\nb\nc\nd\n"))
  (dotimes (i 4)
    (forward-line)))

2. 'C-u C-M-x' on it
3. step once with SPC
4. M-x pop-to-buffer RET  *temp* RET
5. 'W' (edebug-toggle-save-windows)
6. then stepping thru the code doesn't move point in another window

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Wed, 02 Nov 2022 19:40:24 +0200
> 
> 0. emacs -Q
> 1. paste:
> 
> (with-temp-buffer
>   (save-excursion (insert "a\nb\nc\nd\n"))
>   (dotimes (i 4)
>     (forward-line)))
> 
> 2. 'C-u C-M-x' on it
> 3. step once with SPC
> 4. M-x pop-to-buffer RET  *temp* RET
> 5. 'W' (edebug-toggle-save-windows)
> 6. then stepping thru the code doesn't move point in another window

So where's the bug here, may I ask?  This is normal Edebug behavior,
AFAIR.  If you want to see point move, you need to switch to that
buffer, not just display it.

Re: Edebug corrupting point in buffers.

[Juri Linkov]

>> (with-temp-buffer
>>   (save-excursion (insert "a\nb\nc\nd\n"))
>>   (dotimes (i 4)
>>     (forward-line)))
>> 
>> 2. 'C-u C-M-x' on it
>> 3. step once with SPC
>> 4. M-x pop-to-buffer RET  *temp* RET
>> 5. 'W' (edebug-toggle-save-windows)
>> 6. then stepping thru the code doesn't move point in another window
>
> So where's the bug here, may I ask?  This is normal Edebug behavior,
> AFAIR.  If you want to see point move, you need to switch to that
> buffer, not just display it.

I don't understand how to step through the code after switching to
*temp* buffer while allowing point to move.

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: acm@muc.de,  emacs-devel@gnu.org
> Date: Wed, 02 Nov 2022 20:36:08 +0200
> 
> I don't understand how to step through the code after switching to
> *temp* buffer while allowing point to move.

If you want to _see_ the cursor moving, you need to switch to the
buffer each time point moves.

Re: Edebug corrupting point in buffers.

[Juri Linkov]

>> I don't understand how to step through the code after switching to
>> *temp* buffer while allowing point to move.
>
> If you want to _see_ the cursor moving, you need to switch to the
> buffer each time point moves.

Switching to the buffer doesn't help to move point.
And edebug-save-windows is nil.

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: acm@muc.de,  emacs-devel@gnu.org
> Date: Thu, 03 Nov 2022 19:25:46 +0200
> 
> >> I don't understand how to step through the code after switching to
> >> *temp* buffer while allowing point to move.
> >
> > If you want to _see_ the cursor moving, you need to switch to the
> > buffer each time point moves.
> 
> Switching to the buffer doesn't help to move point.

It does here: every time I switch to that buffer during forward-line
calls, I see point moved to a new position.

Re: Edebug corrupting point in buffers.

[Juri Linkov]

>> >> I don't understand how to step through the code after switching to
>> >> *temp* buffer while allowing point to move.
>> >
>> > If you want to _see_ the cursor moving, you need to switch to the
>> > buffer each time point moves.
>>
>> Switching to the buffer doesn't help to move point.
>
> It does here: every time I switch to that buffer during forward-line
> calls, I see point moved to a new position.

I tried again in emacs -Q with edebug-save-windows enabled and disabled,
but point doesn't move after switching to *temp* buffer.

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Eli.

On Tue, Nov 01, 2022 at 19:57:11 +0200, Eli Zaretskii wrote:
> > Date: Tue, 1 Nov 2022 17:24:26 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> This now gets me back to the inability to reproduce the problem with
> your recipe.  If that depends on edebug-save-windows, not on
> edebug-save-displayed-buffer-points, and since edebug-save-windows is
> t by default, why wasn't I able to reproduce the problem?

I've stumbled across a possible reason why you couldn't reproduce the
bug.  There was a crucial step missing from the recipe.  I've inserted
that step into the previous recipe as (iv)a:

With test-edebug.el being
#########################################################################
(defun test-edebug ()
  (let ((A "*scratch*") (B "emacs.README"))
    (set-buffer A)
    (set-buffer B)
    (goto-char (point-max))
    (insert "(2022-11-01)\n")
    ;; B's buffer-point is at point-max.

    (set-buffer A)
    (set-buffer B)
    ;; B's buffer-point is no longer at point-max.
    (insert "(2022-11-01)a\n")))
#########################################################################
,
(i) Emacs -Q.
(ii) On a single frame, arrange buffers *scratch*, test-edebug.el, and
  some other substantial buffer, that I call emacs.README.
(iii) Put point in emacs.README somewhere other than point-max.
(iv) Instrument test-edebug for edebug with C-u C-M-x.
(iv)a Put point into window *scratch*.
(v) M-: (test-edebug).
(vi) Step through test-edebug using the space key.
(vii) Note that the second text insertion happens where point was in the
  window, not at point-max.  This is the bug.

There are bits of code that don't restore buffer-point in the current
buffer.  One of these is probably the explanation why the bug doesn't
trigger with point starting in emacs.README.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> Date: Wed, 2 Nov 2022 11:34:37 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> (i) Emacs -Q.
> (ii) On a single frame, arrange buffers *scratch*, test-edebug.el, and
>   some other substantial buffer, that I call emacs.README.
> (iii) Put point in emacs.README somewhere other than point-max.
> (iv) Instrument test-edebug for edebug with C-u C-M-x.
> (iv)a Put point into window *scratch*.
> (v) M-: (test-edebug).
> (vi) Step through test-edebug using the space key.
> (vii) Note that the second text insertion happens where point was in the
>   window, not at point-max.  This is the bug.

Yes, I see the problem, but setting edebug-save-windows to nil
eliminates it.  So I think we already have a solution for the rare
situations where this is an issue.

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Eli.

On Wed, Nov 02, 2022 at 16:00:52 +0200, Eli Zaretskii wrote:
> > Date: Wed, 2 Nov 2022 11:34:37 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > (i) Emacs -Q.
> > (ii) On a single frame, arrange buffers *scratch*, test-edebug.el, and
> >   some other substantial buffer, that I call emacs.README.
> > (iii) Put point in emacs.README somewhere other than point-max.
> > (iv) Instrument test-edebug for edebug with C-u C-M-x.
> > (iv)a Put point into window *scratch*.
> > (v) M-: (test-edebug).
> > (vi) Step through test-edebug using the space key.
> > (vii) Note that the second text insertion happens where point was in the
> >   window, not at point-max.  This is the bug.

> Yes, I see the problem, but setting edebug-save-windows to nil
> eliminates it.  So I think we already have a solution for the rare
> situations where this is an issue.

It remains a perplexing problem for those who stumble into it.  How about
instead of patching the code, adding some documentation clarifying the
problem?

I would propose the following:



diff --git a/doc/lispref/edebug.texi b/doc/lispref/edebug.texi
index 6a51489d8a..b4f680fe86 100644
--- a/doc/lispref/edebug.texi
+++ b/doc/lispref/edebug.texi
@@ -1019,6 +1019,7 @@ The Outside Context
 @menu
 * Checking Whether to Stop::    When Edebug decides what to do.
 * Edebug Display Update::       When Edebug updates the display.
+* Edebug Buffer Points::        When @code{point} gets corrupted.
 * Edebug Recursive Edit::       When Edebug stops execution.
 @end menu
 
@@ -1100,6 +1101,41 @@ Edebug Display Update
 the cursor shows up in the window.
 @end itemize
 
+@node Edebug Buffer Points
+@subsubsection Edebug's handling of buffer points
+
+When Edebug enters its recursive edit to get a command from the user,
+by default it saves the window points of each window in the selected
+frame (@pxref{Edebug Display Update}).  These are usually, but not
+always, the same as the values of point in the buffers displayed by
+these windows (@pxref{Window Point}).  On leaving the recursive edit,
+these window points get restored, but sometimes buffer points get
+overwritten by them too.
+
+This can be a problem when your program itself sets point in a buffer,
+intending later to use that value of point.  For example, suppose you
+have buffer B displayed in your selected frame, and you step through
+the following Lisp fragment:
+
+@example
+(set-buffer A)
+(set-buffer B)
+(goto-char (point-max))
+(insert "foo")
+(set-buffer A)
+(set-buffer B)
+(insert "bar")
+@end example
+
+@noindent
+``foo'' gets inserted at @code{point-max} as intended, but ``bar''
+sometimes gets wrongly inserted at the window point of the window
+displaying buffer B.
+
+The only known workaround for this problem is to disable
+@code{edebug-save-windows}, for example with the command @kbd{W}
+(@pxref{Edebug Options}).
+
 @node Edebug Recursive Edit
 @subsubsection Edebug Recursive Edit
 
diff --git a/doc/lispref/elisp.texi b/doc/lispref/elisp.texi
index a3d1d80408..b07b1e28cd 100644
--- a/doc/lispref/elisp.texi
+++ b/doc/lispref/elisp.texi
@@ -714,6 +714,7 @@ Top
 
 * Checking Whether to Stop::When Edebug decides what to do.
 * Edebug Display Update::   When Edebug updates the display.
+* Edebug Buffer Points::    When @code{point} gets corrupted.
 * Edebug Recursive Edit::   When Edebug stops execution.
 
 Edebug and Macros


-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> Date: Wed, 2 Nov 2022 16:18:24 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > Yes, I see the problem, but setting edebug-save-windows to nil
> > eliminates it.  So I think we already have a solution for the rare
> > situations where this is an issue.
> 
> It remains a perplexing problem for those who stumble into it.  How about
> instead of patching the code, adding some documentation clarifying the
> problem?
> 
> I would propose the following:

Fine by me, but...

> diff --git a/doc/lispref/edebug.texi b/doc/lispref/edebug.texi
> index 6a51489d8a..b4f680fe86 100644
> --- a/doc/lispref/edebug.texi
> +++ b/doc/lispref/edebug.texi
> @@ -1019,6 +1019,7 @@ The Outside Context
>  @menu
>  * Checking Whether to Stop::    When Edebug decides what to do.
>  * Edebug Display Update::       When Edebug updates the display.
> +* Edebug Buffer Points::        When @code{point} gets corrupted.
>  * Edebug Recursive Edit::       When Edebug stops execution.
>  @end menu

...please document this in the same place where the variables we
discussed are described.  It is not a good idea from the didactic POV
to have this described separately.

> +This can be a problem when your program itself sets point in a buffer,
> +intending later to use that value of point.  For example, suppose you
> +have buffer B displayed in your selected frame, and you step through
> +the following Lisp fragment:
> +
> +@example
> +(set-buffer A)
> +(set-buffer B)
> +(goto-char (point-max))
> +(insert "foo")
> +(set-buffer A)
> +(set-buffer B)
> +(insert "bar")
> +@end example

I very much doubt that showing this example will help.  It just
muddies the water, b y forcing users to try to understand what the
example does, which is completely irrelevant to what we want to
convey.

Just say that if point gets reset in non-selected windows during
Edebug stepping, users should try customizing that variable.

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Eli.

On Wed, Nov 02, 2022 at 18:57:39 +0200, Eli Zaretskii wrote:
> > Date: Wed, 2 Nov 2022 16:18:24 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > Yes, I see the problem, but setting edebug-save-windows to nil
> > > eliminates it.  So I think we already have a solution for the rare
> > > situations where this is an issue.

> > It remains a perplexing problem for those who stumble into it.  How about
> > instead of patching the code, adding some documentation clarifying the
> > problem?

> > I would propose the following:

> Fine by me, but...

> > diff --git a/doc/lispref/edebug.texi b/doc/lispref/edebug.texi
> > index 6a51489d8a..b4f680fe86 100644
> > --- a/doc/lispref/edebug.texi
> > +++ b/doc/lispref/edebug.texi
> > @@ -1019,6 +1019,7 @@ The Outside Context
> >  @menu
> >  * Checking Whether to Stop::    When Edebug decides what to do.
> >  * Edebug Display Update::       When Edebug updates the display.
> > +* Edebug Buffer Points::        When @code{point} gets corrupted.
> >  * Edebug Recursive Edit::       When Edebug stops execution.
> >  @end menu

> ...please document this in the same place where the variables we
> discussed are described.  It is not a good idea from the didactic POV
> to have this described separately.

The place I put it, under "The Outside Context" has as its purpose
"Edebug tries to be transparent to the program you are debugging, but it
does not succeed completely. ..... This section explains precisely what
context Edebug restores, and how Edebug fails to be completely
transparent."

The current phenomenon fits precisely into that category.  Surely we need
to describe it there, otherwise the section will be incomplete.

I envisage the main purpose of this new documentation is not for somebody
calmly reading through the manual learning about edebug-save-windows.
Rather it's for somebody in a highly emotional state, who's just had
edebug corrupt his buffers due to an apparent bug in edebug.  I think
this user is more likely to find the new doc quickly where my patch puts
it.

On the other hand, as you say, we do need a description in the list item
for edebug-save-windows in "Edebug Options" too.

> > +This can be a problem when your program itself sets point in a buffer,
> > +intending later to use that value of point.  For example, suppose you
> > +have buffer B displayed in your selected frame, and you step through
> > +the following Lisp fragment:
> > +
> > +@example
> > +(set-buffer A)
> > +(set-buffer B)
> > +(goto-char (point-max))
> > +(insert "foo")
> > +(set-buffer A)
> > +(set-buffer B)
> > +(insert "bar")
> > +@end example

> I very much doubt that showing this example will help.  It just
> muddies the water, b y forcing users to try to understand what the
> example does, which is completely irrelevant to what we want to
> convey.

OK, I'll take it out, together with the description.  Instead I'll insert
something vague about switching buffers and moving point in them.

> Just say that if point gets reset in non-selected windows during
> Edebug stepping, users should try customizing that variable.

I'll do that.


Here's an amended patch, somewhat shorter than yesterday's:



diff --git a/doc/lispref/edebug.texi b/doc/lispref/edebug.texi
index 6a51489d8a..aab8db989a 100644
--- a/doc/lispref/edebug.texi
+++ b/doc/lispref/edebug.texi
@@ -1019,6 +1019,7 @@ The Outside Context
 @menu
 * Checking Whether to Stop::    When Edebug decides what to do.
 * Edebug Display Update::       When Edebug updates the display.
+* Edebug Buffer Points::        When @code{point} gets corrupted.
 * Edebug Recursive Edit::       When Edebug stops execution.
 @end menu
 
@@ -1100,6 +1101,22 @@ Edebug Display Update
 the cursor shows up in the window.
 @end itemize
 
+@node Edebug Buffer Points
+@subsubsection Edebug's handling of buffer points
+
+When Edebug enters its recursive edit to get a command from the user,
+by default it saves the window points of each window in the selected
+frame (@pxref{Edebug Display Update}).  These are usually, but not
+always, the same as the values of point in the buffers displayed by
+these windows (@pxref{Window Point}).  On leaving the recursive edit,
+these window points get restored, but sometimes buffer points get
+overwritten by them too.
+
+This can occasionally be a problem when your program switches buffers
+and sets point in them.  The recommended workaround is to disable the
+option @code{edebug-save-windows}, for example with the command
+@kbd{W} (@pxref{Edebug Options}).
+
 @node Edebug Recursive Edit
 @subsubsection Edebug Recursive Edit
 
@@ -1657,6 +1674,11 @@ Edebug Options
 what happens to the window configurations, it is better to set this
 variable to @code{nil}.
 
+Saving the window configuration can sometimes corrupt the values of
+point in buffers displayed by these windows.  If this happens, you are
+recommended to set @code{edebug-save-windos} to @code{nil}.
+@xref{Edebug Buffer Points}.
+
 If the value is a list, only the listed windows are saved and
 restored.
 
diff --git a/doc/lispref/elisp.texi b/doc/lispref/elisp.texi
index a3d1d80408..b07b1e28cd 100644
--- a/doc/lispref/elisp.texi
+++ b/doc/lispref/elisp.texi
@@ -714,6 +714,7 @@ Top
 
 * Checking Whether to Stop::When Edebug decides what to do.
 * Edebug Display Update::   When Edebug updates the display.
+* Edebug Buffer Points::    When @code{point} gets corrupted.
 * Edebug Recursive Edit::   When Edebug stops execution.
 
 Edebug and Macros


-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> Date: Thu, 3 Nov 2022 11:32:29 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> +@node Edebug Buffer Points
> +@subsubsection Edebug's handling of buffer points
> +
> +When Edebug enters its recursive edit to get a command from the user,
> +by default it saves the window points of each window in the selected
> +frame (@pxref{Edebug Display Update}).  These are usually, but not
> +always, the same as the values of point in the buffers displayed by
> +these windows (@pxref{Window Point}).  On leaving the recursive edit,
> +these window points get restored, but sometimes buffer points get
> +overwritten by them too.
> +
> +This can occasionally be a problem when your program switches buffers
> +and sets point in them.  The recommended workaround is to disable the
> +option @code{edebug-save-windows}, for example with the command
> +@kbd{W} (@pxref{Edebug Options}).
> +
>  @node Edebug Recursive Edit
>  @subsubsection Edebug Recursive Edit
>  
> @@ -1657,6 +1674,11 @@ Edebug Options
>  what happens to the window configurations, it is better to set this
>  variable to @code{nil}.
>  
> +Saving the window configuration can sometimes corrupt the values of
> +point in buffers displayed by these windows.  If this happens, you are
> +recommended to set @code{edebug-save-windos} to @code{nil}.
> +@xref{Edebug Buffer Points}.
> +

The node you added is very short, barely a dozen lines.  It makes
little sense to have it separate from where edebug-save-windows is
described.  So I think you should move it there.  The location of the
node inside the manual's hierarchy is much less important than to have
the information pertaining to edebug-save-windows in a single place,
because no one reads the ELisp reference manual in its entirety.  The
only thing we need to facilitate people finding this place is add good
index entries there.

Thanks.

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Eli.

On Thu, Nov 03, 2022 at 15:29:57 +0200, Eli Zaretskii wrote:
> > Date: Thu, 3 Nov 2022 11:32:29 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > +@node Edebug Buffer Points
> > +@subsubsection Edebug's handling of buffer points
> > +
> > +When Edebug enters its recursive edit to get a command from the user,
> > +by default it saves the window points of each window in the selected
> > +frame (@pxref{Edebug Display Update}).  These are usually, but not
> > +always, the same as the values of point in the buffers displayed by
> > +these windows (@pxref{Window Point}).  On leaving the recursive edit,
> > +these window points get restored, but sometimes buffer points get
> > +overwritten by them too.
> > +
> > +This can occasionally be a problem when your program switches buffers
> > +and sets point in them.  The recommended workaround is to disable the
> > +option @code{edebug-save-windows}, for example with the command
> > +@kbd{W} (@pxref{Edebug Options}).
> > +
> >  @node Edebug Recursive Edit
> >  @subsubsection Edebug Recursive Edit

> > @@ -1657,6 +1674,11 @@ Edebug Options
> >  what happens to the window configurations, it is better to set this
> >  variable to @code{nil}.

> > +Saving the window configuration can sometimes corrupt the values of
> > +point in buffers displayed by these windows.  If this happens, you are
> > +recommended to set @code{edebug-save-windos} to @code{nil}.
> > +@xref{Edebug Buffer Points}.
> > +

> The node you added is very short, barely a dozen lines.  It makes
> little sense to have it separate from where edebug-save-windows is
> described.  So I think you should move it there.  The location of the
> node inside the manual's hierarchy is much less important than to have
> the information pertaining to edebug-save-windows in a single place,
> because no one reads the ELisp reference manual in its entirety.  The
> only thing we need to facilitate people finding this place is add good
> index entries there.

So you're proposing leaving the "The outside context" node incomplete,
according to its clearly defined purpose, and therefore wrong?  Why?

Remember, this patch is not about edebug-save-windows.  It's about point
getting corrupted.

> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> Date: Thu, 3 Nov 2022 18:07:16 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > The node you added is very short, barely a dozen lines.  It makes
> > little sense to have it separate from where edebug-save-windows is
> > described.  So I think you should move it there.  The location of the
> > node inside the manual's hierarchy is much less important than to have
> > the information pertaining to edebug-save-windows in a single place,
> > because no one reads the ELisp reference manual in its entirety.  The
> > only thing we need to facilitate people finding this place is add good
> > index entries there.
> 
> So you're proposing leaving the "The outside context" node incomplete,
> according to its clearly defined purpose, and therefore wrong?  Why?

If you want, you can add a short sentence there about the issue, with
a cross-reference to where the issue is described in full.

This is how we organize our manuals: when some topic could be relevant
to more than one situation, we describe it in full in one place, and
have short references in all the others.

> Remember, this patch is not about edebug-save-windows.  It's about point
> getting corrupted.

The index entries and the cross-references should solve this.  And the
issue _is_ related to edebug-save-windows and to the other similar
option described in the same node.  So having all of this info there
makes the description more comprehensive.

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Eli.

On Thu, Nov 03, 2022 at 20:15:08 +0200, Eli Zaretskii wrote:
> > Date: Thu, 3 Nov 2022 18:07:16 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > The node you added is very short, barely a dozen lines.  It makes
> > > little sense to have it separate from where edebug-save-windows is
> > > described.  So I think you should move it there.  The location of the
> > > node inside the manual's hierarchy is much less important than to have
> > > the information pertaining to edebug-save-windows in a single place,
> > > because no one reads the ELisp reference manual in its entirety.  The
> > > only thing we need to facilitate people finding this place is add good
> > > index entries there.

> > So you're proposing leaving the "The outside context" node incomplete,
> > according to its clearly defined purpose, and therefore wrong?  Why?

> If you want, you can add a short sentence there about the issue, with
> a cross-reference to where the issue is described in full.

"There"?  There is no suitable place to put such a link, other than my
new node.  Such a strategy would unbalance "The Outside Context" by
having most of its contents in subsubsections, and the bit about point
corruption at the other end of a link, in some random page.

As a matter of interest, one of the other nodes under "The Outside
Context", namely "Checking Whether to Stop" has just 13 lines.

> This is how we organize our manuals: when some topic could be relevant
> to more than one situation, we describe it in full in one place, and
> have short references in all the others.

We should describe it in the PRIMARY relevant place.

> > Remember, this patch is not about edebug-save-windows.  It's about point
> > getting corrupted.

> The index entries and the cross-references should solve this.  And the
> issue _is_ related to edebug-save-windows ....

It is only tangentially related to edebug-save-windows.  It is about
point getting corrupted.  An angry victim of this bug should be be able
to find the description by searching for "corrupt".

> .... and to the other similar option described in the same node.  So
> having all of this info there makes the description more
> comprehensive.

Yes, stuff about options belongs in the "Options" page.  Stuff about
point getting corrupted does not, except at the other end of a link.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> Date: Thu, 3 Nov 2022 20:25:12 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > > > The node you added is very short, barely a dozen lines.  It makes
> > > > little sense to have it separate from where edebug-save-windows is
> > > > described.  So I think you should move it there.  The location of the
> > > > node inside the manual's hierarchy is much less important than to have
> > > > the information pertaining to edebug-save-windows in a single place,
> > > > because no one reads the ELisp reference manual in its entirety.  The
> > > > only thing we need to facilitate people finding this place is add good
> > > > index entries there.
> 
> > > So you're proposing leaving the "The outside context" node incomplete,
> > > according to its clearly defined purpose, and therefore wrong?  Why?
> 
> > If you want, you can add a short sentence there about the issue, with
> > a cross-reference to where the issue is described in full.
> 
> "There"?  There is no suitable place to put such a link, other than my
> new node.  Such a strategy would unbalance "The Outside Context" by
> having most of its contents in subsubsections, and the bit about point
> corruption at the other end of a link, in some random page.
> 
> As a matter of interest, one of the other nodes under "The Outside
> Context", namely "Checking Whether to Stop" has just 13 lines.
> 
> > This is how we organize our manuals: when some topic could be relevant
> > to more than one situation, we describe it in full in one place, and
> > have short references in all the others.
> 
> We should describe it in the PRIMARY relevant place.
> 
> > > Remember, this patch is not about edebug-save-windows.  It's about point
> > > getting corrupted.
> 
> > The index entries and the cross-references should solve this.  And the
> > issue _is_ related to edebug-save-windows ....
> 
> It is only tangentially related to edebug-save-windows.  It is about
> point getting corrupted.  An angry victim of this bug should be be able
> to find the description by searching for "corrupt".
> 
> > .... and to the other similar option described in the same node.  So
> > having all of this info there makes the description more
> > comprehensive.
> 
> Yes, stuff about options belongs in the "Options" page.  Stuff about
> point getting corrupted does not, except at the other end of a link.

Instead of continuing this endless argument, I prefer to fix this
myself, using your text where appropriate.  Are you okay with that?

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Eli.

On Sat, Nov 05, 2022 at 13:24:06 +0200, Eli Zaretskii wrote:
> > Date: Thu, 3 Nov 2022 20:25:12 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > > > The node you added is very short, barely a dozen lines.  It makes
> > > > > little sense to have it separate from where edebug-save-windows is
> > > > > described.  So I think you should move it there.  The location of the
> > > > > node inside the manual's hierarchy is much less important than to have
> > > > > the information pertaining to edebug-save-windows in a single place,
> > > > > because no one reads the ELisp reference manual in its entirety.  The
> > > > > only thing we need to facilitate people finding this place is add good
> > > > > index entries there.

> > > > So you're proposing leaving the "The outside context" node incomplete,
> > > > according to its clearly defined purpose, and therefore wrong?  Why?

> > > If you want, you can add a short sentence there about the issue, with
> > > a cross-reference to where the issue is described in full.

> > "There"?  There is no suitable place to put such a link, other than my
> > new node.  Such a strategy would unbalance "The Outside Context" by
> > having most of its contents in subsubsections, and the bit about point
> > corruption at the other end of a link, in some random page.

> > As a matter of interest, one of the other nodes under "The Outside
> > Context", namely "Checking Whether to Stop" has just 13 lines.

> > > This is how we organize our manuals: when some topic could be relevant
> > > to more than one situation, we describe it in full in one place, and
> > > have short references in all the others.

> > We should describe it in the PRIMARY relevant place.

> > > > Remember, this patch is not about edebug-save-windows.  It's about point
> > > > getting corrupted.

> > > The index entries and the cross-references should solve this.  And the
> > > issue _is_ related to edebug-save-windows ....

> > It is only tangentially related to edebug-save-windows.  It is about
> > point getting corrupted.  An angry victim of this bug should be be able
> > to find the description by searching for "corrupt".

> > > .... and to the other similar option described in the same node.  So
> > > having all of this info there makes the description more
> > > comprehensive.

> > Yes, stuff about options belongs in the "Options" page.  Stuff about
> > point getting corrupted does not, except at the other end of a link.

> Instead of continuing this endless argument, I prefer to fix this
> myself, using your text where appropriate.  Are you okay with that?

Yes, I think that would be best.  Please do take account of my points
about the "The Outside Context", and put the phrase "corrupted point" (or
something like it) in somewhere.

Thanks!

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> Date: Sat, 5 Nov 2022 16:50:44 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > Instead of continuing this endless argument, I prefer to fix this
> > myself, using your text where appropriate.  Are you okay with that?
> 
> Yes, I think that would be best.  Please do take account of my points
> about the "The Outside Context", and put the phrase "corrupted point" (or
> something like it) in somewhere.

Now done.

(I didn't use the term "corrupt" because I don't think it belongs in a
manual that describes what a program does.)

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Eli.

On Sun, Nov 06, 2022 at 10:10:31 +0200, Eli Zaretskii wrote:
> > Date: Sat, 5 Nov 2022 16:50:44 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > Instead of continuing this endless argument, I prefer to fix this
> > > myself, using your text where appropriate.  Are you okay with that?

> > Yes, I think that would be best.  Please do take account of my points
> > about the "The Outside Context", and put the phrase "corrupted point" (or
> > something like it) in somewhere.

> Now done.

> (I didn't use the term "corrupt" because I don't think it belongs in a
> manual that describes what a program does.)

I've had a look at the patch.  It seems OK.

Thanks!

I'll close the bug, if you haven't already done so.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Stefan Monnier]

Eli Zaretskii [2022-11-02 16:00:52] wrote:

>> Date: Wed, 2 Nov 2022 11:34:37 +0000
>> Cc: emacs-devel@gnu.org
>> From: Alan Mackenzie <acm@muc.de>
>> 
>> (i) Emacs -Q.
>> (ii) On a single frame, arrange buffers *scratch*, test-edebug.el, and
>>   some other substantial buffer, that I call emacs.README.
>> (iii) Put point in emacs.README somewhere other than point-max.
>> (iv) Instrument test-edebug for edebug with C-u C-M-x.
>> (iv)a Put point into window *scratch*.
>> (v) M-: (test-edebug).
>> (vi) Step through test-edebug using the space key.
>> (vii) Note that the second text insertion happens where point was in the
>>   window, not at point-max.  This is the bug.
>
> Yes, I see the problem, but setting edebug-save-windows to nil
> eliminates it.  So I think we already have a solution for the rare
> situations where this is an issue.

I wish Someone™ could dig into the problem further and find the source
of the problem and an actual fix, but indeed, this seems like a fair
workaround in the mean time.

Maybe a good short term "fix/workaround" is to change the implementation
of `edebug-save-windows` so that in addition to the windows's info it
also saves&restores the buffer-point of those buffers displayed in the
saved windows.


        Stefan

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Alan Mackenzie <acm@muc.de>,  emacs-devel@gnu.org
> Date: Thu, 03 Nov 2022 15:29:38 -0400
> 
> I wish Someone™ could dig into the problem further and find the source
> of the problem and an actual fix, but indeed, this seems like a fair
> workaround in the mean time.

I think before digging into the reasons, we should decide what kind of
behavior we would consider "correct" and/or "useful" in the relevant
use cases with Edebug.  The answer is not easy, because AFAIU Edebug
cannot easily know which window(s) and which buffer(s) are affected by
the program being debugged.

Re: Edebug corrupting point in buffers.

[Stefan Monnier]

> I think before digging into the reasons, we should decide what kind of
> behavior we would consider "correct" and/or "useful" in the relevant
> use cases with Edebug.  The answer is not easy, because AFAIU Edebug
> cannot easily know which window(s) and which buffer(s) are affected by
> the program being debugged.

If we follow the model or "traditional debuggers" which run in
a separate process, then it would make a lot of sense for Edebug to
save&restore points in all the buffers that it (Edebug) touches, so as
to better preserve the behavior we get when Edebug is not invoked.

For that Edebug doesn't need to know which buffers are affected by the
program being debugged, it just needs to know the buffers that it
(itself) affects, which doesn't sound impractically difficult.


        Stefan

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: acm@muc.de,  emacs-devel@gnu.org
> Date: Thu, 03 Nov 2022 16:39:23 -0400
> 
> > I think before digging into the reasons, we should decide what kind of
> > behavior we would consider "correct" and/or "useful" in the relevant
> > use cases with Edebug.  The answer is not easy, because AFAIU Edebug
> > cannot easily know which window(s) and which buffer(s) are affected by
> > the program being debugged.
> 
> If we follow the model or "traditional debuggers" which run in
> a separate process, then it would make a lot of sense for Edebug to
> save&restore points in all the buffers that it (Edebug) touches, so as
> to better preserve the behavior we get when Edebug is not invoked.
> 
> For that Edebug doesn't need to know which buffers are affected by the
> program being debugged, it just needs to know the buffers that it
> (itself) affects, which doesn't sound impractically difficult.

What do you mean by "the buffers that Edebug touches", exactly?
"Touches" or "affects" in what sense?

The next question is "how would Edebug know which buffers it touches?"

And the next one after that would be "what about buffers Edebug
touches that are displayed in more than one window?"

Re: Edebug corrupting point in buffers.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: acm@muc.de,  emacs-devel@gnu.org
> Date: Thu, 03 Nov 2022 16:39:23 -0400
> 
> If we follow the model or "traditional debuggers" which run in
> a separate process

Btw, the crucial difference between Edebug and the "traditional
debuggers" is that with Edebug the user can switch to any buffer and
move point there, as well as move the window-point of any window.  A
Lisp program that depends on the position of point or window-point
(and most of them do) could be fatally derailed if, when the user then
resumes the debugged program, point would not have been restored, or
even if a window that didn't appear on display before now does apepar,
or vice versa.

That is why I said that defining what is the behavior we want is not
easy.

Re: Edebug corrupting point in buffers.

[Alan Mackenzie]

Hello, Stefan.

On Thu, Nov 03, 2022 at 15:29:38 -0400, Stefan Monnier wrote:
> Eli Zaretskii [2022-11-02 16:00:52] wrote:

> >> Date: Wed, 2 Nov 2022 11:34:37 +0000
> >> Cc: emacs-devel@gnu.org
> >> From: Alan Mackenzie <acm@muc.de>

> >> (i) Emacs -Q.
> >> (ii) On a single frame, arrange buffers *scratch*, test-edebug.el, and
> >>   some other substantial buffer, that I call emacs.README.
> >> (iii) Put point in emacs.README somewhere other than point-max.
> >> (iv) Instrument test-edebug for edebug with C-u C-M-x.
> >> (iv)a Put point into window *scratch*.
> >> (v) M-: (test-edebug).
> >> (vi) Step through test-edebug using the space key.
> >> (vii) Note that the second text insertion happens where point was in the
> >>   window, not at point-max.  This is the bug.

> > Yes, I see the problem, but setting edebug-save-windows to nil
> > eliminates it.  So I think we already have a solution for the rare
> > situations where this is an issue.

> I wish Someone™ could dig into the problem further and find the source
> of the problem and an actual fix, but indeed, this seems like a fair
> workaround in the mean time.

I think I said something about this earlier on in the thread.  The cause
seems to be that random bits of software wrongly consider it their
business to overwrite the buffer point with a window point.

Amongst these bits of software are select-window,
set-window-configuration, and our very own edebug-set-buffer-points
(triggered by the option edebug-save-displayed-buffer-points).  I think
there are many more.

The solution, were it practicable, would be to have only the command
loop or maybe redisplay copying WP to BP, and only just before the
window becomes the current place where the user might edit.

> Maybe a good short term "fix/workaround" is to change the implementation
> of `edebug-save-windows` so that in addition to the windows's info it
> also saves&restores the buffer-point of those buffers displayed in the
> saved windows.

My first patch did something like this.  It saved the buffer points of
all buffers, then restored it in buffers selected by the user.  This
patch was rejected (and I think rightly so) for being too clumsy to use.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Edebug corrupting point in buffers.

[Stefan Monnier]

>> I wish Someone™ could dig into the problem further and find the source
>> of the problem and an actual fix, but indeed, this seems like a fair
>> workaround in the mean time.
>
> I think I said something about this earlier on in the thread.  The cause
> seems to be that random bits of software wrongly consider it their
> business to overwrite the buffer point with a window point.
>
> Amongst these bits of software are select-window,
> set-window-configuration, and our very own edebug-set-buffer-points
> (triggered by the option edebug-save-displayed-buffer-points).  I think
> there are many more.

This characterization is a bit too vague and open-ended to be
actionable, sadly.  I was thinking of "the source of the problem" for
the specific case you described earlier in the bug-report (i.e. one we
can conveniently reproduce).  Admittedly, fixing this one case may
not fix all cases, but will at least get us closer.

>> Maybe a good short term "fix/workaround" is to change the implementation
>> of `edebug-save-windows` so that in addition to the windows's info it
>> also saves&restores the buffer-point of those buffers displayed in the
>> saved windows.
> My first patch did something like this.  It saved the buffer points of
> all buffers, then restored it in buffers selected by the user.  This
> patch was rejected (and I think rightly so) for being too clumsy to use.

I was thinking that by limiting the saved&restored buffer-points to
those displayed in the saved windows, we'd make it more "automatic" than
your patch.


        Stefan