Hello there,

I am going to work a bit on the bookmark handling if you don't mind (see patch). The goal is to extend it to support in-inset bookmarks.

Bo, is that OK for you?

There is by the way a bug in 1.5.x with bookmarks: the current font is not set. Putting this line at the end of BufferView::moveToPosition() should correct it:

        buffer_.text().setCurrentFont(cursor_);

Jose, do you want me to proceed?

Abdel.




Index: BufferView.cpp
===================================================================
--- BufferView.cpp      (revision 19719)
+++ BufferView.cpp      (working copy)
@@ -408,9 +408,12 @@
 }
 
 
-boost::tuple<pit_type, pos_type, int> BufferView::moveToPosition(pit_type 
bottom_pit, pos_type bottom_pos,
+bool BufferView::moveToPosition(pit_type bottom_pit, pos_type bottom_pos,
        int top_id, pos_type top_pos)
 {
+       bool success = false;
+       DocIterator doc_it;
+
        cursor_.clearSelection();
 
        // if a valid par_id is given, try it first
@@ -419,24 +422,23 @@
        if (top_id > 0) {
                ParIterator par = buffer_.getParFromID(top_id);
                if (par != buffer_.par_iterator_end()) {
-                       DocIterator dit = makeDocIterator(par, min(par->size(), 
top_pos));
+                       doc_it = makeDocIterator(par, min(par->size(), 
top_pos));
                        // Some slices of the iterator may not be
                        // reachable (e.g. closed collapsable inset)
                        // so the dociterator may need to be
                        // shortened. Otherwise, setCursor may crash
                        // lyx when the cursor can not be set to these
                        // insets.
-                       size_t const n = dit.depth();
+                       size_t const n = doc_it.depth();
                        for (size_t i = 0; i < n; ++i)
-                               if (dit[i].inset().editable() != 
Inset::HIGHLY_EDITABLE) {
-                                       dit.resize(i);
+                               if (doc_it[i].inset().editable() != 
Inset::HIGHLY_EDITABLE) {
+                                       doc_it.resize(i);
                                        break;
                                }
-                       setCursor(dit);
-                       // Note: return bottom (document) level pit.
-                       return boost::make_tuple(cursor_.bottom().pit(), 
cursor_.bottom().pos(), top_id);
                }
+               success = true;
        }
+
        // if top_id == 0, or searching through top_id failed
        // This is the case for a 'restored' bookmark when only bottom
        // (document level) pit was saved. Because of this, bookmark
@@ -444,15 +446,22 @@
        // it will be restored to the left of the outmost inset that contains
        // the bookmark.
        if (static_cast<size_t>(bottom_pit) < buffer_.paragraphs().size()) {
-               DocIterator it = doc_iterator_begin(buffer_.inset());
-               it.pit() = bottom_pit;
-               it.pos() = min(bottom_pos, it.paragraph().size());
-               setCursor(it);
-               return boost::make_tuple(it.pit(), it.pos(),
-                                        it.paragraph().id());
+               doc_it = doc_iterator_begin(buffer_.inset());
+               doc_it.pit() = bottom_pit;
+               doc_it.pos() = min(bottom_pos, doc_it.paragraph().size());
+               success = true;
        }
-       // both methods fail
-       return boost::make_tuple(pit_type(0), pos_type(0), 0);
+
+       if (success) {
+               // Note: only bottom (document) level pit is set.
+               setCursor(doc_it);
+               // set the current font.
+               buffer_.text().setCurrentFont(cursor_);
+               // center the screen on this new position.
+               center();
+       }
+
+       return success;
 }
 
 
Index: BufferView.h
===================================================================
--- BufferView.h        (revision 19691)
+++ BufferView.h        (working copy)
@@ -107,9 +107,9 @@
        /// Save the current position as bookmark.
        /// if idx == 0, save to temp_bookmark
        void saveBookmark(unsigned int idx);
-       /// goto a specified position, try top_id first, and then bottom_pit
-       /// return the bottom_pit and top_id of the new paragraph
-       boost::tuple<pit_type, pos_type, int> moveToPosition(
+       /// goto a specified position, try top_id first, and then bottom_pit.
+       /// \return true if success
+       bool moveToPosition(
                pit_type bottom_pit, ///< Paragraph pit, used when par_id is 
zero or invalid.
                pos_type bottom_pos, ///< Paragraph pit, used when par_id is 
zero or invalid.
                int top_id, ///< Paragraph ID, \sa Paragraph
Index: frontends/LyXView.cpp
===================================================================
--- frontends/LyXView.cpp       (revision 19718)
+++ frontends/LyXView.cpp       (working copy)
@@ -151,12 +151,7 @@
                boost::tie(pit, pos) = 
LyX::ref().session().lastFilePos().load(filename);
                // if successfully move to pit (returned par_id is not zero),
                // update metrics and reset font
-               BufferView & bv = wa->bufferView();
-               if (bv.moveToPosition(pit, pos, 0, 0).get<1>()) {
-                       if (bv.fitCursor())
-                               bv.updateMetrics(false);
-                       newBuffer->text().setCurrentFont(bv.cursor());
-               }
+               wa->bufferView().moveToPosition(pit, pos, 0, 0);
        }
 
        if (tolastfiles)
Index: LyXFunc.cpp
===================================================================
--- LyXFunc.cpp (revision 19691)
+++ LyXFunc.cpp (working copy)
@@ -273,11 +273,17 @@
                        else
                                return;
                }
-               // moveToPosition use par_id, and par_pit and return new par_id.
-               pit_type new_pit;
-               pos_type new_pos;
-               int new_id;
-               boost::tie(new_pit, new_pos, new_id) = 
view()->moveToPosition(bm.bottom_pit, bm.bottom_pos, bm.top_id, bm.top_pos);
+               // moveToPosition try paragraph id first and then paragraph 
(pit, pos).
+               if (!view()->moveToPosition(bm.bottom_pit, bm.bottom_pos,
+                               bm.top_id, bm.top_pos))
+                       return;
+
+               // Cursor jump succeeded!
+               Cursor const & cur = view()->cursor();
+               pit_type new_pit = cur.pit();
+               pos_type new_pos = cur.pos();
+               int new_id = cur.paragraph().id();
+
                // if bottom_pit, bottom_pos or top_id has been changed, update 
bookmark
                // see http://bugzilla.lyx.org/show_bug.cgi?id=3092
                if (bm.bottom_pit != new_pit || bm.bottom_pos != new_pos || 
bm.top_id != new_id )

Reply via email to