Hi !
I'm doing a undo/redo-system for form designer (
bugs.freepascal.org/view.php?id=22584). This is not the final version now. I post the patches, and each next patch contains more features than the previous one. My code is not perfect, but I'm working on it :) Can any of responsible developers to write a feedback about my work (then I will know that I am doing the right thing)? Thanks. -------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20121216/fbaa6c4c/attachment.html>

I am still thinking of the most important case: undo for delete operation. It is also the most difficult one. Anchors and alignment depend on the order of creation. It is not enough to just insert a deleted control back. Maybe the whole form must be recreated. Maybe there must be a snapshot history of the form.

I guess Mattias must comment on your code at some point. I cannot say whether it fits the big picture when undo for delete is implemented.

Juha

Yes, you need snapshots.

Mattias

I'd like to note that Alexander Strokach is my student, and I have given him this
task as a course work assignment (which is a kind of lighter version of bachelor's thesis).

We had quite a lot of discussion with him about the implementation method. I favored storing a series of snapshots too, while he preferred a "list of reversible actions" design.

Main arguments were: storing snapshots is much simpler to implement and more reliable,
while using a list of actions requires less memory and is similar to what SynEdit does, so should
integrate better.
(I am of the opinion that the IDE should have a common undo history for form designer and code editor).

He went on and implemented his approach as a proof of concept, and after several iterations
the code is working for some cases, but the complexity is indeed rather high. We have now reached a point where expert opinion is needed: is Alexander's approach viable,
or should he abandon it and start from scratch?

Also, please give the opinion on the code quality and architecture of his patch --
he is nearing mid-term evaluation :)

Yes, you need snapshots.

Manager of anchors is not supported in the current version. I will decide the issue in the future. Could you describe the current situation? -------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20121216/1ef76442/attachment.html>

Some operations have side effects and there is no simple reverse operation. These are often the ones you want to undo. E.g. deleting a referenced component. Especially if it was a component referenced by other forms.
Combining the undo with the code editor would be nice, but I don't know how to solve it when there are multiple source editors involved.

Well, the requirements of your course and the preferences of some guy of an open source project are two different shoes. That being said, I can name a few things where I see problems:

- public variables should not be named FUndoState. The F is commonly used for private variables.
- Instead of 0,1,2,3 for Left, Top, Width, Height an enum should be defined.
- TUndoList is logically a TUndoListItem.
- FForm.FindComponent does not find components on frames.
- The procedure SetPropVal checks for specific properties (e.g. 'TColor'). It must not.
- FloaToStr/StrToFloat looses precision.
- Randomize should only be called once in the whole IDE.
- Use increasing numbers instead of random numbers.

Mattias

While I have never looked at the designer code, I think this can be broken down into individual parts:

1) Monitor all properties/ property editors. This allows to detect changes to individual properties, made by the user. Such as name height, caption, align, options....

This is not complete. Some properties may have side effects. hanging anchors, can change positions. Setting an option may clear another published property.
Yet for step 1 those can be ignored. This is a huge topic and needs a discussion off its own. Afaik the only way to detect those changes, is to stream the entire form.

2) creation of new controls

3) deletion of controls (bringing it back, even with the wrong order is already a step, but only if the code allows to fix that later)

4) change Baseclass

5) change z-order (afaik also creation order)

6) .... (I am sure there is more)

----
Then it must record, whenever it makes changes to the editor. So they can be undone too.

And eventually, when it makes changes to the editor, it must register there, so the editor can send an event, if that change is undone by the editors undo system (or redone).

You can analyse/diff the snapshons, and store the diff, or even the action only whenever possible

