Abdelrazak Younes wrote:
Jean-Marc Lasgouttes wrote:
"Abdelrazak" == Abdelrazak Younes
<[EMAIL PROTECTED]> writes:
Abdelrazak> Stefan Schimanski wrote:
The attached patch fixes that. It should fix also a number of
other situation where cursor may be invalidated. I had to add a
test in Cursor::fixIfBroken() for the case were we are not inside
an Inset but in the main Text.
Nice! Testing it for a few minutes now. Looks fine. Couldn't cause
a crash yet.
Abdelrazak> I need a second OK. JMarc?
The general idea is correct, but...
Originally, fixIfBroken got the cursor out of insets when coordinates
were different. Now you add a different test at level 1 the just
resets the values. It would be better to rewrite fixIfBroken to do at
each level
- go out of the inset if the value of cur.inset() is not correct wrt
previous level.
How can I do that? If I erase an inset in one view while the cursor was
in this very inset in the other view, how can I know that the inset the
CursorSlice is referring to has been deleted?
The only simple solution I can think of right now is to emit a signal in
the Inset destructor that we can connect to the CursorSlice. The
CursorSlice will then invalidate itself.
Here is what I came up with. In principle this should solve all problems
of invalid cursors. Unfortunately it doesn't compile... somebody has an
idea why?
Compiling...
MathMacroTemplate.cpp
D:\devel\lyx\trunk\boost\boost/signals/detail/signal_base.hpp(150) :
error C2248: 'boost::noncopyable_::noncopyable::operator =' : cannot
access private member declared in class 'boost::noncopyable_::noncopyable'
D:\devel\lyx\trunk\boost\boost/noncopyable.hpp(28) : see
declaration of 'boost::noncopyable_::noncopyable::operator ='
D:\devel\lyx\trunk\boost\boost/noncopyable.hpp(22) : see
declaration of 'boost::noncopyable_::noncopyable'
This diagnostic occurred in the compiler generated function
'boost::signals::detail::signal_base
&boost::signals::detail::signal_base::operator =(const
boost::signals::detail::signal_base &)'
InsetMathRef.cpp
D:\devel\lyx\trunk\boost\boost/signals/detail/signal_base.hpp(150) :
error C2248: 'boost::noncopyable_::noncopyable::operator =' : cannot
access private member declared in class 'boost::noncopyable_::noncopyable'
D:\devel\lyx\trunk\boost\boost/noncopyable.hpp(28) : see
declaration of 'boost::noncopyable_::noncopyable::operator ='
D:\devel\lyx\trunk\boost\boost/noncopyable.hpp(22) : see
declaration of 'boost::noncopyable_::noncopyable'
This diagnostic occurred in the compiler generated function
'boost::signals::detail::signal_base
&boost::signals::detail::signal_base::operator =(const
boost::signals::detail::signal_base &)'
InsetMathHull.cpp
D:\devel\lyx\trunk\boost\boost/signals/detail/signal_base.hpp(150) :
error C2248: 'boost::noncopyable_::noncopyable::operator =' : cannot
access private member declared in class 'boost::noncopyable_::noncopyable'
D:\devel\lyx\trunk\boost\boost/noncopyable.hpp(28) : see
declaration of 'boost::noncopyable_::noncopyable::operator ='
D:\devel\lyx\trunk\boost\boost/noncopyable.hpp(22) : see
declaration of 'boost::noncopyable_::noncopyable'
This diagnostic occurred in the compiler generated function
'boost::signals::detail::signal_base
&boost::signals::detail::signal_base::operator =(const
boost::signals::detail::signal_base &)'
Generating Code...
Abdel.
Index: BufferView.cpp
===================================================================
--- BufferView.cpp (revision 18473)
+++ BufferView.cpp (working copy)
@@ -215,6 +215,10 @@
cursor_.setSelection();
// do not set selection to the new buffer because we
// only paste recent selection.
+
+ // Make sure that the restored cursor is not broken.
This can happen for
+ // example if this Buffer has been modified by another
view.
+ cursor_.fixIfBroken();
}
}
Index: Cursor.cpp
===================================================================
--- Cursor.cpp (revision 18473)
+++ Cursor.cpp (working copy)
@@ -1299,44 +1299,10 @@
void Cursor::fixIfBroken()
{
- // find out last good level
- Cursor copy = *this;
- size_t newdepth = depth();
- while (!copy.empty()) {
- if (copy.idx() > copy.lastidx()) {
- lyxerr << "wrong idx " << copy.idx()
- << ", max is " << copy.lastidx()
- << " at level " << copy.depth()
- << ". Trying to correct this." << endl;
- lyxerr << "old: " << *this << endl;
- newdepth = copy.depth() - 1;
- }
- else if (copy.pit() > copy.lastpit()) {
- lyxerr << "wrong pit " << copy.pit()
- << ", max is " << copy.lastpit()
- << " at level " << copy.depth()
- << ". Trying to correct this." << endl;
- lyxerr << "old: " << *this << endl;
- newdepth = copy.depth() - 1;
- }
- else if (copy.pos() > copy.lastpos()) {
- lyxerr << "wrong pos " << copy.pos()
- << ", max is " << copy.lastpos()
- << " at level " << copy.depth()
- << ". Trying to correct this." << endl;
- lyxerr << "old: " << *this << endl;
- newdepth = copy.depth() - 1;
- }
- copy.pop();
+ if (DocIterator::fixIfBroken()) {
+ clearSelection();
+ resetAnchor();
}
- // shrink cursor to a size where everything is valid, possibly
- // leaving insets
- while (depth() > newdepth) {
- pop();
- lyxerr << "correcting cursor to level " << depth() << endl;
- lyxerr << "new: " << *this << endl;
- clearSelection();
- }
}
Index: CursorSlice.cpp
===================================================================
--- CursorSlice.cpp (revision 18473)
+++ CursorSlice.cpp (working copy)
@@ -38,9 +38,23 @@
: inset_(&p), idx_(0), pit_(0), pos_(0)
{
BOOST_ASSERT(inset_);
+ inset_->destroyed.connect(
+ boost::bind(&CursorSlice::invalidate, this));
}
+void CursorSlice::invalidate()
+{
+ inset_ = 0;
+}
+
+
+bool CursorSlice::isValid() const
+{
+ return inset_ != 0;
+}
+
+
MathData & CursorSlice::cell() const
{
return inset_->asInsetMath()->cell(idx_);
Index: CursorSlice.h
===================================================================
--- CursorSlice.h (revision 18473)
+++ CursorSlice.h (working copy)
@@ -20,6 +20,8 @@
#include "support/types.h"
#include "insets/Inset.h"
+#include <boost/signals/trackable.hpp>
+
#include <cstddef>
#include <iosfwd>
@@ -38,7 +40,7 @@
// that of MathData and Text should vanish. They are conceptually the
// same (now...)
-class CursorSlice {
+class CursorSlice : public boost::signals::trackable {
public:
/// type for cell number in inset
typedef size_t idx_type;
@@ -51,6 +53,8 @@
CursorSlice();
///
explicit CursorSlice(Inset &);
+ ///
+ bool isValid() const;
/// the current inset
Inset & inset() const { return *inset_; }
@@ -117,6 +121,9 @@
/// pointer to 'owning' inset. This is some kind of cache.
Inset * inset_;
private:
+ ///
+ void invalidate();
+
/*!
* Cell index of a position in this inset.
* This is the primary cell information also for grid like insets,
Index: DocIterator.cpp
===================================================================
--- DocIterator.cpp (revision 18473)
+++ DocIterator.cpp (working copy)
@@ -558,6 +558,58 @@
}
+bool DocIterator::fixIfBroken()
+{
+ bool fixed = false;
+
+ for (size_t i = slices_.size() - 1; i != 0; --i)
+ if (!slices_[i].isValid()) {
+ pop_back();
+ fixed = true;
+ }
+
+ // 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;
+}
+
+
std::ostream & operator<<(std::ostream & os, DocIterator const & dit)
{
for (size_t i = 0, n = dit.depth(); i != n; ++i)
Index: DocIterator.h
===================================================================
--- DocIterator.h (revision 18473)
+++ DocIterator.h (working copy)
@@ -227,6 +227,9 @@
void pop_back() { slices_.pop_back(); }
/// recompute the inset parts of the cursor from the document data
void updateInsets(Inset * inset);
+ /// fix DocIterator in circumstances that should never happen.
+ /// \return true if the DocIterator was fixed.
+ bool fixIfBroken();
private:
/**
Index: frontends/WorkArea.cpp
===================================================================
--- frontends/WorkArea.cpp (revision 18473)
+++ frontends/WorkArea.cpp (working copy)
@@ -140,9 +140,11 @@
// No need to do anything if this is the current view. The BufferView
// metrics are already up to date.
- if (&lyx_view_ != theApp()->currentView())
+ if (&lyx_view_ != theApp()->currentView()) {
// FIXME: it would be nice to optimize for the off-screen case.
buffer_view_->updateMetrics(false);
+ buffer_view_->cursor().fixIfBroken();
+ }
updateScrollbar();
Index: insets/Inset.h
===================================================================
--- insets/Inset.h (revision 18473)
+++ insets/Inset.h (working copy)
@@ -20,6 +20,8 @@
#include "support/docstream.h"
+#include <boost/signal.hpp>
+
#include <memory>
#include <vector>
@@ -70,7 +72,7 @@
typedef size_t col_type;
/// virtual base class destructor
- virtual ~Inset() {}
+ virtual ~Inset() { destroyed();}
/// replicate ourselves
std::auto_ptr<Inset> clone() const;
@@ -473,6 +475,9 @@
//
enum { TEXT_TO_INSET_OFFSET = 4 };
+ /// This signal is emitted when the inset is destroyed.
+ boost::signal<void()> destroyed;
+
protected:
Inset();
Inset(Inset const & i);
