bug#16377: Undo Tree regression: (error "Unrecognized entry in undo list undo-tree-canary")

[Keith David Bershatsky]

Thank you for the helpful critique to substitute `window-buffer` for 
`get-window-buffer`.

I like to keep `debug-on-error` always set to `t`, and known situations I 
generally limit to showing me just a message in the echo area with the name of 
the function that generated it.  That's why I've been using the `quit` signal 
instead of popping open a `*Backrace*` buffer for familiar situations.

The code that the Emacs team implemented in response to bug#25599 is probably 
fine.  I just hadn't seen that before today, and my code predates its 
implementation.

Substituting the `warn` pop-up buffer for a plain old `message` was just a 
matter of personal preference.  Nothing substantive there was intended.

The existence and discussion surrounding `undo-tree.el` predates my usage of 
that library, but from what I can tell, at some point in the past 
`primitive-undo' was "improved" to start checking for errors and this resulted 
in bug #16377.  From an untrained layman's way of thinking, I am "guessing" 
that if undo-tree worked fine before `primitive-undo` was modified to throw an 
error, then perhaps it is not "a problem" with undo-tree except to the extent 
that undo-tree may need to "evolve" to play nice with the current version of 
`primitive-undo`.  Based on that "optional evolution theory", I created the 
workaround to just throw an informative message instead of an error.  However, 
I do not know if that approach could lead to problems.

I've spent quite a bit of time studying certain sections of the undo-tree.el 
library, but there are sections of the code that are still "Greek to me".  My 
understanding of the `undo-tree-canary` symbol inside the `buffer-undo-list` is 
that it is a way for undo-tree to check if it has interacted with the 
`buffer-undo-list`.  If the canary is not there, then handle the situation 
differently.  If the canary is there, then process the `buffer-undo-list` until 
reaching the canary.  It could be any arbitrary symbol so long as it remains at 
the tail end of the `buffer-undo-list` with a `nil` before it while undo-tree 
is being used in the buffer.  There is only one situation I am aware of where 
the `undo-tree-canary` disappears, and it happens sometimes with garbage 
collection (bug #27214).  Whatever symbol is used, it needs to remain in the 
`buffer-undo-list` until `undo-tree-mode` is deactivated.  I suppose the design 
could have been different, but Dr. Cubitt probably had several additiona
 l reasons for using a constant symbol such as the `undo-tree-canary`.

Bug #16377 might very well be resolvable by tweaking/fixing undo-tree.el; 
however, the undo/redo in region code is still a few light years beyond my 
present abilities.

Keith

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> > +          (_
> > +            (if (eq next 'undo-tree-canary)
> > +              (message "undo-tree--primitive-undo:  catch-all found `%s'." 
> > next)
> > +              (error "Unrecognized entry in undo list %S" next)))))
> 
> This might make sense to work around the problem, but is clearly not an
> actual fix.  IIUC Tony said it looked like a bug in undo-tree.
> Has there been any progress on finding/fixing the bug there?
> What is this "canary" meant to do?  If it shouldn't signal an error
> here, maybe rather than the constant `undo-tree-canary`, undo-tree
> should use another constant value, i.e. one that is a valid (and
> harmless) undo entry.
> 
> 
>         Stefan