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

[Keith David Bershatsky <esq@lawlist.com>]

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).  Whate
 ver 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