As before, if you set align, it also changes anchors. Changing align back, may *not* restore the anchors.
For properties in the LCL (or even components that come with Lazarus it is possible to maintain a list. Though that adds maintenance, and permanent danger of the list being out-dated.
For 3rd party components no such list can be done. It is simply not known. They can even change the behaviour of some of the LCL properties.

So actions only will never be able to undo everything. Yet capturing them may have some value.
- It can be used as a trigger to take snapshots. Maybe snapshots can be taken on idle. (That however would mean that some steps would only have "action undo". And it needs to enssure, that a snapshot is taken at least every n actions. And IMHO even complex Forms will stream fast enough to be streamed always. Yet on Laptops this may considerable affect battery live. And 3rd party components could have slow code in the streaming.
- It can be used in order to "redo"

Yes.
Note: Even those properties that do not have side effects might have side effects in descendants of the component.

2) creation of new controls

With side effects: change of source, unit section, package list of project.

It can become very complex. Snapshots are slow, but they would solve most of the issues.

---- Then it must record, whenever it makes changes to the editor. So they can be undone too.

Yes. And the changes can spread over multiple syn edits.

And eventually, when it makes changes to the editor, it must register there, so the editor can send an event, if that change is undone by the editors undo system (or redone).

Yes.

Mattias

When multiple files are modified, together or in sequence, then you have decide about a common undo history, or one for every file. When e.g. a PAS and DFM/LFM file have to be changed together, an Undo must revert both at the same time. Delphi has a bulk change option, so that e.g. a file wide search/replace can be undone in one go. Likewise changes to multiple files can be considered as such one big step, consisting of a number of changes. In the best case the user should have an choice to undo all or only selected steps of such an big change. Just in case of an form designer it would be fine to revert changes in the GUI only, while retaining later changes to the related code.

IMO an Undo (history) list is the state of the art. E.g. Version Control Systems are based on differences, but they may add snapshots of the last file state(s) for better performance.

Also consider that the output of "diff" is usable in both directions, to apply or revert the stored changes. Git e.g. offers to commit/revert selected chunks of a file diff, at least at the top level ("amend last commit"), and on selected files. It also allows to "cherry pick" selected steps from a history, possibly of an different branch.

DoDi

[...] - It can be used as a trigger to take snapshots. Maybe snapshots can be taken on idle.

+1

(That however would mean that some steps would only have "action undo". And it needs to enssure, that a snapshot is taken at least every n actions. And IMHO even complex Forms will stream fast enough to be streamed always.

Embedded images makes streaming slow.
Maybe we need an option/button to turn it off.

Yet on Laptops this may considerable affect battery live. And 3rd party components could have slow code in the streaming.

Yes.

- It can be used in order to "redo"

Mattias

As before, if you set align, it also changes anchors. Changing align back, may *not* restore the anchors.

Align is a special property, which ideally should be write-only (a procedure). Every alignment is equivalent to an set of anchors, but there exist patterns (sets of anchors) which do not map into a defined alignment. My designer would ignore Align, and only watch the Anchors.

DoDi

Hi

When I click the "Save all" button, don't Lazarus save only the files that have changed since the last save? In that case, a "snapshot" needs only store the information that will be saved when the "Save all" button is clicked (and this information should be known somewhere in Lazarus). AFAIK that will be diffs of the .pas, .lfm and possibly the .lrs files of the current unit/form (and possibly of the project file and other units if changes were made to more than 1 unit). The point I am trying to make is that partial diffs of only the files that were actually changed (in memory), will work.

How far back do we want undo/redo to go? Do we want to be able to revert to a state from before the last "Save all"? Do we want to revert to changes done before Lazarus was closed the previous time (i.e. should these changes be saved when the project is closed)?

Cheers
Chavoux

You did miss the point: This is an example.

other (3rd party) components can have lots of such properties. And this is an allowed behaviour, so it must be dealt with.

Hans-Peter Diettrich wrote

IMO an Undo (history) list is the state of the art. E.g. Version Control Systems are based on differences, but they may add snapshots of the last file state(s) for better performance.

Actually, it is the other way around. Modern VCS are based on snapshots, the last diff-based system was Subversion, IIRC.

The procedure SetPropVal checks for specific properties (e.g. 'TColor'). It must not.

Why? Storing it as a number does not make sense when there is a ColorToString.

"????????? ?." hat am 17. Dezember 2012 um 12:08
geschrieben:

> The procedure SetPropVal checks for specific properties (e.g. 'TColor'). It > must not. Why? Storing it as a number does not make sense when there is a ColorToString.

There are generic functions for this: FindIntToIdent and FindIdentToInt. See fpc/rtl/objpas/classes/writer.inc for an example how to use it.

Mattias

What about this architecture:
The new system replaces the old (for the code editor) and contains functions for the form designer. Snapshot is composed of code and form, snapshot is retained after each action in the code editor or in the form designer. Snapshots are stored in an array, which will compressed. Undo-action: snapshot is taken from a array and replaces the current form and code.
One undo-list will be for the two editors in the result.

Integrating the editors undo, will be a later step. When it is done, I am pretty sure SynEdis undo system can be extended, to store some extra info, and trigger callbacks.

The editing and the designing were never 100% in Sync. Undo the "Button1: TButton" in the editor is just one way of breaking thigs. The user can edit/remove this code outside undo events. Fully fixing this (if desired at all) is possible, but not part of designer undo project.

For the designer undo, it will only be of interest, to act, if the user undoes such code.
Therefore most other undo events in the editor, are not of interest.

It is enough, if the designer can register a callback for selected undos with the editor.

There are 2 ways an interaction can happen:
** 1) undo in the designer:
Code in the editor is added or removed like on any designer action. Example:
- user creates TButton ( "Button1: TButton" is added to editor) Designer: has a TButton-add-undo
Editor: has an insert-text-undo (with a callback to designer
- user edits other text, more undo in editor
Now the editor can not perform the above undo, unless all the editing is undone as well (undo can currently only be done in exact reverse order)
- user does UNDO in designer
Designer: executes TButton-add-undo / creates a redo action this will remove the declaration from the editor, much the same way like a normal delete component.
Editor: ADD another undo "text-deleted-undo" (with callback to designer) This is another edit action, with a new undo info

So after that, in the editor, you have 2 undos from designer actions. If you undo in the editor, the designer executes the redo, and adds the button again

This may seem slightly illogical. But the alternatives are
- A - One undo list, with strict order for all actions from editor and designer: You can not undo in the designer, until you undid all the text=changes that you did imbetween. That is a huge restriction. AND that has another problem: The designer interacts with several editors: the undo lists of ALL editors would need to be synced. Undo in file Foo would start being dependent on undo in file Bar....

- B - Allow to execute undo in random order. That is allow to undo selective items, from any position of the undo stack, not just top. That is possible of course. But it is a huge task of it's own. If indeed agreed on: It would have to be done before the designer undo. I would not suggest to this as part of the designer undo.

(B) can still be done later, If the callback system is implemented now, then it will be easy to change. B would still use the callbacks. But if the designer does an undo, instead of triggering an edit to the editor (creating a new edit-undo) it would tell the editor, to go and find and remove the selected undo item.
The fact that it can be changed this way later, shows that this is an editor issue, not part of the designer undo.

** 2) undo in the editor:
works as always, but informs the designer on events created by the designer