bug#731: 23.0.60; Varying point position after undo

bug#731: 23.0.60; Varying point position after undo

[cyd]

> When you put the following two forms in undo1.el:
>
>   (progn
>     (goto-char (point-max))
>     (insert "test line\n"))
>
>   (defun mytest ()
>     (message "hi"))
>
> then do "$ emacs -Q undo1.el", place point after the first form, press
> C-x C-e then C-_, point is placed at the original end of buffer. In
> contrast, every further time you do the same, point is placed at the
> end of the second form after the insertion is undone.

I can't reproduce this with latest CVS.  Do you still see this problem?

bug#731: 23.0.60; Varying point position after undo

[Markus Triska]

cyd@MIT.EDU writes:

> Do you still see this problem?

Yes; it also works if I place point anywhere within the first top-level
form and then press C-M-x, followed by C-_. The first time I do that,
point is moved to the original end of buffer, i.e., after the second
form. Every further time, point isn't moved, until I visit a different
buffer. I can reproduce this with today's Emacs 23 CVS on OSX 10.4 and
Ubuntu Intrepid. Emacs 22.2 never moves point on both systems.

undo1.el is also available from: http://www.logic.at/prolog/undo1.el

As mentioned, the file must be specified on the command line - opening
it with C-x C-f and then doing the above does not move point. However,
you can open undo1.el with C-x C-f, then do C-x b, switch to *scratch*,
then return to undo1.el. If you then do C-M-x C-_, point should move.

bug#731: 23.0.60; Varying point position after undo

[martin rudalics]

 > As mentioned, the file must be specified on the command line - opening
 > it with C-x C-f and then doing the above does not move point. However,
 > you can open undo1.el with C-x C-f, then do C-x b, switch to *scratch*,
 > then return to undo1.el. If you then do C-M-x C-_, point should move.

So the initial values of `buffer-undo-list' differ wrt when you open the
file via the command line or `find-file'.  Can you tell us how?

martin

bug#731: 23.0.60; Varying point position after undo

[Markus Triska]

martin rudalics <rudalics@gmx.at> writes:

> So the initial values of `buffer-undo-list' differ wrt when you open the
> file via the command line or `find-file'.  Can you tell us how?

buffer-undo-list is initially nil regardless of how I open the file.
When I open the file from the command line and then press C-M-x on the
first form, `buffer-undo-list' is with CVS trunk:

   (nil
    (96 . 106)
    (t 18643 . 10108))

whereas with Emacs 22.2, it is:

   (nil
    (96 . 106)
    1
    (t 18643 . 10108))

Starting anew with CVS trunk, after I do C-M-x C-_ M-< C-M-x, it is:

   (nil
    (96 . 106)
    1
    (t 18643 . 10108)
    nil
    (#("test line\n" 0 10
       (fontified t))
     . 96)
    nil
    (96 . 106)
    (t 18643 . 10108))

When I do the same with Emacs 22.2, it is:

   (nil
    (96 . 106)
    1
    (t 18643 . 10108)
    nil
    (#("test line\n" 0 10
       (fontified t))
     . 96)
    nil
    (96 . 106)
    1
    (t 18643 . 10108))

With CVS trunk, when I open the file with C-x C-f and do C-M-x, it is:

   (nil
    (96 . 106)
    1
    (t 18643 . 10108))

bug#731: 23.0.60; Varying point position after undo

[martin rudalics]

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

 > buffer-undo-list is initially nil regardless of how I open the file.
 > When I open the file from the command line and then press C-M-x on the
 > first form, `buffer-undo-list' is with CVS trunk:
 >
 >    (nil
 >     (96 . 106)
 >     (t 18643 . 10108))
 >
 > whereas with Emacs 22.2, it is:
 >
 >    (nil
 >     (96 . 106)
 >     1
 >     (t 18643 . 10108))

Hmmm...  Can you try the attached patch?

martin

