Angus Leeming wrote:
Abdelrazak Younes <[EMAIL PROTECTED]> writes:
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'
You can't copy boost::signals. That means that either you're going to have to
define a copy constructor for the Inset holding the signal member or you need
to define a NonCopyableSignal wrapper to boost::signal that defines a no-op
copy constructor...
Here is the updated patch. Fully working, well tested.
OK?
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)
@@ -22,6 +22,7 @@
#include "mathed/MathData.h"
#include <boost/assert.hpp>
+#include <boost/bind.hpp>
namespace lyx {
@@ -38,9 +39,44 @@
: inset_(&p), idx_(0), pit_(0), pos_(0)
{
BOOST_ASSERT(inset_);
+ inset_->destroyed.connect(
+ boost::bind(&CursorSlice::invalidate, this));
}
+CursorSlice::CursorSlice(CursorSlice const & cs)
+{
+ operator=(cs);
+}
+
+
+CursorSlice & CursorSlice::operator=(CursorSlice const & cs)
+{
+ inset_ = cs.inset_;
+ idx_ = cs.idx_;
+ pit_ = cs.pit_;
+ pos_ = cs.pos_;
+ if (inset_) {
+ BOOST_ASSERT(inset_);
+ inset_->destroyed.connect(
+ boost::bind(&CursorSlice::invalidate, this));
+ }
+ return *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;
@@ -50,7 +52,13 @@
///
CursorSlice();
///
+ CursorSlice(CursorSlice const &);
+ ///
explicit CursorSlice(Inset &);
+ ///
+ CursorSlice & operator=(CursorSlice const &);
+ ///
+ bool isValid() const;
/// the current inset
Inset & inset() const { return *inset_; }
@@ -117,6 +125,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)
@@ -608,11 +660,12 @@
if (inset == 0) {
// FIXME
lyxerr << BOOST_CURRENT_FUNCTION
- << " Should not happen, but does e.g. after C-n
C-l C-z S-C-z"
+ << " Should not happen, but does e.g. after C-n
C-l C-z S-C-z\n"
+ << " or when a Buffer has been concurently
edited by two views"
<< '\n' << "dit: " << dit << '\n'
<< " lastpos: " << dit.lastpos() << endl;
- //break;
- BOOST_ASSERT(false);
+ dit.fixIfBroken();
+ return dit;
}
dit.push_back(data_[i]);
dit.top().inset_ = inset;
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.cpp
===================================================================
--- insets/Inset.cpp (revision 18473)
+++ insets/Inset.cpp (working copy)
@@ -126,6 +126,14 @@
{}
+Inset & Inset::operator=(Inset const & inset)
+{
+ dim_ = inset.dim_;
+ background_color_ = inset.background_color_;
+ return *this;
+}
+
+
std::auto_ptr<Inset> Inset::clone() const
{
std::auto_ptr<Inset> b = doClone();
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>
@@ -54,7 +56,7 @@
// everything storing large quantities of insets. Mathed e.g. would
// suffer.
-class Inset {
+class Inset : public boost::noncopyable {
public:
///
typedef ptrdiff_t difference_type;
@@ -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,9 +475,14 @@
//
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);
+ ///
+ Inset & operator=(Inset const &);
/** The real dispatcher.
* Gets normally called from Cursor::dispatch(). Cursor::dispatch()
* assumes the common case of 'LFUN handled, need update'.
