On 10/22/2012 11:31 AM, Goubier Thierry wrote:
Le 22/10/2012 11:07, Göran Krampe a écrit :
Hi!
(not up to speed on Pharo "goals" in this area, but...)
On 10/22/2012 09:45 AM, Goubier Thierry wrote:
Hi all,
I'd like to ask if I understood correctly this part of the discussion. With a suitable log/change format as Deltas propose and Igor is thinking of implementing, we could undo any changes done on the image (such as method creation, modification, class add/remove, etc...).
Note - Deltas actually *exist* - it is not just a proposal. They work.
And the answer is "yes", it enables full multilevel undo of *source* changes to the image.
It does this by making sure each "Change": - Has captured enough state to reverse itself in full. - Knows how to create its "anti-Change".
...then the code in Delta>>undo basically ends up like:
changes reversed do: [:each | each asAntiChange apply ]
...well, in fact - it is not written exactly like that - we create a full "anti-Delta" first, and then "apply" it. But anyway, you get my point.
Major changes compared to the total misfit ChangeSet:
- A Delta has *ordered* Changes, not a "Set". So we could call a Delta a "ChangeList" instead. :)
- A Delta and its list of Changes is *completely* standalone from the rest of the image. Thus trivially serialized, thrown away etc.
A ChangeSet is *not at all* standalone, which is the worst aspect of them. You can easily see this yourself by:
- Create a ChangeSet and add a method (it gets recorded) - Create a new ChangeSet and modify same method (it gets recorded in the new ChangeSet) - Take a look in the previous ChangeSet, tada! The method there is now not at all what it was from the start...
...so a ChangeSet actually only "points" at methods, they do NOT contain the actual change!
Now, coming back to Change>>asAntiChange - this is where the tricky part starts...
First each SystemChangeAnnoncement MUST capture all state. For example, earlier a "new class comment" didn't send along the old comment! And it was fired *after* the change was done, so information was lost. Not sure how Pharo does on that exact example today - worth checking for fun! :)
Secondly, the delete/remove Change objects end up to have to capture ALL state that was removed... So a "remove class X"-change must in fact capture enough information to be able to reconstruct it. Thus Deltas have CompositeChange which basically is a full list of smaller changes like create a class X, add n methods to it etc.
Ok, then I could imagine having a system browser which is able to use Deltas to show this ability to list changes and undo them.
Yes, and while Matthew wrote one UI for Deltas (a system browser variant) I was not all too comfortable with that "view". I was more thinking of simply rewire Dual change sorter and friends to use Deltas instead.
Is there any provision to selectively undo changes and try to maintain the system in a consistent state? For example, undo a class rename and you may have to recompile / change methods which refer to that class name to maintain the system in a consistent state.
Well, I started on this idea of "condensing" a Delta which would remove all redundancy (changes shadowing other changes) and also typically "move" renames to the beginning or end (and adapt the other changes to that fact of course). This is a fun challenge - but I didn't finish it due to lack of time. And yes, this "shadowing analysis" would also come into play in your uses case.
How does this relates with the undo/redo facility in RefactoringBrowser?
Dunno.
Well, there are some commands such as RBRenameClassRefactoring, when executed, which get stored in an undo/redo facility (RBRefactoryChangeManager) where you can undo or redo the operation.
So, as you can see, it is already a way to record user-made changes and undo them, probably very cleanly, with a huge hierarchy of available commands and ways to group them.
Yeah, and at the moment I can't really comment on it - but it sure is something we would want to look into. Deltas operate on the lowest level - and was meant to do that by design. RB is on another higher level. regards, Göran