[Lazarus] Undo/Redo in Form Designer

[Mattias Gaertner]

On Sun, 16 Dec 2012 22:21:26 +1100
Alexander Klenin <klenin at gmail.com> wrote:

>[...]
> 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).

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.

 
> 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 :)

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