Le 03/02/2016 21:41, Uwe Stöhr a écrit :
Am 03.02.2016 um 10:20 schrieb Jean-Marc Lasgouttes:
The problem is that every time I look
at this box code, my eyes bleed:
* type as a string instead of enum
I have not invented this. Why is important how a parameter is stored? If
it is important to use an enum, why was/is this then not used when the
box inset was created?
With strings, if one types "Ovval" instead of "Oval" in a test, the bug
may stay unnoticed for a long time. With an enum, there is a compile
time check and it is possible to use a switch, which means that the
compile tells you when you forgot to test for a case.
* special length as string instead of properly extending the Length class
* articificial use_parbox, use_makebox, inner_box parameters that have
to be set in a very precise manner in fragile code that I am not able to
follow.
It is not artificial. What do you find artificial? LaTeX is very
complicated here (some box types allow widths, some not, some boxes
allow to change more parameters than others...)
I can add comments to make it more clear why we do what we do.
Simple question: can you set these 3 booleans independently and be sure
that the result makes sense? I would say no.
Moreover, I would say that this inner/outer box thing should not be
exposed to the user the real questions are
* do you want a decoration around the box?
* do you want to limit the width of the box?
- if so, do you want to allow for linebreaks (mbox/makebox vs.
minipage/parbox)
- then there is the issue of minipage vs. parbox, which is difficult to
describe in layman terms. So probably in this particular case it make
sense to use LaTeXese in the interface
I would think that this presentation makes more sense than inner/outer
box. Only the LaTeX output routine has to care about these trivialities.
Could you be more precise. What do you mean? To understand the logic of
the code one must know how LaTeX works - why \fbox\parbox is a different
to \parbox\fbox. Or how do you like to name a framed minipage with a
surrounding shadow box?
Are we supposed to be able to do \parbox{\fbox{...}} with the Box inset??
Can you point me to the relevant code in GuiBox.cpp?
It is
void GuiBox::setInnerType(bool frameless
Thanks. The code is fine and it matches GuiBox.
Additional question: what is this
else // if (for_box)
line about?
I also don't understand this comment too. In my opinion it can be
removed because there are only 2 cases:
Yes, you can remove the comment. In the case !for_box && !changetype, I
do not understand though why one sets cur.undispatched() instead of call
InsetCollapsable::dispatch(). We shall not presume what our parents will
do unless we have a good reason for it.
Apart from this last remark, you have my +1 for the patch.
JMarc
