Improve timestamp handling

[jdtsmith]

undo creates "unknown" timestamps like (t . 0) when it doesn't know the buffer/file modification time. This ignores those, since they can be a nuisance, e.g. in vundo-stress-test. In addition, this enables logging when trimming the "unnecessary" tail of buffer-undo-list removes timestamp data (see #66).

[jdtsmith]

This is not ready to merge, but a WIP on improving and fixing some corner cases in vundo's handling of the timestamp records encoded on the undo list. This will improve undo's ability to notice a given buffer state is unmodified, and to mark it as such, and improve the reliability of saved-state marking (green circles).

Goals here:

• [x] Ensure all TS's in buffer-undo-list are actually displayed (by checking equivalent nodes)
• [x] Improve debug reporting of lost unique timestamps (pretty heavy, likely disable after testing is complete)
• [ ] Solve the "save for the first time and then navigate away" unique timestamp lossage (see #66)

[casouri]

Maybe use a weak hash table to record whether a state has been saved. Basically something like undo-equiv-table.

[jdtsmith]

You mean modify/extend the undo code itself to do this? I think since we're already "trimming" the undo list a little, bringing the about-to-be-lost TS down to its closest-to-root equivalent node might be preferable.

[casouri]

My thought process is this: if trimming the undo list causes us to lose information on the "saved" property of a mod/node, we can just store the "save" property at somewhere else, like a hash table. When we scan the undo list and find that mod/node is "saved", we save that to a hash table, now it's never lost.

[jdtsmith]

Good thoughts. I had assumed since vundo can sometimes fully rebuild the mod-list, any such info would be lost. But if it's just an "on the side" mapping, unrelated to mod-list, of buffer-undo-list -> TS, this could I guess survive full rebuild. Is that the idea?

Then the question is when to do that. Maybe just before trimming the undo-list, and also when checking a state for savedness of the initial draw (i.e. first consult a vundo--saved-mods hash, then look at next index of all equivalent nodes and set the hash if found). I suppose when undo-list gets trimmed by hitting a memory boundary, there will be stale data in the hash, but not too much, and not so important.

Another important question is whether to store TS info on the node which was saved, or the node which modified it (remembering there can be more than one state that modifies the same node)? The undo system itself discards "stale" timestamps that no longer reflect the modified time of the associated file. So we'd probably be justified saving a mapping of buffer-undo-list at the saved node -> latest applicable TS.

[casouri]

Yes, mods can be rebuilt, but the undo-list they reference (the undo-list field) stays the same across rebuilds. We should do it when we scan the undo list. When you scan the undo list and found a timestamp, instead of setting the timestamp field of the mod, map the undo list of the original undo to t in the hash table.

The undo system itself discards "stale" timestamps that no longer reflect the modified time of the associated file.

What do you mean by "stale" timestamps? You mean if I save a buffer, make some edit, go back to the previous state and save again, the old timestamp is removed?

[jdtsmith]

When you scan the undo list and found a timestamp, instead of setting the timestamp field of the mod, map the undo list of the original undo to t in the hash table.

Why not map it to the timestamp itself? By scanning I assume you mean vundo--mod-list-from? I think we will also have to scan the tail being trimmed, since navigating back to some state, saving it, and navigating away usually loses that timestamp entirely, and this can happen at any time, in between scans.

BTW, one issue with not fixing this loss via the buffer-undo-list proper is it only fixes half the problem: vundo can now keep track of saved states, but undo itself cannot. So the buffer-modification flag will not always be correctly cleared when you navigate to saved states. I suppose we could do that ourselves.

What do you mean by "stale" timestamps? You mean if I save a buffer, make some edit, go back to the previous state and save again, the old timestamp is removed?

Or ignored, rather, for purposes of setting the buffer to unmodified:

           ;; If this records an obsolete save
           ;; (not matching the actual disk file)
           ;; then don't mark unmodified.
           (when (or (equal time (visited-file-modtime))
                     (and (consp time)
                          (equal (list (car time) (cdr time))
                                 (visited-file-modtime))))
             (unlock-buffer)
             (set-buffer-modified-p nil)))

[casouri]

Why not map it to the timestamp itself?

That's fine too.

By scanning I assume you mean vundo--mod-list-from? I think we will also have to scan the tail being trimmed, since navigating back to some state, saving it, and navigating away usually loses that timestamp entirely, and this can happen at any time, in between scans.

We rescan before and after every vundo command, by vundo--refresh-buffer. So vundo--mod-list-from should be enough, unless I'm missing something.

BTW, one issue with not fixing this loss via the buffer-undo-list proper is it only fixes half the problem: vundo can now keep track of saved states, but undo itself cannot. So the buffer-modification flag will not always be correctly cleared when you navigate to saved states. I suppose we could do that ourselves.

The second half isn't too big a problem and I'm definitely not inserting stuff into buffer-undo-list to fix it. That has potential of creating unexpected side effects and conflicts with other random stuff accessing buffer-undo-list.

[jdtsmith]

We rescan before and after every vundo command, by vundo--refresh-buffer. So vundo--mod-list-from should be enough, unless I'm missing something.

It's probably my misunderstanding. But in the cycle of:

1. `vundo--refresh-buffer': `buffer-undo-list' -> mod-list
2. `vundo--move-to-node': read mod-list, modify `buffer-undo-list'
3. `vundo--trim-undo-list': trim `buffer-undo-list'
1. `vundo--refresh-buffer': `buffer-undo-list' -> mod-list

isn't there a potential for timestamp loss between 2 & 3, i.e. before refresh-buffer will have a chance to notice and store the newly created (by move-to-node) timestamp? In fact in my experience, those (generated by undos = "move") timestamps are the only ones which get lost due to trimming. Any created by "real" non-undo edits persist (at the level of equivalent nodes).

I'm definitely not inserting stuff into buffer-undo-list to fix it

I agree that's a bit dicier. How do you feel about (set-buffer-modified-p nil) when we know that it's a saved state corresponding to (visited-file-modetime), but undo itself does not, because we trimmed that info away?

[casouri]

isn't there a potential for timestamp loss between 2 & 3, i.e. before refresh-buffer will have a chance to notice and store the newly created (by move-to-node) timestamp? In fact in my experience, those (generated by undos = "move") timestamps are the only ones which get lost due to trimming. Any created by "real" non-undo edits persist (at the level of equivalent nodes).

Thanks, I think you're right. Then we need to scan for timestamps before trimming, it seems.

I agree that's a bit dicier. How do you feel about (set-buffer-modified-p nil) when we know that it's a saved state corresponding to (visited-file-modetime), but undo itself does not, because we trimmed that info away?

I think that's completely fine.

[jdtsmith]

Closed in favor of #81.