-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A possible approach to the "lost timestamp" issue #66
Comments
Commenting as a user, although I have some experience with Emacs undo internals. Note that one of the things I really like about If including such a feature adds complexity / performance overhead, then I'd prefer not support the feature at all or, optionally support the feature - so the extra complexity can be avoided if the feature is disabled. |
Valid points; I share that experience with undo-tree, and do love the simplicity and efficiency. There is a config option to disable save marking in #62 if you want to avoid seeing that information for some reason. That said, it should not impact performance at all (and will still be gathered). This particular fix to a deep and subtle bug (timestamp information loss) would need some testing, but I don't expect it would have much performance impact either, since it's a one-time cost not associated with moving around. Note that, even if you do not care to see saved state, this matters for correctly marking buffers unmodified, which currently vundo cannot reliably do, because it trims away timestamps. I'm not sure if you've encountered that, but I definitely have... "didn't I just save that there?". BTW, is there a test framework you or others have used to stress-test the undo system? It would be good to stress/performance test any changes. |
Here is a small stress test I wrote of the undo and vundo system. See #67. |
While I am thinking of it: I want to be clear that I consider the "incorrect buffer unmodified state" (impact 1) to be the far more significant issue than "getting the last-saved highlighted node right at all times" (impact 2). But the latter makes it quite clear that information loss is occurring. Here's a recipe to see this yourself (using #62):
If you had done the precise same set of modifications and undos/redos with the built-in emacs commands, the saved buffer state would have been correctly marked as unmodified on returning to it. This is the current price paid for "trimming the tail". Also note that TS lossage does not always happen, since vundo does save "some" of the undo/redo-only tail of buffer-undo-list, as needed. So sometimes the undo/redo-inserted TS is retained, other times it is lost. This of course makes the situation even more confusing. I often experience a related dissonance using vundo — "Wait, I did save there, didn't I?". |
OK, I did some more exploration of the "timestamp lossage" issue. I've understood a few new things:
More info in #73. |
For me, May I ask if that implies if I increase the |
My basic understanding is that moving a big distance in the undo tree adds significant length to the buffer-undo-list, which may, prior to vundo lopping off the unnecessary "leafy growth" at the tips of the tree, lead to list trimming from near the trunk by emacs. Then some branches of the tree (including the one which you may be on) can be severed and unreachable. You can likely increase the various undo limits to help avoid this. But never totally solve. A smart pruning that respects the tree structure could help, but sounds pretty complicated. Another idea is to break large moves into a series of smaller ones, trimming the shrubbery at each intermediate step. Of course if emacs had used a tree structure (instead of a list that loops back on itself) that would be simpler. |
Yes, increasing |
Current simplified thinking on this issue is in #81. |
This is still a (minor) issue. |
Recap of the issue
A recap from #62: there is one type of information that is lost when vundo trims the "useless" tails of very long undo-buffer lists — timestamps. Emacs inserts timestamps into the modification record when modifying a saved (aka unmodified) buffer. These timestamp (TS) records are simple, and look like:
Importantly, timestamps are inserted whether the modification in question is an "ordinary" modification, or an "undo/redo" modification. But vundo trims the latter to avoid the
buffer-undo-list
growing exponentially (which happens fast thanks to its ability to fly around a tree of undos). In doing this trimming, timestamps associated with undo/redo modifications are lost.Impact
These lost timestamps show up in two ways:
Potential Solution
Here is a possible solution: for any new TS records found past the last ordinary modification, "teleport" them back to the equivalent node closest to the root with the same directionality. Said more precisely: for
t
representing the newly discovered node with the timestamp, pair (t-1
,t
) is mapped to equivalent pair (p-1
,p
), for the minimalp
. Then we teleport the TS fromt
→p
, overwriting any found there.Diagram
In picture form (borrowing from @casouri's excellent blog post):
Outcome
To evaluate if a given node is saved, we check if the next state of that node or any equivalent node has a TS record. For example, in the diagram, node 1 is known to be saved, because its equivalent node 5 sees a TS in the next state up (node 6). This will fix problem 2 above.
Problem 1 is also fixed, as long as vundo selects a path that arrives via the adjacent TS node. So for example, in the above right-hand example, going from 0→1 is via 6→5, which works correctly. No path from 2→1 will set the buffer to unmodified, but the same would be true for regular undo/redo as well. Also note that the teleportation process, even if it overwrites an older TS, does not cause problems; undo already ignores older timestamps that do not match the file's modification time.
Cost
The cost is (sometimes) maintaining a bit more tail to track save-then-undo's. In the above example (pink node is current), normally you'd trim back to node 5. For the left scenario, you do that. For the right scenario, you trim only back to node 6.
The text was updated successfully, but these errors were encountered: