Re: Road to rc2 - fixIfBroken
On Sat, Jun 09, 2007 at 12:49:09PM +0200, Stefan Schimanski wrote:
It works fine (as far as I can judge after 2 minutes testing). I
added some comments and pulled apart the loop to make the control
flow easier. Alfredo, can you check please? I would be happy if we
could get rid of the signals finally like this.
Stefan
Index: lyx-devel/src/CursorSlice.cpp
===
--- lyx-devel.orig/src/CursorSlice.cpp2007-06-02 11:24:26.0
+0200
+++ lyx-devel/src/CursorSlice.cpp 2007-06-09 12:11:13.0 +0200
@@ -42,10 +42,6 @@
: inset_(p), idx_(0), pit_(0), pos_(0)
{
BOOST_ASSERT(inset_);
- boost::signalvoid() * destroyed_signal = inset_-destroyedSignal();
- if (destroyed_signal)
- inset_connection_ = destroyed_signal-connect(
- boost::bind(CursorSlice::invalidate, this));
}
@@ -55,22 +51,12 @@
}
-CursorSlice::~CursorSlice()
-{
- inset_connection_.disconnect();
-}
-
-
CursorSlice CursorSlice::operator=(CursorSlice const cs)
{
inset_ = cs.inset_;
idx_ = cs.idx_;
pit_ = cs.pit_;
pos_ = cs.pos_;
- if (inset_ inset_-destroyedSignal()) {
- inset_connection_ = inset_-destroyedSignal()-connect(
- boost::bind(CursorSlice::invalidate, this));
- }
return *this;
}
Note that probably the whole operator=() can go.
Andre'
Re: Road to rc2 - fixIfBroken
CursorSlice CursorSlice::operator=(CursorSlice const cs)
{
inset_ = cs.inset_;
idx_ = cs.idx_;
pit_ = cs.pit_;
pos_ = cs.pos_;
- if (inset_ inset_-destroyedSignal()) {
- inset_connection_ = inset_-destroyedSignal()-connect(
- boost::bind(CursorSlice::invalidate, this));
- }
return *this;
}
Note that probably the whole operator=() can go.
It already did. Look in the svn logs by me and Abdel.
Stefan
PGP.sig
Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
On Sat, Jun 09, 2007 at 12:49:09PM +0200, Stefan Schimanski wrote:
> It works fine (as far as I can judge after 2 minutes testing). I
> added some comments and pulled apart the loop to make the control
> flow easier. Alfredo, can you check please? I would be happy if we
> could get rid of the signals finally like this.
>
> Stefan
>
>
> Index: lyx-devel/src/CursorSlice.cpp
> ===
> --- lyx-devel.orig/src/CursorSlice.cpp2007-06-02 11:24:26.0
> +0200
> +++ lyx-devel/src/CursorSlice.cpp 2007-06-09 12:11:13.0 +0200
> @@ -42,10 +42,6 @@
> : inset_(), idx_(0), pit_(0), pos_(0)
> {
> BOOST_ASSERT(inset_);
> - boost::signal * destroyed_signal = inset_->destroyedSignal();
> - if (destroyed_signal)
> - inset_connection_ = destroyed_signal->connect(
> - boost::bind(::invalidate, this));
> }
> @@ -55,22 +51,12 @@
> }
> -CursorSlice::~CursorSlice()
> -{
> - inset_connection_.disconnect();
> -}
> -
> -
> CursorSlice & CursorSlice::operator=(CursorSlice const & cs)
> {
> inset_ = cs.inset_;
> idx_ = cs.idx_;
> pit_ = cs.pit_;
> pos_ = cs.pos_;
> - if (inset_ && inset_->destroyedSignal()) {
> - inset_connection_ = inset_->destroyedSignal()->connect(
> - boost::bind(::invalidate, this));
> - }
> return *this;
> }
Note that probably the whole operator=() can go.
Andre'
Re: Road to rc2 - fixIfBroken
CursorSlice & CursorSlice::operator=(CursorSlice const & cs)
{
inset_ = cs.inset_;
idx_ = cs.idx_;
pit_ = cs.pit_;
pos_ = cs.pos_;
- if (inset_ && inset_->destroyedSignal()) {
- inset_connection_ = inset_->destroyedSignal()->connect(
- boost::bind(::invalidate, this));
- }
return *this;
}
Note that probably the whole operator=() can go.
It already did. Look in the svn logs by me and Abdel.
Stefan
PGP.sig
Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
Am 09.06.2007 um 00:28 schrieb Alfredo Braunstein: Jean-Marc Lasgouttes wrote: Yes, the patch looks good, except that the messages are not very informative (but as a usr I would be scared to see all these messages in normal operation). And there is a very long line. Fixed. Note that the scary messages are already there in svn. Please apply. A/ PS: should I prepare another patch with the removal of the destroyed signals etc? fixifbroken2.diff Some small questions: Why don't you like comments? Why do you need this complicated logic to set the inset to 0 in many cases. Won't that end the loop anyway in the next round? And, if the inset = 0 it's a broken cursor in any way, no? So take a wrong idx, hence inset=0. In the next loop with inset()==inset will not cut if off. I think it's wrong. Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote:
Some small questions:
Why don't you like comments?
? Be more specific. OTOH, I would have like some comment of yours when I
asked for them a week ago... ;-)
Why do you need this complicated logic to set the inset to 0 in many
cases. Won't that end the loop anyway in the next round?
Yes, but cutting off the DocIterator.
And, if the inset = 0 it's a broken cursor in any way, no? So take a
wrong idx, hence inset=0. In the next loop with inset()==inset will
not cut if off. I think it's wrong.
Oh yes I forgot (too much time passed). This patch is intended to work
*without* the signaling mechanism. So we should have no slice with inset ==
0. So if the patch is to be applied without removing/deactivating the
signaling mechanism, this slightly different version should do it, agreed?
A/
Index: DocIterator.cpp
===
--- DocIterator.cpp (revision 18720)
+++ DocIterator.cpp (working copy)
@@ -562,50 +562,48 @@
{
bool fixed = false;
- for (size_t i = slices_.size() - 1; i != 0; --i)
- if (!slices_[i].isValid()) {
- pop_back();
+ Inset * inset = slices_[0].inset();
+ for (size_t i = 0, n = slices_.size(); i != n; ++i) {
+ CursorSlice cs = slices_[i];
+ if (cs.inset() != inset || inset == 0) {
+ LYXERR(Debug::DEBUG)
+ fixIfBroken(): cursor chopped at
+ i because cs.inset()
+ != inset endl;
+ resize(i);
+ return true;
+ } else if (cs.idx() cs.lastidx()) {
+ cs.idx() = cs.lastidx();
+ cs.pit() = cs.lastpit();
+ cs.pos() = cs.lastpos();
+ inset = 0;
fixed = true;
+ LYXERR(Debug::DEBUG)
+ fixIfBroken(): idx fixed endl;
+ } else if (cs.pit() cs.lastpit()) {
+ cs.pit() = cs.lastpit();
+ cs.pos() = cs.lastpos();
+ inset = 0;
+ fixed = true;
+ LYXERR(Debug::DEBUG)
+ fixIfBroken(): pit fixed endl;
+ } else if (cs.pos() cs.lastpos()) {
+ cs.pos() = cs.lastpos();
+ inset = 0;
+ fixed = true;
+ LYXERR(Debug::DEBUG)
+ fixIfBroken(): pos fixed endl;
+ } else if (i != n - 1 cs.pos() != cs.lastpos()) {
+ if (cs.inset().inMathed()) {
+inset = (cs.cell().begin()
+ + cs.pos())-nucleus();
+ } else {
+inset = cs.paragraph().isInset(cs.pos()) ?
+ cs.paragraph().getInset(cs.pos()) : 0;
+ }
}
-
- // The top level CursorSlice should always be valid.
- BOOST_ASSERT(slices_[0].isValid());
-
- if (idx() lastidx()) {
- lyxerr wrong idx idx()
- , max is lastidx()
- at level depth()
- . Trying to correct this. endl;
- lyxerr old: *this endl;
- for (size_t i = idx(); i != lastidx(); --i)
- pop_back();
- idx() = lastidx();
- pit() = lastpit();
- pos() = lastpos();
- fixed = true;
}
- else if (pit() lastpit()) {
- lyxerr wrong pit pit()
- , max is lastpit()
- at level depth()
- . Trying to correct this. endl;
- lyxerr old: *this endl;
- pit() = lastpit();
- pos() = 0;
- fixed = true;
- }
- else if (pos() lastpos()) {
- lyxerr wrong pos pos()
- , max is lastpos()
- at level depth()
- . Trying to correct this. endl;
- lyxerr old: *this endl;
- pos() = lastpos();
- fixed = true;
- }
- if (fixed) {
- lyxerr new: *this endl;
- }
+
return fixed;
}
Index: CursorSlice.h
===
--- CursorSlice.h (revision 18720)
+++ CursorSlice.h (working copy)
@@ -81,6 +81,8 @@
pit_type pit() const { return pit_; }
/// set the offset of the paragraph this cursor is in
pit_type pit() { return pit_; }
+ /// return the last paragraph offset this cursor is in
+ pit_type lastpit() const;
/// increments the paragraph this cursor is in
void incrementPar();
/// decrements the paragraph this cursor is in
Index: CursorSlice.cpp
===
--- CursorSlice.cpp (revision 18720)
+++ CursorSlice.cpp (working copy)
@@ -112,6 +112,14 @@
}
+pit_type CursorSlice::lastpit() const
+{
+ if (inset().inMathed())
+ return 0;
+ return text()-paragraphs().size() - 1;
+}
+
+
CursorSlice::row_type CursorSlice::row() const
{
BOOST_ASSERT(asInsetMath());
Re: Road to rc2 - fixIfBroken
Some small questions: Why don't you like comments? ? Be more specific. OTOH, I would have like some comment of yours when I asked for them a week ago... ;-) Sorry, meant something like two lines describing what the big loop is doing. Not the comments here :) Why do you need this complicated logic to set the inset to 0 in many cases. Won't that end the loop anyway in the next round? Yes, but cutting off the DocIterator. And, if the inset = 0 it's a broken cursor in any way, no? So take a wrong idx, hence inset=0. In the next loop with inset()==inset will not cut if off. I think it's wrong. Oh yes I forgot (too much time passed). This patch is intended to work *without* the signaling mechanism. So we should have no slice with inset == 0. So if the patch is to be applied without removing/deactivating the signaling mechanism, this slightly different version should do it, agreed? I guess yes. Compiling right now. If it does, that would be great. Signals in those CursorSlices, always feel in a strange way when thinking about it :) Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: I guess yes. Compiling right now. If it does, that would be great. Signals in those CursorSlices, always feel in a strange way when thinking about it :) Since you are at it, could you please just commit if you think it is correct? Jürgen
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: Some small questions: Why don't you like comments? ? Be more specific. OTOH, I would have like some comment of yours when I asked for them a week ago... ;-) Sorry, meant something like two lines describing what the big loop is doing. Not the comments here :) Something like this? //At the end of this loop the DocIterator should be valid, we //ensure this by working out this condition from the bottom() //to the top() slices. I guess yes. Compiling right now. If it does, that would be great. Thanks Stefan. Signals in those CursorSlices, always feel in a strange way when thinking about it :) Yes, I don't like it either. I'm trying to code something to having edition-persistent iterators(*). I have a partial solution, that includes a MarkersList administrated by the InsetList inside every paragraph. (where markers are like insets, but they just don't take out editing space). The problem right now is what to do when the paragraph is removed completely; then we lose track completely and something else has to be considered. (*) this could have many uses: cursor in other window, bookmarks, error lists etc. A/
Re: Road to rc2 - fixIfBroken
It works fine (as far as I can judge after 2 minutes testing). I
added some comments and pulled apart the loop to make the control
flow easier. Alfredo, can you check please? I would be happy if we
could get rid of the signals finally like this.
Stefan
Index: lyx-devel/src/CursorSlice.cpp
===
--- lyx-devel.orig/src/CursorSlice.cpp 2007-06-02 11:24:26.0
+0200
+++ lyx-devel/src/CursorSlice.cpp 2007-06-09 12:11:13.0 +0200
@@ -42,10 +42,6 @@
: inset_(p), idx_(0), pit_(0), pos_(0)
{
BOOST_ASSERT(inset_);
- boost::signalvoid() * destroyed_signal = inset_-destroyedSignal();
- if (destroyed_signal)
- inset_connection_ = destroyed_signal-connect(
- boost::bind(CursorSlice::invalidate, this));
}
@@ -55,22 +51,12 @@
}
-CursorSlice::~CursorSlice()
-{
- inset_connection_.disconnect();
-}
-
-
CursorSlice CursorSlice::operator=(CursorSlice const cs)
{
inset_ = cs.inset_;
idx_ = cs.idx_;
pit_ = cs.pit_;
pos_ = cs.pos_;
- if (inset_ inset_-destroyedSignal()) {
- inset_connection_ = inset_-destroyedSignal()-connect(
- boost::bind(CursorSlice::invalidate, this));
- }
return *this;
}
@@ -112,6 +98,14 @@
}
+pit_type CursorSlice::lastpit() const
+{
+ if (inset().inMathed())
+ return 0;
+ return text()-paragraphs().size() - 1;
+}
+
+
CursorSlice::row_type CursorSlice::row() const
{
BOOST_ASSERT(asInsetMath());
Index: lyx-devel/src/CursorSlice.h
===
--- lyx-devel.orig/src/CursorSlice.h2007-06-02 11:24:26.0 +0200
+++ lyx-devel/src/CursorSlice.h 2007-06-09 12:12:17.0 +0200
@@ -41,7 +41,7 @@
// that of MathData and Text should vanish. They are conceptually the
// same (now...)
-class CursorSlice : public boost::signals::trackable {
+class CursorSlice {
public:
/// Those needs inset_ access.
///@{
@@ -63,8 +63,6 @@
///
explicit CursorSlice(Inset );
///
- virtual ~CursorSlice();
- ///
CursorSlice operator=(CursorSlice const );
///
bool isValid() const;
@@ -81,6 +79,8 @@
pit_type pit() const { return pit_; }
/// set the offset of the paragraph this cursor is in
pit_type pit() { return pit_; }
+ /// return the last paragraph offset this cursor is in
+ pit_type lastpit() const;
/// increments the paragraph this cursor is in
void incrementPar();
/// decrements the paragraph this cursor is in
@@ -158,8 +158,6 @@
bool pit_valid_;
/// position in this cell
pos_type pos_;
- /// connection to referred \c inset_ destruction signal.
- boost::signals::connection inset_connection_;
};
/// test for equality
Index: lyx-devel/src/DocIterator.cpp
===
--- lyx-devel.orig/src/DocIterator.cpp 2007-06-08 22:20:09.0
+0200
+++ lyx-devel/src/DocIterator.cpp 2007-06-09 12:44:07.0 +0200
@@ -4,6 +4,7 @@
* Licence details can be found in the file COPYING.
*
* \author André Pönitz
+ * \author Alfredo Braunstein
*
* Full author contact details are available in file CREDITS.
*/
@@ -560,53 +561,55 @@
bool DocIterator::fixIfBroken()
{
- bool fixed = false;
-
- for (size_t i = slices_.size() - 1; i != 0; --i)
- if (!slices_[i].isValid()) {
- pop_back();
- fixed = true;
+ // Go through the slice stack from the bottom.
+ // Check that all coordinates (idx, pit, pos) are correct and
+ // that the inset is the one which is claimed to be there
+ Inset * inset = slices_[0].inset();
+ size_t i = 0;
+ size_t n = slices_.size();
+ for (; i != n; ++i) {
+ CursorSlice cs = slices_[i];
+ if (cs.inset() != inset) {
+ // the whole slice is wrong, chop off this as well
+ --i;
+ LYXERR(Debug::DEBUG) fixIfBroken(): inset changed
endl;
+ break;
+ } else if (cs.idx() cs.lastidx()) {
+ cs.idx() = cs.lastidx();
+ cs.pit() = cs.lastpit();
+ cs.pos() = cs.lastpos();
+ LYXERR(Debug::DEBUG) fixIfBroken(): idx fixed
endl;
+ break;
+ } else if (cs.pit() cs.lastpit()) {
+ cs.pit() = cs.lastpit();
+ cs.pos() = cs.lastpos();
+ LYXERR(Debug::DEBUG) fixIfBroken(): pit fixed
endl;
+ break;
+ } else if (cs.pos() cs.lastpos()) {
+ cs.pos() = cs.lastpos();
+
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: It works fine (as far as I can judge after 2 minutes testing). I added some comments and pulled apart the loop to make the control flow easier. Alfredo, can you check please? I would be happy if we could get rid of the signals finally like this. Sure, but I cannot right now. Give me a couple of hours. A/
Re: Road to rc2 - fixIfBroken
Am 09.06.2007 um 13:09 schrieb Alfredo Braunstein: Stefan Schimanski wrote: It works fine (as far as I can judge after 2 minutes testing). I added some comments and pulled apart the loop to make the control flow easier. Alfredo, can you check please? I would be happy if we could get rid of the signals finally like this. Sure, but I cannot right now. Give me a couple of hours. Ok. Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: It works fine (as far as I can judge after 2 minutes testing). I added some comments and pulled apart the loop to make the control flow easier. Alfredo, can you check please? I would be happy if we could get rid of the signals finally like this. I am not Alfredo but it looks fine. Abdel.
Re: Road to rc2 - fixIfBroken
Abdelrazak Younes wrote: Stefan Schimanski wrote: It works fine (as far as I can judge after 2 minutes testing). I added some comments and pulled apart the loop to make the control flow easier. Alfredo, can you check please? I would be happy if we could get rid of the signals finally like this. I am not Alfredo but it looks fine. I'd say shove it in. ;-) A/
Re: Road to rc2 - fixIfBroken
Ok, committed. So let's see if everything is alright now. We still have some days to the RC2 for testing. Stefan Am 09.06.2007 um 14:32 schrieb Alfredo Braunstein: Abdelrazak Younes wrote: Stefan Schimanski wrote: It works fine (as far as I can judge after 2 minutes testing). I added some comments and pulled apart the loop to make the control flow easier. Alfredo, can you check please? I would be happy if we could get rid of the signals finally like this. I am not Alfredo but it looks fine. I'd say shove it in. ;-) A/ PGP.sig Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: Ok, committed. So let's see if everything is alright now. We still have some days to the RC2 for testing. If the testing reveals that we don't need the Inset::destroyed() signal, this should be deleted before RC2 too. Abdel.
Re: Road to rc2 - fixIfBroken
Am 09.06.2007 um 14:46 schrieb Abdelrazak Younes: Stefan Schimanski wrote: Ok, committed. So let's see if everything is alright now. We still have some days to the RC2 for testing. If the testing reveals that we don't need the Inset::destroyed() signal, this should be deleted before RC2 too. Right. But then we can also do it now and add it again if we need it? Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: Am 09.06.2007 um 14:46 schrieb Abdelrazak Younes: Stefan Schimanski wrote: Ok, committed. So let's see if everything is alright now. We still have some days to the RC2 for testing. If the testing reveals that we don't need the Inset::destroyed() signal, this should be deleted before RC2 too. Right. But then we can also do it now and add it again if we need it? Yep, as you feel. Abdel.
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: Am 09.06.2007 um 14:46 schrieb Abdelrazak Younes: Stefan Schimanski wrote: Ok, committed. So let's see if everything is alright now. We still have some days to the RC2 for testing. If the testing reveals that we don't need the Inset::destroyed() signal, this should be deleted before RC2 too. Right. But then we can also do it now and add it again if we need it? I'll do it. Abdel
Re: Road to rc2 - fixIfBroken
Am 09.06.2007 um 15:05 schrieb Abdelrazak Younes: Stefan Schimanski wrote: Am 09.06.2007 um 14:46 schrieb Abdelrazak Younes: Stefan Schimanski wrote: Ok, committed. So let's see if everything is alright now. We still have some days to the RC2 for testing. If the testing reveals that we don't need the Inset::destroyed() signal, this should be deleted before RC2 too. Right. But then we can also do it now and add it again if we need it? Nice, now it's even usable again with stdlib-debug enabled. Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
Am 09.06.2007 um 00:28 schrieb Alfredo Braunstein: Jean-Marc Lasgouttes wrote: Yes, the patch looks good, except that the messages are not very informative (but as a usr I would be scared to see all these messages in normal operation). And there is a very long line. Fixed. Note that the scary messages are already there in svn. Please apply. A/ PS: should I prepare another patch with the removal of the destroyed signals etc? Some small questions: Why don't you like comments? Why do you need this complicated logic to set the inset to 0 in many cases. Won't that end the loop anyway in the next round? And, if the inset = 0 it's a broken cursor in any way, no? So take a wrong idx, hence inset=0. In the next loop with inset()==inset will not cut if off. I think it's wrong. Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote:
> Some small questions:
> Why don't you like comments?
? Be more specific. OTOH, I would have like some comment of yours when I
asked for them a week ago... ;-)
> Why do you need this complicated logic to set the inset to 0 in many
> cases. Won't that end the loop anyway in the next round?
Yes, but cutting off the DocIterator.
> And, if the inset = 0 it's a broken cursor in any way, no? So take a
> wrong idx, hence inset=0. In the next loop with inset()==inset will
> not cut if off. I think it's wrong.
Oh yes I forgot (too much time passed). This patch is intended to work
*without* the signaling mechanism. So we should have no slice with inset ==
0. So if the patch is to be applied without removing/deactivating the
signaling mechanism, this slightly different version should do it, agreed?
A/
Index: DocIterator.cpp
===
--- DocIterator.cpp (revision 18720)
+++ DocIterator.cpp (working copy)
@@ -562,50 +562,48 @@
{
bool fixed = false;
- for (size_t i = slices_.size() - 1; i != 0; --i)
- if (!slices_[i].isValid()) {
- pop_back();
+ Inset * inset = _[0].inset();
+ for (size_t i = 0, n = slices_.size(); i != n; ++i) {
+ CursorSlice & cs = slices_[i];
+ if (() != inset || inset == 0) {
+ LYXERR(Debug::DEBUG)
+<< "fixIfBroken(): cursor chopped at "
+<< i << " because " << ()
+<< " != " << inset << endl;
+ resize(i);
+ return true;
+ } else if (cs.idx() > cs.lastidx()) {
+ cs.idx() = cs.lastidx();
+ cs.pit() = cs.lastpit();
+ cs.pos() = cs.lastpos();
+ inset = 0;
fixed = true;
+ LYXERR(Debug::DEBUG)
+<< "fixIfBroken(): idx fixed" << endl;
+ } else if (cs.pit() > cs.lastpit()) {
+ cs.pit() = cs.lastpit();
+ cs.pos() = cs.lastpos();
+ inset = 0;
+ fixed = true;
+ LYXERR(Debug::DEBUG)
+<< "fixIfBroken(): pit fixed" << endl;
+ } else if (cs.pos() > cs.lastpos()) {
+ cs.pos() = cs.lastpos();
+ inset = 0;
+ fixed = true;
+ LYXERR(Debug::DEBUG)
+<< "fixIfBroken(): pos fixed" << endl;
+ } else if (i != n - 1 && cs.pos() != cs.lastpos()) {
+ if (cs.inset().inMathed()) {
+inset = (cs.cell().begin()
+ + cs.pos())->nucleus();
+ } else {
+inset = cs.paragraph().isInset(cs.pos()) ?
+ cs.paragraph().getInset(cs.pos()) : 0;
+ }
}
-
- // The top level CursorSlice should always be valid.
- BOOST_ASSERT(slices_[0].isValid());
-
- if (idx() > lastidx()) {
- lyxerr << "wrong idx " << idx()
- << ", max is " << lastidx()
- << " at level " << depth()
- << ". Trying to correct this." << endl;
- lyxerr << "old: " << *this << endl;
- for (size_t i = idx(); i != lastidx(); --i)
- pop_back();
- idx() = lastidx();
- pit() = lastpit();
- pos() = lastpos();
- fixed = true;
}
- else if (pit() > lastpit()) {
- lyxerr << "wrong pit " << pit()
- << ", max is " << lastpit()
- << " at level " << depth()
- << ". Trying to correct this." << endl;
- lyxerr << "old: " << *this << endl;
- pit() = lastpit();
- pos() = 0;
- fixed = true;
- }
- else if (pos() > lastpos()) {
- lyxerr << "wrong pos " << pos()
- << ", max is " << lastpos()
- << " at level " << depth()
- << ". Trying to correct this." << endl;
- lyxerr << "old: " << *this << endl;
- pos() = lastpos();
- fixed = true;
- }
- if (fixed) {
- lyxerr << "new: " << *this << endl;
- }
+
return fixed;
}
Index: CursorSlice.h
===
--- CursorSlice.h (revision 18720)
+++ CursorSlice.h (working copy)
@@ -81,6 +81,8 @@
pit_type pit() const { return pit_; }
/// set the offset of the paragraph this cursor is in
pit_type & pit() { return pit_; }
+ /// return the last paragraph offset this cursor is in
+ pit_type lastpit() const;
/// increments the paragraph this cursor is in
void incrementPar();
/// decrements the paragraph this cursor is in
Index: CursorSlice.cpp
===
--- CursorSlice.cpp (revision 18720)
+++ CursorSlice.cpp (working copy)
@@ -112,6 +112,14 @@
}
+pit_type CursorSlice::lastpit() const
+{
+ if (inset().inMathed())
+ return 0;
+ return text()->paragraphs().size() - 1;
+}
+
+
CursorSlice::row_type CursorSlice::row() const
{
BOOST_ASSERT(asInsetMath());
Re: Road to rc2 - fixIfBroken
Some small questions: Why don't you like comments? ? Be more specific. OTOH, I would have like some comment of yours when I asked for them a week ago... ;-) Sorry, meant something like two lines describing what the big loop is doing. Not the comments here :) Why do you need this complicated logic to set the inset to 0 in many cases. Won't that end the loop anyway in the next round? Yes, but cutting off the DocIterator. And, if the inset = 0 it's a broken cursor in any way, no? So take a wrong idx, hence inset=0. In the next loop with inset()==inset will not cut if off. I think it's wrong. Oh yes I forgot (too much time passed). This patch is intended to work *without* the signaling mechanism. So we should have no slice with inset == 0. So if the patch is to be applied without removing/deactivating the signaling mechanism, this slightly different version should do it, agreed? I guess yes. Compiling right now. If it does, that would be great. Signals in those CursorSlices, always feel in a strange way when thinking about it :) Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: > I guess yes. Compiling right now. If it does, that would be great. > Signals in those CursorSlices, always feel in a strange way when > thinking about it :) Since you are at it, could you please just commit if you think it is correct? Jürgen
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: >>> Some small questions: >>> Why don't you like comments? >> >> ? Be more specific. OTOH, I would have like some comment of yours >> when I >> asked for them a week ago... ;-) > > Sorry, meant something like two lines describing what the big loop is > doing. Not the comments here :) Something like this? //At the end of this loop the DocIterator should be valid, we //ensure this by working out this condition from the bottom() //to the top() slices. > I guess yes. Compiling right now. If it does, that would be great. Thanks Stefan. > Signals in those CursorSlices, always feel in a strange way when > thinking about it :) Yes, I don't like it either. I'm trying to code something to having edition-persistent iterators(*). I have a partial solution, that includes a MarkersList administrated by the InsetList inside every paragraph. (where markers are like insets, but they just don't take out editing space). The problem right now is what to do when the paragraph is removed completely; then we lose track completely and something else has to be considered. (*) this could have many uses: cursor in other window, bookmarks, error lists etc. A/
Re: Road to rc2 - fixIfBroken
It works fine (as far as I can judge after 2 minutes testing). I
added some comments and pulled apart the loop to make the control
flow easier. Alfredo, can you check please? I would be happy if we
could get rid of the signals finally like this.
Stefan
Index: lyx-devel/src/CursorSlice.cpp
===
--- lyx-devel.orig/src/CursorSlice.cpp 2007-06-02 11:24:26.0
+0200
+++ lyx-devel/src/CursorSlice.cpp 2007-06-09 12:11:13.0 +0200
@@ -42,10 +42,6 @@
: inset_(), idx_(0), pit_(0), pos_(0)
{
BOOST_ASSERT(inset_);
- boost::signal * destroyed_signal = inset_->destroyedSignal();
- if (destroyed_signal)
- inset_connection_ = destroyed_signal->connect(
- boost::bind(::invalidate, this));
}
@@ -55,22 +51,12 @@
}
-CursorSlice::~CursorSlice()
-{
- inset_connection_.disconnect();
-}
-
-
CursorSlice & CursorSlice::operator=(CursorSlice const & cs)
{
inset_ = cs.inset_;
idx_ = cs.idx_;
pit_ = cs.pit_;
pos_ = cs.pos_;
- if (inset_ && inset_->destroyedSignal()) {
- inset_connection_ = inset_->destroyedSignal()->connect(
- boost::bind(::invalidate, this));
- }
return *this;
}
@@ -112,6 +98,14 @@
}
+pit_type CursorSlice::lastpit() const
+{
+ if (inset().inMathed())
+ return 0;
+ return text()->paragraphs().size() - 1;
+}
+
+
CursorSlice::row_type CursorSlice::row() const
{
BOOST_ASSERT(asInsetMath());
Index: lyx-devel/src/CursorSlice.h
===
--- lyx-devel.orig/src/CursorSlice.h2007-06-02 11:24:26.0 +0200
+++ lyx-devel/src/CursorSlice.h 2007-06-09 12:12:17.0 +0200
@@ -41,7 +41,7 @@
// that of MathData and Text should vanish. They are conceptually the
// same (now...)
-class CursorSlice : public boost::signals::trackable {
+class CursorSlice {
public:
/// Those needs inset_ access.
///@{
@@ -63,8 +63,6 @@
///
explicit CursorSlice(Inset &);
///
- virtual ~CursorSlice();
- ///
CursorSlice & operator=(CursorSlice const &);
///
bool isValid() const;
@@ -81,6 +79,8 @@
pit_type pit() const { return pit_; }
/// set the offset of the paragraph this cursor is in
pit_type & pit() { return pit_; }
+ /// return the last paragraph offset this cursor is in
+ pit_type lastpit() const;
/// increments the paragraph this cursor is in
void incrementPar();
/// decrements the paragraph this cursor is in
@@ -158,8 +158,6 @@
bool pit_valid_;
/// position in this cell
pos_type pos_;
- /// connection to referred \c inset_ destruction signal.
- boost::signals::connection inset_connection_;
};
/// test for equality
Index: lyx-devel/src/DocIterator.cpp
===
--- lyx-devel.orig/src/DocIterator.cpp 2007-06-08 22:20:09.0
+0200
+++ lyx-devel/src/DocIterator.cpp 2007-06-09 12:44:07.0 +0200
@@ -4,6 +4,7 @@
* Licence details can be found in the file COPYING.
*
* \author André Pönitz
+ * \author Alfredo Braunstein
*
* Full author contact details are available in file CREDITS.
*/
@@ -560,53 +561,55 @@
bool DocIterator::fixIfBroken()
{
- bool fixed = false;
-
- for (size_t i = slices_.size() - 1; i != 0; --i)
- if (!slices_[i].isValid()) {
- pop_back();
- fixed = true;
+ // Go through the slice stack from the bottom.
+ // Check that all coordinates (idx, pit, pos) are correct and
+ // that the inset is the one which is claimed to be there
+ Inset * inset = _[0].inset();
+ size_t i = 0;
+ size_t n = slices_.size();
+ for (; i != n; ++i) {
+ CursorSlice & cs = slices_[i];
+ if (() != inset) {
+ // the whole slice is wrong, chop off this as well
+ --i;
+ LYXERR(Debug::DEBUG) << "fixIfBroken(): inset changed"
<< endl;
+ break;
+ } else if (cs.idx() > cs.lastidx()) {
+ cs.idx() = cs.lastidx();
+ cs.pit() = cs.lastpit();
+ cs.pos() = cs.lastpos();
+ LYXERR(Debug::DEBUG) << "fixIfBroken(): idx fixed" <<
endl;
+ break;
+ } else if (cs.pit() > cs.lastpit()) {
+ cs.pit() = cs.lastpit();
+ cs.pos() = cs.lastpos();
+ LYXERR(Debug::DEBUG) << "fixIfBroken(): pit fixed" <<
endl;
+ break;
+ } else if (cs.pos() > cs.lastpos()) {
+ cs.pos() = cs.lastpos();
+
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: > It works fine (as far as I can judge after 2 minutes testing). I > added some comments and pulled apart the loop to make the control > flow easier. Alfredo, can you check please? I would be happy if we > could get rid of the signals finally like this. Sure, but I cannot right now. Give me a couple of hours. A/
Re: Road to rc2 - fixIfBroken
Am 09.06.2007 um 13:09 schrieb Alfredo Braunstein: Stefan Schimanski wrote: It works fine (as far as I can judge after 2 minutes testing). I added some comments and pulled apart the loop to make the control flow easier. Alfredo, can you check please? I would be happy if we could get rid of the signals finally like this. Sure, but I cannot right now. Give me a couple of hours. Ok. Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: It works fine (as far as I can judge after 2 minutes testing). I added some comments and pulled apart the loop to make the control flow easier. Alfredo, can you check please? I would be happy if we could get rid of the signals finally like this. I am not Alfredo but it looks fine. Abdel.
Re: Road to rc2 - fixIfBroken
Abdelrazak Younes wrote: > Stefan Schimanski wrote: >> It works fine (as far as I can judge after 2 minutes testing). I added >> some comments and pulled apart the loop to make the control flow easier. >> Alfredo, can you check please? I would be happy if we could get rid of >> the signals finally like this. > > I am not Alfredo but it looks fine. I'd say shove it in. ;-) A/
Re: Road to rc2 - fixIfBroken
Ok, committed. So let's see if everything is alright now. We still have some days to the RC2 for testing. Stefan Am 09.06.2007 um 14:32 schrieb Alfredo Braunstein: Abdelrazak Younes wrote: Stefan Schimanski wrote: It works fine (as far as I can judge after 2 minutes testing). I added some comments and pulled apart the loop to make the control flow easier. Alfredo, can you check please? I would be happy if we could get rid of the signals finally like this. I am not Alfredo but it looks fine. I'd say shove it in. ;-) A/ PGP.sig Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: Ok, committed. So let's see if everything is alright now. We still have some days to the RC2 for testing. If the testing reveals that we don't need the Inset::destroyed() signal, this should be deleted before RC2 too. Abdel.
Re: Road to rc2 - fixIfBroken
Am 09.06.2007 um 14:46 schrieb Abdelrazak Younes: Stefan Schimanski wrote: Ok, committed. So let's see if everything is alright now. We still have some days to the RC2 for testing. If the testing reveals that we don't need the Inset::destroyed() signal, this should be deleted before RC2 too. Right. But then we can also do it now and add it again if we need it? Stefan PGP.sig Description: Signierter Teil der Nachricht
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: Am 09.06.2007 um 14:46 schrieb Abdelrazak Younes: Stefan Schimanski wrote: Ok, committed. So let's see if everything is alright now. We still have some days to the RC2 for testing. If the testing reveals that we don't need the Inset::destroyed() signal, this should be deleted before RC2 too. Right. But then we can also do it now and add it again if we need it? Yep, as you feel. Abdel.
Re: Road to rc2 - fixIfBroken
Stefan Schimanski wrote: Am 09.06.2007 um 14:46 schrieb Abdelrazak Younes: Stefan Schimanski wrote: Ok, committed. So let's see if everything is alright now. We still have some days to the RC2 for testing. If the testing reveals that we don't need the Inset::destroyed() signal, this should be deleted before RC2 too. Right. But then we can also do it now and add it again if we need it? I'll do it. Abdel
Re: Road to rc2 - fixIfBroken
Am 09.06.2007 um 15:05 schrieb Abdelrazak Younes: Stefan Schimanski wrote: Am 09.06.2007 um 14:46 schrieb Abdelrazak Younes: Stefan Schimanski wrote: Ok, committed. So let's see if everything is alright now. We still have some days to the RC2 for testing. If the testing reveals that we don't need the Inset::destroyed() signal, this should be deleted before RC2 too. Right. But then we can also do it now and add it again if we need it? Nice, now it's even usable again with stdlib-debug enabled. Stefan PGP.sig Description: Signierter Teil der Nachricht
