On Wed, Aug 15, 2007 at 04:05:16PM +0200, Jean-Marc Lasgouttes wrote:
> Martin Vermeer <[EMAIL PROTECTED]> writes:
>
> > Attached is a first attempt to streamline the handling of the
> > various presentation modes of collapsable insets.
>
> I think this is something that is very worth doing and the structure
> of the patch is sound.
Thanks!
Follow-up patch attached.
> Some comments on the code follow.
>
> JMarc
>
>
> > + virtual Decoration decoration() const { return Conglomerate; }
> > +
> > + ///
>
> These things could _maybe_ be moved to insetlayout.
Yes... but I'd rather not do that right now, as it would
touch some files containing another pending patch.
> > + case InlinedD:
> > + return InlinedG;
> > + break;
>
> I do not like these InlinedD/InlinedG. Isn't it possible to use
> Decoration::Inlined and Geometry::Inlined?
I tried something like that; not sure you like the result.
Should I use namespaces instead?
> > + if (decoration() == InlinedD || decoration() == Conglomerate) {
> > InsetText::metrics(mi, dim);
> > } else {
> > dim = dimensionCollapsed();
> > - if (status() == Open) {
> > + if (geometry() == OpenG || geometry() == OpenInlined) {
> > InsetText::metrics(mi, textdim_);
> > // This expression should not contain mi.base.texwidth
> > openinlined_ = !hasFixedWidth()
>
> I think in general it is better to use a switch, so that one sees
> immediately where each case is handled:
>
> switch(decoration()) {
> case Classic:
> ...
> case InlinedD;
> case Conglomerate;
> ...
> }
Done. You're right, it looks clearer.
> It is a good idea to avoid using a default: case for these cases where
> the enum takes a few values. One adavantage is that, if you add
> another value to the enum, the compiler will point to you all the
> places that need to be updated.
+1
- Martin
Index: insets/InsetERT.h
===================================================================
--- insets/InsetERT.h (revision 19567)
+++ insets/InsetERT.h (working copy)
@@ -33,7 +33,7 @@
class InsetERT : public InsetCollapsable {
public:
///
- InsetERT(BufferParams const &, CollapseStatus status = Open);
+ InsetERT(BufferParams const &, CollapseStatus status = Inset::Open);
#if 0
///
InsetERT(BufferParams const &,
Index: insets/InsetERT.cpp
===================================================================
--- insets/InsetERT.cpp (revision 19567)
+++ insets/InsetERT.cpp (working copy)
@@ -448,7 +448,7 @@
void InsetERTMailer::string2params(string const & in,
InsetCollapsable::CollapseStatus & status)
{
- status = InsetCollapsable::Collapsed;
+ status = Inset::Collapsed;
if (in.empty())
return;
Index: insets/InsetBranch.cpp
===================================================================
--- insets/InsetBranch.cpp (revision 19567)
+++ insets/InsetBranch.cpp (working copy)
@@ -166,13 +166,13 @@
if (cmd.argument() == "assign") {
// The branch inset uses "assign".
if (isBranchSelected(cur.buffer())) {
- if (status() != Open)
- setStatus(cur, Open);
+ if (status() != Inset::Open)
+ setStatus(cur, Inset::Open);
else
cur.undispatched();
} else {
- if (status() != Collapsed)
- setStatus(cur, Collapsed);
+ if (status() != Inset::Collapsed)
+ setStatus(cur, Inset::Collapsed);
else
cur.undispatched();
}
@@ -204,9 +204,9 @@
else if (cmd.argument() == "assign"
|| cmd.argument().empty()) {
if (isBranchSelected(cur.buffer()))
- flag.enabled(status() != Open);
+ flag.enabled(status() != Inset::Open);
else
- flag.enabled(status() != Collapsed);
+ flag.enabled(status() != Inset::Collapsed);
} else
flag.enabled(true);
break;
Index: insets/InsetCharStyle.h
===================================================================
--- insets/InsetCharStyle.h (revision 19567)
+++ insets/InsetCharStyle.h (working copy)
@@ -78,6 +78,9 @@
///
bool forceDefaultParagraphs(idx_type) const { return true; }
///
+ virtual Decoration decoration() const { return Conglomerate; }
+
+ ///
int latex(Buffer const &, odocstream &,
OutputParams const &) const;
///
Index: insets/InsetListingsParams.h
===================================================================
--- insets/InsetListingsParams.h (revision 19567)
+++ insets/InsetListingsParams.h (working copy)
@@ -26,7 +26,7 @@
///
InsetListingsParams(std::string const &, bool in=false,
- InsetCollapsable::CollapseStatus s = InsetCollapsable::Open);
+ InsetCollapsable::CollapseStatus s = Inset::Open);
/// write parameters to an ostream
void write(std::ostream &) const;
Index: insets/Inset.h
===================================================================
--- insets/Inset.h (revision 19567)
+++ insets/Inset.h (working copy)
@@ -486,7 +486,6 @@
///
enum CollapseStatus {
Collapsed,
- Inlined,
Open
};
///
Index: insets/InsetCharStyle.cpp
===================================================================
--- insets/InsetCharStyle.cpp (revision 19567)
+++ insets/InsetCharStyle.cpp (working copy)
@@ -51,7 +51,6 @@
void InsetCharStyle::init()
{
- setInlined();
setDrawFrame(false);
}
@@ -134,7 +133,6 @@
{
params_.read(lex);
InsetCollapsable::read(buf, lex);
- setInlined();
}
@@ -236,7 +234,6 @@
void InsetCharStyle::doDispatch(Cursor & cur, FuncRequest & cmd)
{
- setInlined();
switch (cmd.action) {
case LFUN_MOUSE_RELEASE:
Index: insets/InsetListingsParams.cpp
===================================================================
--- insets/InsetListingsParams.cpp (revision 19567)
+++ insets/InsetListingsParams.cpp (working copy)
@@ -696,7 +696,7 @@
ParValidator * par_validator = NULL;
InsetListingsParams::InsetListingsParams()
- : inline_(false), params_(), status_(InsetCollapsable::Open)
+ : inline_(false), params_(), status_(Inset::Open)
{
}
Index: insets/InsetCollapsable.cpp
===================================================================
--- insets/InsetCollapsable.cpp (revision 19567)
+++ insets/InsetCollapsable.cpp (working copy)
@@ -45,10 +45,32 @@
InsetCollapsable::CollapseStatus InsetCollapsable::status() const
{
- return (autoOpen_ && status_ != Inlined) ? Open : status_;
+ return autoOpen_ ? Inset::Open : status_;
}
+InsetCollapsable::Geometry InsetCollapsable::geometry() const
+{
+ switch (decoration()) {
+ case Classic:
+ if (status_ == Inset::Open || autoOpen_) {
+ if (openinlined_)
+ return OpenInlined;
+ else
+ return InsetCollapsable::Open;
+ } else
+ return InsetCollapsable::Collapsed;
+ break;
+ case InlinedDecoration:
+ return InsetCollapsable::Inlined;
+ break;
+ case Conglomerate:
+ return ( status_ == Inset::Open ? SubLabel : Corners );
+ break;
+ }
+}
+
+
InsetCollapsable::InsetCollapsable
(BufferParams const & bp, CollapseStatus status)
: InsetText(bp), label(from_ascii("Label")), status_(status),
@@ -89,15 +111,12 @@
{
os << "status ";
switch (status_) {
- case Open:
+ case Inset::Open:
os << "open";
break;
- case Collapsed:
+ case Inset::Collapsed:
os << "collapsed";
break;
- case Inlined:
- os << "inlined";
- break;
}
os << "\n";
text_.write(buf, os);
@@ -114,14 +133,11 @@
lex.next();
string const tmp_token = lex.getString();
- if (tmp_token == "inlined") {
- status_ = Inlined;
+ if (tmp_token == "collapsed") {
+ status_ = Inset::Collapsed;
token_found = true;
- } else if (tmp_token == "collapsed") {
- status_ = Collapsed;
- token_found = true;
} else if (tmp_token == "open") {
- status_ = Open;
+ status_ = Inset::Open;
token_found = true;
} else {
lyxerr << "InsetCollapsable::read: Missing
status!"
@@ -139,7 +155,7 @@
InsetText::read(buf, lex);
if (!token_found)
- status_ = isOpen() ? Open : Collapsed;
+ status_ = isOpen() ? Inset::Open : Inset::Collapsed;
setButtonLabel();
}
@@ -159,11 +175,15 @@
autoOpen_ = mi.base.bv->cursor().isInside(this);
mi.base.textwidth -= (int) (1.5 * TEXT_TO_INSET_OFFSET);
- if (status() == Inlined) {
+ switch (decoration()) {
+ case InlinedDecoration:
+ case Conglomerate:
InsetText::metrics(mi, dim);
- } else {
+ break;
+ case Classic:
dim = dimensionCollapsed();
- if (status() == Open) {
+ if (geometry() == InsetCollapsable::Open
+ || geometry() == OpenInlined) {
InsetText::metrics(mi, textdim_);
// This expression should not contain mi.base.texwidth
openinlined_ = !hasFixedWidth()
@@ -182,6 +202,7 @@
dim.wid = max(dim.wid,
mi.base.textwidth);
}
}
+ break;
}
dim.asc += TEXT_TO_INSET_OFFSET;
dim.des += TEXT_TO_INSET_OFFSET;
@@ -203,7 +224,8 @@
void InsetCollapsable::draw(PainterInfo & pi, int x, int y) const
{
const int xx = x + TEXT_TO_INSET_OFFSET;
- if (status() == Inlined) {
+
+ if (decoration() == InlinedDecoration) {
InsetText::draw(pi, xx, y);
} else {
Dimension dimc = dimensionCollapsed();
@@ -215,16 +237,30 @@
pi.pain.buttonText(xx, top + dimc.asc, label,
layout_.labelfont, mouse_hover_);
- if (status() == Open) {
- int textx, texty;
- if (openinlined_) {
- textx = xx + dimc.width();
- texty = top + textdim_.asc;
- } else {
- textx = xx;
- texty = top + dimc.height() + textdim_.asc;
- }
+ int textx, texty;
+ switch (geometry()) {
+ case OpenInlined:
+ textx = xx + dimc.width();
+ texty = top + textdim_.asc;
InsetText::draw(pi, textx, texty);
+ break;
+ case InsetCollapsable::Open:
+ textx = xx;
+ texty = top + dimc.height() + textdim_.asc;
+ InsetText::draw(pi, textx, texty);
+ break;
+ case InsetCollapsable::Collapsed:
+ break;
+ case Inlined:
+ textx = xx;
+ texty = top + textdim_.asc;
+ InsetText::draw(pi, textx, texty);
+ break;
+ case SubLabel:
+ case Corners:
+ // FIXME add handling of SubLabel, Corners
+ // still in CharStyle
+ break;
}
}
setPosCache(pi, x, y);
@@ -234,30 +270,49 @@
void InsetCollapsable::drawSelection(PainterInfo & pi, int x, int y) const
{
x += TEXT_TO_INSET_OFFSET;
- if (status() == Open) {
- if (openinlined_)
- x += dimensionCollapsed().wid;
- else
- y += dimensionCollapsed().des + textdim_.asc;
+ switch (geometry()) {
+ case OpenInlined:
+ x += dimensionCollapsed().wid;
+ InsetText::drawSelection(pi, x, y);
+ break;
+ case InsetCollapsable::Open:
+ y += dimensionCollapsed().des + textdim_.asc;
+ InsetText::drawSelection(pi, x, y);
+ break;
+ case InsetCollapsable::Collapsed:
+ break;
+ case Inlined:
+ case SubLabel:
+ case Corners:
+ InsetText::drawSelection(pi, x, y);
+ break;
}
- if (status() != Collapsed)
- InsetText::drawSelection(pi, x, y);
}
void InsetCollapsable::cursorPos(BufferView const & bv,
CursorSlice const & sl, bool boundary, int & x, int & y) const
{
- BOOST_ASSERT(status() != Collapsed);
+ BOOST_ASSERT(geometry() != InsetCollapsable::Collapsed);
InsetText::cursorPos(bv, sl, boundary, x, y);
- if (status() == Open) {
- if (openinlined_)
- x += dimensionCollapsed().wid;
- else
- y += dimensionCollapsed().height() - ascent()
- + TEXT_TO_INSET_OFFSET + textdim_.asc;
+ switch (geometry()) {
+ case OpenInlined:
+ x += dimensionCollapsed().wid;
+ break;
+ case InsetCollapsable::Open:
+ y += dimensionCollapsed().height() - ascent()
+ + TEXT_TO_INSET_OFFSET + textdim_.asc;
+ break;
+ case Inlined:
+ case SubLabel:
+ case Corners:
+ // Do nothing
+ break;
+ case InsetCollapsable::Collapsed:
+ // Cannot get here
+ break;
}
x += TEXT_TO_INSET_OFFSET;
}
@@ -265,13 +320,13 @@
Inset::EDITABLE InsetCollapsable::editable() const
{
- return status() != Collapsed ? HIGHLY_EDITABLE : IS_EDITABLE;
+ return geometry() != InsetCollapsable::Collapsed ? HIGHLY_EDITABLE :
IS_EDITABLE;
}
bool InsetCollapsable::descendable() const
{
- return status() != Collapsed;
+ return geometry() != InsetCollapsable::Collapsed;
}
@@ -313,7 +368,9 @@
Inset * InsetCollapsable::editXY(Cursor & cur, int x, int y)
{
//lyxerr << "InsetCollapsable: edit xy" << endl;
- if (status() == Collapsed || (button_dim.contains(x, y) && status() !=
Inlined))
+ if (geometry() == InsetCollapsable::Collapsed
+ || (button_dim.contains(x, y)
+ && decoration() != InlinedDecoration))
return this;
cur.push(*this);
return InsetText::editXY(cur, x, y);
@@ -327,7 +384,9 @@
switch (cmd.action) {
case LFUN_MOUSE_PRESS:
- if (cmd.button() == mouse_button::button1 && hitButton(cmd) &&
status() != Inlined) {
+ if (cmd.button() == mouse_button::button1
+ && hitButton(cmd)
+ && decoration() != InlinedDecoration) {
// reset selection if necessary (see bug 3060)
if (cur.selection())
cur.bv().cursor().clearSelection();
@@ -336,9 +395,10 @@
cur.dispatched();
break;
}
- if (status() == Inlined)
+ if (decoration() == InlinedDecoration)
InsetText::doDispatch(cur, cmd);
- else if (status() == Open && !hitButton(cmd))
+ else if (geometry() != InsetCollapsable::Collapsed
+ && !hitButton(cmd))
InsetText::doDispatch(cur, cmd);
else
cur.undispatched();
@@ -347,9 +407,10 @@
case LFUN_MOUSE_MOTION:
case LFUN_MOUSE_DOUBLE:
case LFUN_MOUSE_TRIPLE:
- if (status_ == Inlined)
+ if (decoration() == InlinedDecoration)
InsetText::doDispatch(cur, cmd);
- else if (status() && !hitButton(cmd))
+ else if (geometry() != InsetCollapsable::Collapsed
+ && !hitButton(cmd))
InsetText::doDispatch(cur, cmd);
else
cur.undispatched();
@@ -362,7 +423,7 @@
break;
}
- if (status() == Inlined) {
+ if (decoration() == InlinedDecoration) {
// The mouse click has to be within the inset!
InsetText::doDispatch(cur, cmd);
break;
@@ -377,34 +438,35 @@
// toggle the inset visual state.
cur.dispatched();
cur.updateFlags(Update::Force | Update::FitCursor);
- if (status() == Collapsed) {
- setStatus(cur, Open);
+ if (geometry() == InsetCollapsable::Collapsed) {
+ setStatus(cur, Inset::Open);
edit(cur, true);
}
else {
- setStatus(cur, Collapsed);
+ setStatus(cur, Inset::Collapsed);
}
cur.bv().cursor() = cur;
break;
}
// The mouse click is within the opened inset.
- if (status() == Open)
+ if (geometry() == InsetCollapsable::Open
+ || geometry() == OpenInlined)
InsetText::doDispatch(cur, cmd);
break;
case LFUN_INSET_TOGGLE:
if (cmd.argument() == "open")
- setStatus(cur, Open);
+ setStatus(cur, Inset::Open);
else if (cmd.argument() == "close")
- setStatus(cur, Collapsed);
+ setStatus(cur, Inset::Collapsed);
else if (cmd.argument() == "toggle" || cmd.argument().empty())
if (isOpen()) {
- setStatus(cur, Collapsed);
+ setStatus(cur, Inset::Collapsed);
cur.top().forwardPos();
}
else
- setStatus(cur, Open);
+ setStatus(cur, Inset::Open);
else // if assign or anything else
cur.undispatched();
cur.dispatched();
@@ -446,7 +508,7 @@
{
status_ = status;
setButtonLabel();
- if (status_ == Collapsed)
+ if (status_ == Inset::Collapsed)
cur.leaveInset(*this);
// Because the collapse status is part of the inset and thus an
// integral part of the Buffer contents a changed status must be
Index: insets/InsetCollapsable.h
===================================================================
--- insets/InsetCollapsable.h (revision 19567)
+++ insets/InsetCollapsable.h (working copy)
@@ -42,7 +42,7 @@
///
static int const TEXT_TO_BOTTOM_OFFSET = 2;
///
- InsetCollapsable(BufferParams const &, CollapseStatus status = Open);
+ InsetCollapsable(BufferParams const &, CollapseStatus status =
Inset::Open);
///
InsetCollapsable(InsetCollapsable const & rhs);
///
@@ -77,12 +77,52 @@
///
void setLabelFont(Font const & f);
///
- bool isOpen() const { return status_ == Open || status_ == Inlined; }
+ bool isOpen() const { return geometry() != InsetCollapsable::Collapsed;
}
///
- bool inlined() const { return status_ == Inlined; }
+ bool inlined() const { return decoration() == InlinedDecoration ||
decoration() == Conglomerate; }
///
CollapseStatus status() const;
+ /** Of the old CollapseStatus we only keep the values
+ * Open and Collapsed.
+ * We define a list of possible inset decoration
+ * styles, and a list of possible (concrete, visual)
+ * inset geometries. Relationships between them
+ * (geometries in body of table):
+ *
+ * \ CollapseStatus:
+ * Decoration: \ Open Collapsed
+ * -------------+---------------------------
+ * Classic | *) Open, <--x) Collapsed
+ * | OpenInlined
+ * InlinedDec | Inlined Inlined
+ * Conglomerate | SubLabel Corners
+ * -----------------------------------------
+ * *) toggled by openinlined_
+ * x) toggled by autoOpen_
+ */
+
///
+ enum Decoration {
+ Classic,
+ InlinedDecoration,
+ Conglomerate
+ };
+ /// Default looks
+ virtual Decoration decoration() const { return Classic; }
+ ///
+ enum Geometry {
+ Open,
+ Collapsed,
+ Inlined,
+ OpenInlined,
+ SubLabel,
+ Corners
+ };
+ /// Returns the geometry based on CollapseStatus
+ /// (status_), autoOpen_ and openinlined_, and of
+ /// course decoration().
+ Geometry geometry() const;
+ ///
bool allowSpellCheck() const { return true; }
///
bool getStatus(Cursor &, FuncRequest const &, FuncStatus &) const;
@@ -103,8 +143,6 @@
///
Inset * editXY(Cursor & cur, int x, int y);
///
- void setInlined() { status_ = Inlined; }
- ///
docstring floatName(std::string const & type, BufferParams const &)
const;
protected:
ndex: frontends/qt4/ui/ERTUi.ui
===================================================================
--- frontends/qt4/ui/ERTUi.ui (revision 19567)
+++ frontends/qt4/ui/ERTUi.ui (working copy)
@@ -37,17 +37,7 @@
<property name="spacing" >
<number>6</number>
</property>
- <item>
- <widget class="QRadioButton" name="inlineRB" >
- <property name="toolTip" >
- <string>Show ERT inline</string>
- </property>
- <property name="text" >
- <string>&Inline</string>
- </property>
- </widget>
- </item>
- <item>
+ <item>
<widget class="QRadioButton" name="collapsedRB" >
<property name="toolTip" >
<string>Show ERT button only</string>
Index: frontends/qt4/QERT.cpp
===================================================================
--- frontends/qt4/QERT.cpp (revision 19567)
+++ frontends/qt4/QERT.cpp (working copy)
@@ -36,7 +36,6 @@
setupUi(this);
connect(okPB, SIGNAL(clicked()), form, SLOT(slotOK()));
connect(closePB, SIGNAL(clicked()), form, SLOT(slotClose()));
- connect(inlineRB, SIGNAL(clicked()), this, SLOT(change_adaptor()));
connect(collapsedRB, SIGNAL(clicked()), this, SLOT(change_adaptor()));
connect(openRB, SIGNAL(clicked()), this, SLOT(change_adaptor()));
}
@@ -82,11 +81,9 @@
void QERT::apply()
{
if (dialog_->openRB->isChecked())
- controller().setStatus(InsetERT::Open);
- else if (dialog_->inlineRB->isChecked())
- controller().setStatus(InsetERT::Inlined);
+ controller().setStatus(Inset::Open);
else
- controller().setStatus(InsetERT::Collapsed);
+ controller().setStatus(Inset::Collapsed);
}
@@ -95,9 +92,8 @@
QRadioButton * rb = 0;
switch (controller().status()) {
- case InsetERT::Open: rb = dialog_->openRB; break;
- case InsetERT::Inlined: rb = dialog_->inlineRB; break;
- case InsetERT::Collapsed: rb = dialog_->collapsedRB; break;
+ case Inset::Open: rb = dialog_->openRB; break;
+ case Inset::Collapsed: rb = dialog_->collapsedRB; break;
}
rb->setChecked(true);
Index: frontends/controllers/ControlERT.cpp
===================================================================
--- frontends/controllers/ControlERT.cpp (revision 19567)
+++ frontends/controllers/ControlERT.cpp (working copy)
@@ -21,7 +21,7 @@
namespace frontend {
ControlERT::ControlERT(Dialog & parent)
- : Dialog::Controller(parent), status_(InsetERT::Collapsed)
+ : Dialog::Controller(parent), status_(Inset::Collapsed)
{}
@@ -34,7 +34,7 @@
void ControlERT::clearParams()
{
- status_ = InsetERT::Collapsed;
+ status_ = Inset::Collapsed;
}
