rgheck wrote:
Abdelrazak Younes wrote:
rgheck wrote:
Abdelrazak Younes wrote:
OK, I see. Well, I think it works that way, but only because
doRecordUndo() only gets called with isFullBuffer being true at
this one point in the code. If it ever got called that way from
elsewhere, you'd have a problem.
I don't see this could happen but we should make sure this doesn't
happen indeed. FWIW, this is the same technique as in mathed.
So it seems to me to be clearer to do this in the constructor.
But then there is no optimisation AFAICS.
Sure there is: You only do the copy if isFullBuffer is true.
OK, but you don't save the BufferParam creation. I've been using LyX
with my patch today without any problem...
You only make the copy if you need to:
Index: Undo.cpp
===================================================================
--- Undo.cpp (revision 25567)
+++ Undo.cpp (working copy)
@@ -73,8 +73,17 @@
MathData * ar, BufferParams const & bp,
bool ifb) :
kind(kin), cursor(cur), cell(cel), from(fro), end(en),
- pars(pl), array(ar), bparams(bp), isFullBuffer(ifb)
- {}
+ pars(pl), array(ar), isFullBuffer(ifb)
+ {
+ if (isFullBuffer)
+ bparams = new BufferParams(bp);
Ah yes, I didn't think of that :-)
+ }
+ ///
+ ~UndoElement()
+ {
+ if (bparams)
+ delete bparams;
Warning, should be :
+ if (isFullBuffer)
+ delete bparams;
+ }
/// Which kind of operation are we recording for?
UndoKind kind;
/// the position of the cursor
Obviously, this isn't a big deal. Yours should work. But this one
seems marginally safer.
Agreed. No more objection :-)
Abdel.
