rgheck wrote:
Abdelrazak Younes wrote:
rgheck wrote:
Abdelrazak Younes wrote:
rgheck wrote:
Abdelrazak Younes wrote:
rgheck wrote:
Andre Poenitz wrote:
But if the idea is to store the params _because they change_,
that should be a copy, i.e.
otherstack.top().bparams = new BufferParams(buffer_.params())
with a corresponing delete somewhere, shouldn't it?
Well, here's a patch I suggested previously. JMarc thought it
might be better to use a shared_ptr here. I have no strong
views. You?
Isn't the attached patch simpler?
This doesn't make a copy of the BufferParams but just stores a
pointer to the old one. It has to make a copy.
Isn't that what I do here?
+ otherstack.top().bparams = new
BufferParams(buffer_.params());
It has to make a copy in the constructor, whereas:
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), bparams(&bp), isFullBuffer(ifb)
{}
/// Which kind of operation are we recording for?
UndoKind kind;
@@ -90,7 +90,7 @@
/// the contents of the saved MathData (for mathed)
MathData * array;
/// Only used in case of full backups
- BufferParams bparams;
+ BufferParams const * bparams;
/// Only used in case of full backups
bool isFullBuffer;
you only store a pointer to the original BufferParams.
I still don't understand (or you don't ;-)). The purpose of my patch
was to avoid making a copy of the BufferParams in the case of a
non-full undo or redo. In that case storing a pointer to the original
BufferParams is fine. In the cas of a full buffer undo/redo operation
on the contrary we want to make a full copy. So making a copy in the
constuctor is exactly what I don't want to do because this will then
no different than storing a BufferParams instance as we do now.
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.
And while we're on micro-optimization ;~/, how about the attached?
Saves a copy.
If it is safe, and it seems to be, all optimisations are welcome. This
one is not micro if you think about it, an undo operation can contains a
lot of material! And I often seen delay with undo/redo when typing
ctrl-z or ctrl-y rapidly.
Abdel.