[-- Attachment #2: 731.diff --]
[-- Type: text/plain, Size: 659 bytes --]

*** undo.c.~1.85.~	2008-05-14 09:49:54.000000000 +0200
--- undo.c	2008-09-21 13:44:44.468750000 +0200
***************
*** 79,85 ****
    if (NILP (pending_boundary))
      pending_boundary = Fcons (Qnil, Qnil);
  
!   if (current_buffer != last_undo_buffer)
      Fundo_boundary ();
    last_undo_buffer = current_buffer;
  
--- 79,88 ----
    if (NILP (pending_boundary))
      pending_boundary = Fcons (Qnil, Qnil);
  
!   if ((current_buffer != last_undo_buffer)
!       /* Don't make an undo boundary when record_first_change will do it
! 	 anyway.  */
!       && (MODIFF > SAVE_MODIFF))
      Fundo_boundary ();
    last_undo_buffer = current_buffer;
  

bug#731: 23.0.60; Varying point position after undo

[Markus Triska]

martin rudalics <rudalics@gmx.at> writes:

> Hmmm...  Can you try the attached patch?

It fixes this problem for me, thank you!

bug#731: 23.0.60; Varying point position after undo

[Stefan Monnier]

> !   if ((current_buffer != last_undo_buffer)
> !       /* Don't make an undo boundary when record_first_change will do it
> ! 	 anyway.  */
> !       && (MODIFF > SAVE_MODIFF))

Hmm.. the comment seems to say that calling it is just redundant,
whereas this thread shows that it can actually be harmful.
Could you improve the comment so as to explain why it's harmful?


        Stefan

bug#731: 23.0.60; Varying point position after undo

[martin rudalics]

 > Hmm.. the comment seems to say that calling it is just redundant,
 > whereas this thread shows that it can actually be harmful.
 > Could you improve the comment so as to explain why it's harmful?

I checked it in with a slightly more accurate comment.  IMHO, however,
last_boundary_buffer and last_boundary_position should be documented
better.  In particular, I don't understand the comment near the end of
record_point saying

   /* If we are just after an undo boundary, and
      point wasn't at start of deleted range, record where it was.  */

BTW, why isn't there a separate last_boundary_position for each buffer?

martin

bug#731: 23.0.60; Varying point position after undo

[Stefan Monnier]

> I checked it in with a slightly more accurate comment.  IMHO, however,

Thank you.

> last_boundary_buffer and last_boundary_position should be documented
> better.  In particular, I don't understand the comment near the end of
> record_point saying

>   /* If we are just after an undo boundary, and
>      point wasn't at start of deleted range, record where it was.  */

> BTW, why isn't there a separate last_boundary_position for each buffer?

Maybe Richard knows a bit more, but I suspect, nobody knows.


        Stefan

bug#731: 23.0.60; Varying point position after undo

[Markus Triska]

When you put the following two forms in undo1.el:

   (progn
     (goto-char (point-max))
     (insert "test line\n"))

   (defun mytest ()
     (message "hi"))

then do "$ emacs -Q undo1.el", place point after the first form, press
C-x C-e then C-_, point is placed at the original end of buffer. In
contrast, every further time you do the same, point is placed at the
end of the second form after the insertion is undone. When you then
switch to the *scratch* buffer and then switch back and do the same,
point is again placed at the original end of the buffer exactly the
first time you do it, and placed at the other spot every further time.

I find it desirable that point position after undo become more
predictable in this case. I have also seen cases where point after
undo is completely misplaced (i.e., neither at the place where point
originally was, nor where the inserted text started), though I cannot
yet reproduce it reliably.

In GNU Emacs 23.0.60.1 (i386-apple-darwin8.11.1, GTK+ Version 2.12.9)
 of 2008-08-15 on mt-computer.local
Windowing system distributor `The XFree86 Project, Inc', version 11.0.40400000
Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: nil
  default-enable-multibyte-characters: t