Le 17/10/2015 12:00, Georg Baum a écrit :
Sorry, I don't have the time right now to really measure, but I think it is
not needed, since the numbers you got are OK IMHO.
… As I see it, we do now have
a trade off between easier debugging (ids), and less memory consumption
(pointers). Apart from that, both solutions would work. Since you are the
one who is probably doing most of the debugging work please decide, I am now
fine with either solution.
I'll give pointers a try.
… Having no temp InsetMathString is a key
point for understanding the code IMHO. If you have no temp inset, then
there is suddenly no need to circumvent anything imposed by the MathAtom and
MathData design anymore. If you have a temp inset, it is not obvious that
the only reason for having it is to use the write() method.
Since I believe that we somehow misunderstood each other I attached a sketch
of what I meant. In order to keep the patch small and to show the main
intent, I did keep writeString() in InsetMathString.cpp. If this gets
applied then it should of course move to MathExtern.cpp.
The reason why I think that this is more readable is that there is no temp
inset involved, and there is only one simple loop instead of two nested
ones.
Indeed, I misunderstood what you meant. Thanks for the patch, I will use
it. But the point to me seems more to do a single loop than removing the
InsetMathString, which may be why I misunderstood.
Regarding the other parts of the patches I have some nitpicks: Please no
semicolons which are not needed, so
uid_type id() const { return uid_.id(); }
instead of
uid_type id() const { return uid_.id(); };
Ok.
Also, an operator==() which does not compare anything is rather dangerous
IMHO, I think it is safer to simply exclude the id in the comparison
operator of the owner.
Yes: at the time I believed that C++ provided default comparison
operators based on the member's, now I see that this is useless.
Then, is id() needed to be virtual in InsetMath? When using it you need to
distinguish anyway between a valid and an invalid one, so can this maybe
simply be kept nonvirtual in InsetMathNest?
Yes.
I do not know the TeXRow stuff too well, so I won't comment on that, but I
believe you should apply 0003-Splitting-otexstream-into-otexrowstream-and-
otexstre.patch immediately.
Ok.
Guillaume
