[patch] bug 1921: lyxbreaker is still asserting...
While thinking about the last (!) remaining critical bug in the 1.4 pipe, I
stumbled over an old patch from Alfredo, which was rejected due to the
feature freeze
(http://marc.theaimsgroup.com/?l=lyx-devel&m=110186623406865&q=p3)
The thing is, now, that this patch fixes the assert in a really elegant way:
collapsables are automatically opened when the cursor enters (and closed when
it leaves, except status_ is open). Something like this has to be done anyway
to cure the bug. I think this is enough judgement to let the minor feature
enhancement, which the patch originally aimed at, pass thru.
I have adapted the patch to current cvs (see attached). The autoopen feature
is not yet used by the spellchecker, though (which would be a oneliner, I
guess, but could certainly also wait).
Lars, could this get into favour with you?
Jürgen
Index: insetcollapsable.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/insets/insetcollapsable.C,v
retrieving revision 1.278
diff -p -u -r1.278 insetcollapsable.C
--- insetcollapsable.C 15 Jul 2005 22:10:21 - 1.278
+++ insetcollapsable.C 16 Sep 2005 14:06:55 -
@@ -40,9 +40,16 @@ using std::min;
using std::ostream;
+InsetCollapsable::CollapseStatus InsetCollapsable::status() const
+{
+ return autoOpen_ ? Open : status_;
+}
+
+
InsetCollapsable::InsetCollapsable
(BufferParams const & bp, CollapseStatus status)
- : InsetText(bp), label("Label"), status_(status), openinlined_(false)
+ : InsetText(bp), label("Label"), status_(status),
+ openinlined_(false), autoOpen_(false)
{
setAutoBreakRows(true);
setDrawFrame(true);
@@ -122,12 +129,14 @@ Dimension InsetCollapsable::dimensionCol
void InsetCollapsable::metrics(MetricsInfo & mi, Dimension & dim) const
{
+ autoOpen_ = mi.base.bv->cursor().isInside(this);
mi.base.textwidth -= 2 * TEXT_TO_INSET_OFFSET;
- if (status_ == Inlined) {
+
+ if (status() == Inlined) {
InsetText::metrics(mi, dim);
} else {
dim = dimensionCollapsed();
- if (status_ == Open) {
+ if (status() == Open) {
InsetText::metrics(mi, textdim_);
openinlined_ = (textdim_.wid + dim.wid <= mi.base.textwidth);
if (openinlined_) {
@@ -151,7 +160,7 @@ void InsetCollapsable::metrics(MetricsIn
void InsetCollapsable::draw(PainterInfo & pi, int x, int y) const
{
const int xx = x + TEXT_TO_INSET_OFFSET;
- if (status_ == Inlined) {
+ if (status() == Inlined) {
InsetText::draw(pi, xx, y);
} else {
Dimension dimc = dimensionCollapsed();
@@ -162,7 +171,7 @@ void InsetCollapsable::draw(PainterInfo
button_dim.y2 = top + dimc.height();
pi.pain.buttonText(xx, top + dimc.asc, label, labelfont_);
- if (status_ == Open) {
+ if (status() == Open) {
int textx, texty;
if (openinlined_) {
textx = xx + dimc.width();
@@ -181,13 +190,13 @@ void InsetCollapsable::draw(PainterInfo
void InsetCollapsable::drawSelection(PainterInfo & pi, int x, int y) const
{
x += TEXT_TO_INSET_OFFSET;
- if (status_ == Open) {
+ if (status() == Open) {
if (openinlined_)
x += dimensionCollapsed().wid;
else
y += dimensionCollapsed().des + textdim_.asc;
}
- if (status_ != Collapsed)
+ if (status() != Collapsed)
InsetText::drawSelection(pi, x, y);
}
@@ -195,32 +204,30 @@ void InsetCollapsable::drawSelection(Pai
void InsetCollapsable::cursorPos
(CursorSlice const & sl, bool boundary, int & x, int & y) const
{
- if (status_ == Collapsed) {
- x = xo();
- y = yo();
- } else {
- InsetText::cursorPos(sl, boundary, x, y);
- if (status_ == Open) {
- if (openinlined_)
-x += dimensionCollapsed().wid;
- else
-y += dimensionCollapsed().height() - ascent()
- + TEXT_TO_INSET_OFFSET + textdim_.asc;
- }
- x += TEXT_TO_INSET_OFFSET;
+ BOOST_ASSERT(status() != Collapsed);
+
+ InsetText::cursorPos(sl, boundary, x, y);
+
+ if (status() == Open) {
+ if (openinlined_)
+ x += dimensionCollapsed().wid;
+ else
+ y += dimensionCollapsed().height() - ascent()
++ TEXT_TO_INSET_OFFSET + textdim_.asc;
}
+ x += TEXT_TO_INSET_OFFSET;
}
InsetBase::EDITABLE InsetCollapsable::editable() const
{
- return status_ != Collapsed ? HIGHLY_EDITABLE : IS_EDITABLE;
+ return status() != Collapsed ? HIGHLY_EDITABLE : IS_EDITABLE;
}
bool InsetCollapsable::descendable() const
{
- return status_ != Collapsed;
+ return status() != Collapsed;
}
@@ -262,7 +269,7 @@ void InsetCollapsable::edit(LCursor & cu
InsetBase * InsetCollapsable::editXY(LCursor & cur, int x, int y)
{
//lyxerr << "InsetCollapsable: edit xy" << endl;
- if (status_ == Collapsed)
+ if (status() == Collapsed)
return this;
cur.push(*this);
return InsetText::editXY(cur, x, y);
@@ -276,9 +283,9 @@ void InsetCollapsable::doDispatch(LCurso
switch (cmd.action) {
case LFUN_MOUSE_PRESS:
- if (status_ == Inlined)
+ if (status() == Inlined)
InsetText::doDispatch(cur, cmd);
- else if (status_ == Open && !hitButton(cmd))
+ else if (status() == Open && !h
Re: [patch] bug 1921: lyxbreaker is still asserting...
> "Juergen" == Juergen Spitzmueller <[EMAIL PROTECTED]> writes:
Juergen> Juergen Spitzmueller wrote:
>> I have adapted the patch to current cvs (see attached).
Juergen> After some more testing, I came up with the attached,
Juergen> slightly improved version, which does also respect inlined
Juergen> (as well opened) insets (i.e. does not force inlined ERTs to
Juergen> opened ones when the cursor enters, which btw also s&r
Juergen> currently does, due to André's change below). The only
Juergen> difference to the prior patch is the change in status().
Hello Juergen,
Since the auto-open feature is a `big' change, code-wise (should
auto-open be a property of the cursor or of the inset?) and UI-wise
(do I want closed footnotes to pop-up on me when I do char-forward?),
I propose a much less ambitious patch.
I removes André's patch as you did, but for the reason that everything
is done in BufferView::setCursor now. Although I took the occasion to
use the same code that André used (on the premise that he knows what
he does), the main fix in the patch is to remove a use of cursor()
where dit should have been used instead.
As far as I know this fixes 1921, but I'd be interested to see what
was André's patch supposed to fix, to check that I did not break
anything.
JMarc
Index: src/BufferView.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/BufferView.C,v
retrieving revision 1.263
diff -u -p -r1.263 BufferView.C
--- src/BufferView.C 15 Jul 2005 22:10:15 - 1.263
+++ src/BufferView.C 22 Sep 2005 10:28:14 -
@@ -323,12 +323,12 @@ LyXText * BufferView::text() const
void BufferView::setCursor(DocIterator const & dit)
{
- size_t const n = dit.depth();
- for (size_t i = 0; i < n; ++i)
- dit[i].inset().edit(cursor(), true);
-
cursor().setCursor(dit);
cursor().selection() = false;
+ // Open all collapsed insets
+ for (int i = cursor().depth() - 1; i >= 0; --i)
+ cursor()[i].inset().setStatus(cursor(), InsetBase::Open);
+
}
Index: src/ChangeLog
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/ChangeLog,v
retrieving revision 1.2288
diff -u -p -r1.2288 ChangeLog
--- src/ChangeLog 21 Sep 2005 13:18:14 - 1.2288
+++ src/ChangeLog 22 Sep 2005 10:28:15 -
@@ -1,3 +1,12 @@
+2005-09-22 Jean-Marc Lasgouttes <[EMAIL PROTECTED]>
+
+ * cursor.C (setSelection): do not open collapsable insets; this is
+ done in the only caller (BufferView::putSelection) by a call to
+ BufferView::setCursor.
+
+ * BufferView.C (setCursor): open correctly collapsable insets.
+ (bug 1921)
+
2005-09-12 Jean-Marc Lasgouttes <[EMAIL PROTECTED]>
* lyxfunc.C (sendDispatchMessage): do not update menubar/toolbar
Index: src/cursor.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/cursor.C,v
retrieving revision 1.135
diff -u -p -r1.135 cursor.C
--- src/cursor.C 21 Sep 2005 09:56:54 - 1.135
+++ src/cursor.C 22 Sep 2005 10:28:15 -
@@ -468,9 +468,6 @@ void LCursor::setSelection(DocIterator c
selection() = true;
anchor_ = where;
pos() += n;
- // Open all collapsed insets
- for (int i = depth() - 1; i >= 0; --i)
- operator[](i).inset().setStatus(*this, InsetBase::Open);
}
Re: [patch] bug 1921: lyxbreaker is still asserting...
Jean-Marc Lasgouttes wrote: > Since the auto-open feature is a `big' change, code-wise (should > auto-open be a property of the cursor or of the inset?) It's not such a big change. Most of the change is status_ -> status(). > and UI-wise > (do I want closed footnotes to pop-up on me when I do char-forward?), That does not happen. char-forward skips closed footnotes, as it used to be. > I propose a much less ambitious patch. > > I removes André's patch as you did, but for the reason that everything > is done in BufferView::setCursor now. Although I took the occasion to > use the same code that André used (on the premise that he knows what > he does), the main fix in the patch is to remove a use of cursor() > where dit should have been used instead. Without having actually tested your patch, I think this introduces two problems: (1) all insets are forced to status Open (even inlined ERT insets). At least this should be fixed, the manual reversion of this change is too annoying. (2) one of the most-hated behaviour is back: all insets remain open after s&r _and_ spellchecking. We had fixed at least a big part of it (s&r), so the patch is a ui-regression wrt 1.3 (while Alfredo's fixes the problem completely, i.e. also after spellchecking, next-note etc.). > As far as I know this fixes 1921, but I'd be interested to see what > was André's patch supposed to fix, to check that I did not break > anything. André might correct me, but the comment indicates that the patch also aimed at bug 1921. Regards, Jürgen
Re: [patch] bug 1921: lyxbreaker is still asserting...
> "Juergen" == Juergen Spitzmueller <[EMAIL PROTECTED]> writes:
Juergen> Jean-Marc Lasgouttes wrote:
>> Since the auto-open feature is a `big' change, code-wise (should
>> auto-open be a property of the cursor or of the inset?)
Juergen> It's not such a big change. Most of the change is status_ ->
Juergen> status().
I took a closer look at the patch and I agree with you.
Juergen> Without having actually tested your patch,
I see you use the same method as I do :)
Juergen> I think this introduces two problems: (1) all insets are
Juergen> forced to status Open (even inlined ERT insets). At least
Juergen> this should be fixed, the manual reversion of this change is
Juergen> too annoying. (2) one of the most-hated behaviour is back:
Juergen> all insets remain open after s&r _and_ spellchecking.
Well, the is no auto-open (obviously) but this has been the case.
Juergen> We had fixed at least a big part of it (s&r), so the patch is
Juergen> a ui-regression wrt 1.3 (while Alfredo's fixes the problem
Juergen> completely, i.e. also after spellchecking, next-note etc.).
Only the insets where a match is found get opened. I do not see why my
patch could get us back to the old behaviour where all insets got
opened...
As far as I can see, the only difference is whether insets will close
again when one leaves the insets. So the difference is an enhancement.
One thing that makes me nervous with your approach is that it does not
touch BufferView::setCursor:
void BufferView::setCursor(DocIterator const & dit)
{
size_t const n = dit.depth();
for (size_t i = 0; i < n; ++i)
dit[i].inset().edit(cursor(), true);
cursor().setCursor(dit);
cursor().selection() = false;
}
The loop in this code calls edit() with the last value of cursor(),
not the new value that we set later. It seems to me that this can
create big problems (also I saw none yet). Should this code be moved
after the cursor().setCursorcall?
JMarc
Re: [patch] bug 1921: lyxbreaker is still asserting...
Jean-Marc Lasgouttes wrote:
> Juergen> Without having actually tested your patch,
>
> I see you use the same method as I do :)
;-)
(actually, I didn't know where to test it. Both my trees have been full of
changes lately, and I'd like to keep track).
> Juergen> We had fixed at least a big part of it (s&r), so the patch is
> Juergen> a ui-regression wrt 1.3 (while Alfredo's fixes the problem
> Juergen> completely, i.e. also after spellchecking, next-note etc.).
>
> Only the insets where a match is found get opened. I do not see why my
> patch could get us back to the old behaviour where all insets got
> opened...
Sorry, I haven't expressed myself clearly. All insets where a match is found
get and remain opened. In 1.3, we close them again after leaving, and I think
this feature is lost with you patch (I haven't tested, so please correct me
if I'm wrong).
> As far as I can see, the only difference is whether insets will close
> again when one leaves the insets. So the difference is an enhancement.
Partly. As said, in case of s&r, we already close, so we have a regression
with your patch, an enhancement with Alfredo's.
> One thing that makes me nervous with your approach is that it does not
> touch BufferView::setCursor:
>
> void BufferView::setCursor(DocIterator const & dit)
> {
> size_t const n = dit.depth();
> for (size_t i = 0; i < n; ++i)
> dit[i].inset().edit(cursor(), true);
>
> cursor().setCursor(dit);
> cursor().selection() = false;
> }
>
>
> The loop in this code calls edit() with the last value of cursor(),
> not the new value that we set later. It seems to me that this can
> create big problems (also I saw none yet). Should this code be moved
> after the cursor().setCursorcall?
This looks suspicious indeed. I don't really know why it is like it is (there
might be reasons).
Anyway, if you'd like me to change the order in that function, I can do that.
That does not particularly disqualify my approach, no?
Jürgen
Re: [patch] bug 1921: lyxbreaker is still asserting...
> "Juergen" == Juergen Spitzmueller <[EMAIL PROTECTED]> writes: >> As far as I can see, the only difference is whether insets will >> close again when one leaves the insets. So the difference is an >> enhancement. Juergen> Partly. As said, in case of s&r, we already close, so we have Juergen> a regression with your patch, an enhancement with Alfredo's. Sorry, I actually did not realize that we did that. Juergen> This looks suspicious indeed. I don't really know why it is Juergen> like it is (there might be reasons). Anyway, if you'd like me Juergen> to change the order in that function, I can do that. Yes, please. Juergen> That does not particularly disqualify my approach, no? Did I write that? I meant to say that your patch is OK with me, except that I would like this particular bit of code (which is not yours) fixed. JMarc
Re: [patch] bug 1921: lyxbreaker is still asserting...
Jean-Marc Lasgouttes wrote:
> Juergen> That does not particularly disqualify my approach, no?
>
> Did I write that?
I just wanted to make clear that I got you right. Seems that misunderstandings
scale very well ;-)
> I meant to say that your patch is OK with me, except
> that I would like this particular bit of code (which is not yours)
> fixed.
I tried to move the setCursor call in front of the loop, but that leads to
cursor misdrawings in the insets, so setCursor _has_ to be called afterwards.
Now I have put a call before the loop and left the one behind (see attached
patch). Does that make sense to you?
Jürgen
Index: BufferView.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/BufferView.C,v
retrieving revision 1.263
diff -u -r1.263 BufferView.C
--- BufferView.C 15 Jul 2005 22:10:15 - 1.263
+++ BufferView.C 27 Sep 2005 15:57:55 -
@@ -323,6 +323,7 @@
void BufferView::setCursor(DocIterator const & dit)
{
+ cursor().setCursor(dit);
size_t const n = dit.depth();
for (size_t i = 0; i < n; ++i)
dit[i].inset().edit(cursor(), true);
Index: cursor.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/cursor.C,v
retrieving revision 1.135
diff -u -r1.135 cursor.C
--- cursor.C 21 Sep 2005 09:56:54 - 1.135
+++ cursor.C 27 Sep 2005 15:58:00 -
@@ -468,9 +468,6 @@
selection() = true;
anchor_ = where;
pos() += n;
- // Open all collapsed insets
- for (int i = depth() - 1; i >= 0; --i)
- operator[](i).inset().setStatus(*this, InsetBase::Open);
}
Index: insets/insetcollapsable.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/insets/insetcollapsable.C,v
retrieving revision 1.278
diff -u -r1.278 insetcollapsable.C
--- insets/insetcollapsable.C 15 Jul 2005 22:10:21 - 1.278
+++ insets/insetcollapsable.C 27 Sep 2005 15:58:05 -
@@ -40,9 +40,16 @@
using std::ostream;
+InsetCollapsable::CollapseStatus InsetCollapsable::status() const
+{
+ return (autoOpen_ && status_ != Inlined) ? Open : status_;
+}
+
+
InsetCollapsable::InsetCollapsable
(BufferParams const & bp, CollapseStatus status)
- : InsetText(bp), label("Label"), status_(status), openinlined_(false)
+ : InsetText(bp), label("Label"), status_(status),
+ openinlined_(false), autoOpen_(false)
{
setAutoBreakRows(true);
setDrawFrame(true);
@@ -122,12 +129,14 @@
void InsetCollapsable::metrics(MetricsInfo & mi, Dimension & dim) const
{
+ autoOpen_ = mi.base.bv->cursor().isInside(this);
mi.base.textwidth -= 2 * TEXT_TO_INSET_OFFSET;
- if (status_ == Inlined) {
+
+ if (status() == Inlined) {
InsetText::metrics(mi, dim);
} else {
dim = dimensionCollapsed();
- if (status_ == Open) {
+ if (status() == Open) {
InsetText::metrics(mi, textdim_);
openinlined_ = (textdim_.wid + dim.wid <= mi.base.textwidth);
if (openinlined_) {
@@ -151,7 +160,7 @@
void InsetCollapsable::draw(PainterInfo & pi, int x, int y) const
{
const int xx = x + TEXT_TO_INSET_OFFSET;
- if (status_ == Inlined) {
+ if (status() == Inlined) {
InsetText::draw(pi, xx, y);
} else {
Dimension dimc = dimensionCollapsed();
@@ -162,7 +171,7 @@
button_dim.y2 = top + dimc.height();
pi.pain.buttonText(xx, top + dimc.asc, label, labelfont_);
- if (status_ == Open) {
+ if (status() == Open) {
int textx, texty;
if (openinlined_) {
textx = xx + dimc.width();
@@ -181,13 +190,13 @@
void InsetCollapsable::drawSelection(PainterInfo & pi, int x, int y) const
{
x += TEXT_TO_INSET_OFFSET;
- if (status_ == Open) {
+ if (status() == Open) {
if (openinlined_)
x += dimensionCollapsed().wid;
else
y += dimensionCollapsed().des + textdim_.asc;
}
- if (status_ != Collapsed)
+ if (status() != Collapsed)
InsetText::drawSelection(pi, x, y);
}
@@ -195,32 +204,30 @@
void InsetCollapsable::cursorPos
(CursorSlice const & sl, bool boundary, int & x, int & y) const
{
- if (status_ == Collapsed) {
- x = xo();
- y = yo();
- } else {
- InsetText::cursorPos(sl, boundary, x, y);
- if (status_ == Open) {
- if (openinlined_)
-x += dimensionCollapsed().wid;
- else
-y += dimensionCollapsed().height() - ascent()
- + TEXT_TO_INSET_OFFSET + textdim_.asc;
- }
- x += TEXT_TO_INSET_OFFSET;
+ BOOST_ASSERT(status() != Collapsed);
+
+ InsetText::cursorPos(sl, boundary, x, y);
+
+ if (status() == Open) {
+ if (openinlined_)
+ x += dimensionCollapsed().wid;
+ else
+ y += dimensionCollapsed().height() - ascent()
++ TEXT_TO_INSET_OFFSET + textdim_.asc;
}
+ x += TEXT_TO_INSET_OFFSET;
}
InsetBase::EDITABLE InsetCollapsable::editable() const
{
- return status_ != Collapsed ? HIGHLY_EDITABLE : IS_EDITABLE;
+ return status() != Collapsed ? HIGHLY_EDITABLE : IS_EDITABLE;
}
bool InsetCollapsable::descendable() const
{
- return status_ != Collapsed;
+ return status() != Collapsed;
Re: [patch] bug 1921: lyxbreaker is still asserting...
> "Juergen" == Juergen Spitzmueller <[EMAIL PROTECTED]> writes: Juergen> Jean-Marc Lasgouttes wrote: That does not particularly Juergen> disqualify my approach, no? >> Did I write that? Juergen> I just wanted to make clear that I got you right. Seems that Juergen> misunderstandings scale very well ;-) :) Juergen> I tried to move the setCursor call in front of the loop, but Juergen> that leads to cursor misdrawings in the insets, so setCursor Juergen> _has_ to be called afterwards. Juergen> Now I have put a call before the loop and left the one behind Juergen> (see attached patch). Does that make sense to you? It does not really make more sense than what we have, IMO. I propose to leave this code alone (I have no evidence that it is actually broken), apply the autoopen patch and revert andre's patch. JMarc
Re: [patch] bug 1921: lyxbreaker is still asserting...
Jean-Marc Lasgouttes wrote: > It does not really make more sense than what we have, IMO. I propose > to leave this code alone (I have no evidence that it is actually > broken), apply the autoopen patch and revert andre's patch. OK, it's in. Many thanks for being such a patient and careful reviewer. Jürgen
Re: [patch] bug 1921: lyxbreaker is still asserting...
> "Juergen" == Juergen Spitzmueller <[EMAIL PROTECTED]> writes: Juergen> OK, it's in. Many thanks for being such a patient and careful Juergen> reviewer. I think I can share the 'patience' part with you. JMarc
Re: [patch] bug 1921: lyxbreaker is still asserting...
Juergen Spitzmueller <[EMAIL PROTECTED]> writes: | While thinking about the last (!) remaining critical bug in the 1.4 pipe, I | stumbled over an old patch from Alfredo, which was rejected due to the | feature freeze | (http://marc.theaimsgroup.com/?l=lyx-devel&m=110186623406865&q=p3) | | The thing is, now, that this patch fixes the assert in a really elegant way: | collapsables are automatically opened when the cursor enters (and closed when | it leaves, except status_ is open). Something like this has to be done anyway | to cure the bug. I think this is enough judgement to let the minor feature | enhancement, which the patch originally aimed at, pass thru. | | I have adapted the patch to current cvs (see attached). The autoopen feature | is not yet used by the spellchecker, though (which would be a oneliner, I | guess, but could certainly also wait). | | Lars, could this get into favour with you? Test it hard, and get ok from J-M as well. -- Lgb
Re: [patch] bug 1921: lyxbreaker is still asserting...
Lars Gullik Bjønnes wrote: > Test it hard, and get ok from J-M as well. Certainly. Maybe even André finds the time to review the three assert fixes over the weekend. Jürgen
Re: [patch] bug 1921: lyxbreaker is still asserting...
Juergen Spitzmueller wrote:
> I have adapted the patch to current cvs (see attached).
After some more testing, I came up with the attached, slightly improved
version, which does also respect inlined (as well opened) insets (i.e. does
not force inlined ERTs to opened ones when the cursor enters, which btw also
s&r currently does, due to André's change below).
The only difference to the prior patch is the change in status().
> The autoopen
> feature is not yet used by the spellchecker, though (which would be a
> oneliner, I guess, but could certainly also wait).
The reason why they are not closed is this recent change by André in cursor.C,
which ought to fix this very problem (but doesn't), if I'm not mistaken.
http://www.lyx.org/cgi-bin/viewcvs.cgi/lyx-devel/src/cursor.C.diff?r1=1.126&r2=1.127
I think this change can (and should be) reverted now (see second patch). I
have done this here and haven't found any regression.
André, are you listening?
Jürgen
Index: cursor.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/cursor.C,v
retrieving revision 1.133
diff -p -u -r1.133 cursor.C
--- cursor.C 18 Jul 2005 17:12:22 - 1.133
+++ cursor.C 17 Sep 2005 14:10:49 -
@@ -467,9 +467,6 @@ void LCursor::setSelection(DocIterator c
selection() = true;
anchor_ = where;
pos() += n;
- // Open all collapsed insets
- for (int i = depth() - 1; i >= 0; --i)
- operator[](i).inset().setStatus(*this, InsetBase::Open);
}
Index: insetcollapsable.C
===
RCS file: /usr/local/lyx/cvsroot/lyx-devel/src/insets/insetcollapsable.C,v
retrieving revision 1.278
diff -p -u -r1.278 insetcollapsable.C
--- insetcollapsable.C 15 Jul 2005 22:10:21 - 1.278
+++ insetcollapsable.C 17 Sep 2005 14:10:23 -
@@ -40,9 +40,16 @@ using std::min;
using std::ostream;
+InsetCollapsable::CollapseStatus InsetCollapsable::status() const
+{
+ return (autoOpen_ && status_ != Inlined) ? Open : status_;
+}
+
+
InsetCollapsable::InsetCollapsable
(BufferParams const & bp, CollapseStatus status)
- : InsetText(bp), label("Label"), status_(status), openinlined_(false)
+ : InsetText(bp), label("Label"), status_(status),
+ openinlined_(false), autoOpen_(false)
{
setAutoBreakRows(true);
setDrawFrame(true);
@@ -122,12 +129,14 @@ Dimension InsetCollapsable::dimensionCol
void InsetCollapsable::metrics(MetricsInfo & mi, Dimension & dim) const
{
+ autoOpen_ = mi.base.bv->cursor().isInside(this);
mi.base.textwidth -= 2 * TEXT_TO_INSET_OFFSET;
- if (status_ == Inlined) {
+
+ if (status() == Inlined) {
InsetText::metrics(mi, dim);
} else {
dim = dimensionCollapsed();
- if (status_ == Open) {
+ if (status() == Open) {
InsetText::metrics(mi, textdim_);
openinlined_ = (textdim_.wid + dim.wid <= mi.base.textwidth);
if (openinlined_) {
@@ -151,7 +160,7 @@ void InsetCollapsable::metrics(MetricsIn
void InsetCollapsable::draw(PainterInfo & pi, int x, int y) const
{
const int xx = x + TEXT_TO_INSET_OFFSET;
- if (status_ == Inlined) {
+ if (status() == Inlined) {
InsetText::draw(pi, xx, y);
} else {
Dimension dimc = dimensionCollapsed();
@@ -162,7 +171,7 @@ void InsetCollapsable::draw(PainterInfo
button_dim.y2 = top + dimc.height();
pi.pain.buttonText(xx, top + dimc.asc, label, labelfont_);
- if (status_ == Open) {
+ if (status() == Open) {
int textx, texty;
if (openinlined_) {
textx = xx + dimc.width();
@@ -181,13 +190,13 @@ void InsetCollapsable::draw(PainterInfo
void InsetCollapsable::drawSelection(PainterInfo & pi, int x, int y) const
{
x += TEXT_TO_INSET_OFFSET;
- if (status_ == Open) {
+ if (status() == Open) {
if (openinlined_)
x += dimensionCollapsed().wid;
else
y += dimensionCollapsed().des + textdim_.asc;
}
- if (status_ != Collapsed)
+ if (status() != Collapsed)
InsetText::drawSelection(pi, x, y);
}
@@ -195,32 +204,30 @@ void InsetCollapsable::drawSelection(Pai
void InsetCollapsable::cursorPos
(CursorSlice const & sl, bool boundary, int & x, int & y) const
{
- if (status_ == Collapsed) {
- x = xo();
- y = yo();
- } else {
- InsetText::cursorPos(sl, boundary, x, y);
- if (status_ == Open) {
- if (openinlined_)
-x += dimensionCollapsed().wid;
- else
-y += dimensionCollapsed().height() - ascent()
- + TEXT_TO_INSET_OFFSET + textdim_.asc;
- }
- x += TEXT_TO_INSET_OFFSET;
+ BOOST_ASSERT(status() != Collapsed);
+
+ InsetText::cursorPos(sl, boundary, x, y);
+
+ if (status() == Open) {
+ if (openinlined_)
+ x += dimensionCollapsed().wid;
+ else
+ y += dimensionCollapsed().height() - ascent()
++ TEXT_TO_INSET_OFFSET + textdim_.asc;
}
+ x += TEXT_TO_INSET_OFFSET;
}
InsetBase::EDITABLE InsetCollapsable::editable() const
{
- return status_ != Collapsed ? HIGHLY_EDITABLE : IS_EDITABLE;
+ return status() != Collapsed ? HIGHLY_EDITABLE : IS_EDITABLE;
}
bool InsetCollapsable::d
