Closed Bug 508733 Opened 16 years ago Closed 16 years ago

Replace current undo/redo mechanism with Julian's new model-centric approach

Categories

(Skywriter Graveyard :: General, enhancement, P4)

0.4.0
enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ben, Assigned: ben)

References

Details

Attachments

(1 file, 1 obsolete file)

From Julian: I worked over the weekend on getting a first implementation of this done... and it works :) http://bitbucket.org/j4c/bespin/changeset/cd8be0e90b56/ I know this is different to the thought of Alex and I think also to the ones of Kevin in some way. What you get with that patch is a history manager for the editor as we have it now, *BUT* this manager is not based on the editor.actions functions but on the editor.model functions. More then less it recordes the calls to deleteCharacters, insertCharacters, deleteChunk, insertChunk and joinRow within the model (yes, not the action). If you want to undo something it just calles the opposite-function (delete <=> insert). Also you can group up many insertChunk etc. calls within the actions to be one undo/redo stepp. This is great if you think of the actions indent function. This history manager is working well but I left the old history manager in as well for the moment. To perform an undo/redo of the new history manager use ALT+Z, ALT+Y, ALT+SHIFT+Z. You should use only one of the manager if you want to undo/redo something (don't mix up CMD+Z with ALT+Z!). It's a little bit hard to explain for me, how it works internal. As the code isn't that complicated you should be able to have a look at it and understand it (I hope). Otherwise just let me know ;)
Severity: normal → enhancement
Priority: -- → P4
Assignee: nobody → bgalbraith
I've added this and it largely works great! There's an issue with a selection area: - add some text to the top of a document, hit enter - select a region of text lower in the document - hit enter to clear the text - type a few characters - use alt-z to undo these operations. when you start undoing the text added to the top in the first step, the text selection from the second step stays put. this shouldn't be. instead, the text selection should be cleared when the cursor is moved to the top of the document as a result of undoing the operation.
Also, seems that the cursor is sometimes placed *before* the character when undoing when it should be placed *after* the character.
Actually, to comment #1, I think I was confusing alt-z with cmd-z mid-stream. The code looks fine and I can't reproduce it.
Julian, I had to pull this out as presently it doesn't aggregate commands properly. For example, if I select a block of text and paste new text over it, when I undo, it removes the paste text first and then I have to undo again to get the original text back; two undos for one action. I'll add a bug for this, change the target to 0.4.1, and then file a new bug that this bug depends on.
Target Milestone: 0.4.0 → 0.4.1
Depends on: 508878
We should find a conclusion first, which way to implement this we should follow in the future (Alex's way, my way, another way). As long as we haven't done this, this feature should be marked as proposal. We can pull the code to tip but we shouldn't replace the current history code until we have this running all well. See: http://groups.google.com/group/bespin-core/browse_thread/thread/21f897fc419ec596
I sent a few messages to the thread on this. Waiting for an updated patch from Alex that works on the tip
Attached patch WIP Patch (obsolete) — Splinter Review
Attached patch Finished? PatchSplinter Review
Here is a tested version of the patch. Hopefully, it's good.
Attachment #393226 - Attachment is obsolete: true
Applied patch to tip and it looks good after my own minor testing. Switched target to 0.4 to reflect that this bug will make it into 0.4.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: 0.4.1 → 0.4.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: