unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Misery with undo.  Help, please!
@ 2008-10-20 19:06 Alan Mackenzie
  2008-10-20 21:49 ` Andreas Schwab
  0 siblings, 1 reply; 4+ messages in thread
From: Alan Mackenzie @ 2008-10-20 19:06 UTC (permalink / raw)
  To: Richard Stallman, emacs-devel

Hi, Richard and Emacs!

I'm having a very bad day, and help would be appreciated.  The following
scenario arose in my investigation of bug#1202:

Open a fresh buffer in fundamental mode.  Type in "GNU" (three letters).
Then look at the undo list, M-: buffer-undo-list

    (nil (1 . 4) (t 65535 . 65535))

.  The three elements are:
   nil          ; undo boundary
   (1 . 4)      ; beginning and end of "GNU"
   (t 65535 .65535) ; Timestamp of file (here, there isn't one).

In this state, you can undo with C-_ (or whatever).

Now prefix a POSITION element and another undo boundary with
M-: (push (point) buffer-undo-list
M-: (push nil buffer-undo-list)

buffer-undo-list now looks like

    (nil 4 nil (1 . 4) (t 65535 . 65535))  [*]

, where the extra two elements are:
   nil           ; undo boundary
   4             ; POSITION.

[*] Actually, it inserts an extra, spurious, nil too, or I've badly
misunderstood something.  This seems to be a bug in the macro `push'.  Do
M-: (pop buffer-undo-list) to get rid of this.  I'm having a bad day.
:-(

o-o-O-o-o-O-o-o-O-o-o

If you now attempt an undo, point is indeed moved to bufpos 4, but
buffer-undo-list doesn't lose its first two elements as it should.  THIS
IS A BUG.

o-o-O-o-o-O-o-o-O-o-o

However, if you do M-2 C-_, the (double) undo works, but buffer-undo-list
now looks like:

   (nil
    ("GNU" . 1)
    (#<marker at 1 in GNU> . -3)
    (#<marker at 1 in GNU> . -3)
    (#<marker at 1 in GNU> . -3)
    (#<marker at 1 in GNU> . -3)
    (#<marker at 1 in GNU> . -3)
    (#<marker at 1 in GNU> . -3)
    (#<marker at 1 in GNU> . -3)
    (#<marker at 1 in GNU> . -3)
    (#<marker at 1 in GNU> . -3)
    (#<marker at 1 in GNU> . -3)
    (#<marker at 1 in GNU> . -3)
    (#<marker at 1 in GNU> . -3)
    nil
    4
    nil
    (1 . 4)
    (t 65535 . 65535))

[The #<marker ... elements mean "this marker was shifted this number of
characters forward in the change."]

    (eq (car (nth 2 buffer-undo-list)) (car (nth 3 buffer-undo-list)))
evaluates to nil, suggesting that 12 markers (not merely 12 copies of a
single marker) have been created and inserted into buffer-undo-list.  I
have a suspicion this is a bug.  I can't be sure.  When I attempt C-x
C-x, Emacs says "No mark set in this buffer".

o-o-O-o-o-O-o-o-O-o-o

I've tried to track all this down for several hours, and only got very
confused and miserable.  Help would be appreciated.

When buffer changes are made, Emacs pushes stuff onto buffer-undo-list (a
buffer local global variable) directly, each primitive calling one of the C
functions such as record_insert (in ..../src/undo.c).  This is fine.

When `undo' is invoked, things aren't quite so simple.  buffer-undo-list
is first copied into pending-undo-list, some changes are undone,
(possibly pushing more stuff onto buffer-undo-list), and finally, L1670,
after this comment:

    ;; Don't specify a position in the undo record for the undo command.
    ;; Instead, undoing this should move point to where the change is.

, trims POSITION elements from b-u-l, up to the first undo boundary.

Richard, some while ago, you added in this bit which trims these POSITION
elements.  This was in simple.el V1.121.  Can you remember why you did
this?  It kind of feels to me that there might have been some problem
with POSITIONs in `primitive_undo', and this was a temporary kludge to
get round it.  Can you remember?  It was only 14 years 7 months ago.
(Don't you just love VCSs? :-).

Help, please.

-- 
Alan Mackenzie (Nuremberg, Germany).




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Misery with undo.  Help, please!
  2008-10-20 19:06 Misery with undo. Help, please! Alan Mackenzie
@ 2008-10-20 21:49 ` Andreas Schwab
  2008-10-21 20:34   ` Alan Mackenzie
  0 siblings, 1 reply; 4+ messages in thread
From: Andreas Schwab @ 2008-10-20 21:49 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Richard Stallman, emacs-devel

Alan Mackenzie <acm@muc.de> writes:

> [*] Actually, it inserts an extra, spurious, nil too, or I've badly
> misunderstood something.  This seems to be a bug in the macro `push'.

No, it isn't.  The command loop automatically inserts an undo boundary.
You should use ielm for your experiments.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Misery with undo.  Help, please!
  2008-10-20 21:49 ` Andreas Schwab
@ 2008-10-21 20:34   ` Alan Mackenzie
  2008-10-23 18:33     ` Stefan Monnier
  0 siblings, 1 reply; 4+ messages in thread
From: Alan Mackenzie @ 2008-10-21 20:34 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Richard Stallman, emacs-devel

Hi, Andreas!

On Mon, Oct 20, 2008 at 11:49:23PM +0200, Andreas Schwab wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > [*] Actually, it inserts an extra, spurious, nil too, or I've badly
> > misunderstood something.  This seems to be a bug in the macro `push'.

> No, it isn't.  The command loop automatically inserts an undo boundary.
> You should use ielm for your experiments.

Ah, thanks!  How come I didn't know about ielm?

I understand what's been happening, now.  While primitive-undo is busily
pulling stuff off pending-undo-list, it's piling stuff back onto
buffer-undo-list.  That's not obvious from looking at the source code.

The problem, with a buffer-undo-list of (nil 6 nil ......) is that
primitive-undo piles nothing onto it, merely moving point to 6.
buffer-undo-list thus remains unaltered after C-_.  This is a bug.

There seem two alternative ways to fix this:
(i) In the Elisp manual declare a buffer-undo-list of this form as
  malformed, and not to be passed to `undo'.
(ii) In `undo', check for (not (car buffer-undo-list)), and strip off
  the initial nil from b-u-l.

I think (ii) is better.  What do others think?

> Andreas.

-- 
Alan Mackenzie (Nuremberg, Germany).




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Misery with undo.  Help, please!
  2008-10-21 20:34   ` Alan Mackenzie
@ 2008-10-23 18:33     ` Stefan Monnier
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Monnier @ 2008-10-23 18:33 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Andreas Schwab, Richard Stallman, emacs-devel

>> > [*] Actually, it inserts an extra, spurious, nil too, or I've badly
>> > misunderstood something.  This seems to be a bug in the macro `push'.
>> No, it isn't.  The command loop automatically inserts an undo boundary.
>> You should use ielm for your experiments.
> Ah, thanks!  How come I didn't know about ielm?

Indeed, how come?

> The problem, with a buffer-undo-list of (nil 6 nil ......) is that
> primitive-undo piles nothing onto it, merely moving point to 6.
> buffer-undo-list thus remains unaltered after C-_.  This is a bug.

In itself maybe it's not a bug (tho it should probably record some
other point-motion entry), but it probably introduces problem
w.r.t. undo-equiv-table.  Can someone try and figure out why C-_ doesn't
record a point-motion entry on the buffer-undo-list?


        Stefan




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-10-23 18:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-20 19:06 Misery with undo. Help, please! Alan Mackenzie
2008-10-20 21:49 ` Andreas Schwab
2008-10-21 20:34   ` Alan Mackenzie
2008-10-23 18:33     ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).