Fix bug 3092, use both pit and id to keep track of bookmarks.

	* src/session.h/C: save pit instead of id
	* src/lyxfunc.h/C: gotoBookmark function
	* src/BufferView.h/C: proper update of bookmarks

git-svn-id: svn://svn.lyx.org/lyx/lyx-devel/trunk@16647 a592a061-630c-0410-9148-cb99ea01b6c8

diff --git a/src/BufferView.C b/src/BufferView.C
index 47d222843c4cf393ba5a679e53c3a14c0db2771a..06bae06ca3259d96b1841eaf7d4be5ce9d6e13ef 100644 (file)
--- a/src/BufferView.C
+++ b/src/BufferView.C
@@ -264,22 +264,10 @@ bool BufferView::loadLyXFile(FileName const & filename, bool tolastfiles)
                pit_type pit;
                pos_type pos;
                boost::tie(pit, pos) = LyX::ref().session().lastFilePos().load(filename);
-               // I am not sure how to separate the following part to a function
-               // so I will leave this to Lars.
-               //
-               // check pit since the document may be externally changed.
-               if ( static_cast<size_t>(pit) < b->paragraphs().size() ) {
-                       ParIterator it = b->par_iterator_begin();
-                       ParIterator const end = b->par_iterator_end();
-                       for (; it != end; ++it)
-                               if (it.pit() == pit) {
-                                       // restored pos may be bigger than it->size
-                                       setCursor(makeDocIterator(it, min(pos, it->size())));
-                                       // No need to update the metrics if fitCursor returns false.
-                                       if (fitCursor())
-                                               updateMetrics(false);
-                                       break;
-                               }
+               // if successfully move to pit (returned par_id is not zero), update metrics
+               if (moveToPosition(pit, 0, pos).get<1>()) {
+                       if (fitCursor())
+                               updateMetrics(false);
                }
        }
 
@@ -536,6 +524,7 @@ void BufferView::saveBookmark(bool persistent)
 {
        LyX::ref().session().bookmarks().save(
                FileName(buffer_->fileName()),
+               cursor_.pit(),
                cursor_.paragraph().id(),
                cursor_.pos(),
                persistent
@@ -546,15 +535,31 @@ void BufferView::saveBookmark(bool persistent)
 }
 
 
-void BufferView::moveToPosition(int par_id, pos_type par_pos)
+boost::tuple<pit_type, int> BufferView::moveToPosition(pit_type par_pit, int par_id, pos_type par_pos)
 {
        cursor_.clearSelection();
 
-       ParIterator par = buffer_->getParFromID(par_id);
-       if (par == buffer_->par_iterator_end())
-               return;
-
-       setCursor(makeDocIterator(par, min(par->size(), par_pos)));
+       // if a valid par_id is given, try it first
+       if (par_id > 0) {
+               ParIterator par = buffer_->getParFromID(par_id);
+               if (par != buffer_->par_iterator_end()) {
+                       setCursor(makeDocIterator(par, min(par->size(), par_pos)));
+                       return boost::make_tuple(cursor_.pit(), par_id);
+               }
+       }
+       // if par_id == 0, or searching through par_id failed
+       if (static_cast<size_t>(par_pit) < buffer_->paragraphs().size()) {
+               ParIterator it = buffer_->par_iterator_begin();
+               ParIterator const end = buffer_->par_iterator_end();
+               for (; it != end; ++it)
+                       if (it.pit() == par_pit) {
+                               // restored pos may be bigger than it->size
+                               setCursor(makeDocIterator(it, min(par_pos, it->size())));
+                               return boost::make_tuple(par_pit, it->id());
+                       }
+       }
+       // both methods fail
+       return boost::make_tuple(pit_type(0), 0);
 }
 
 

diff --git a/src/BufferView.h b/src/BufferView.h
index b57d348048c2b33b87837a671d95369507584c14..1ccf16ed1ad5d7e8295388134bb1fbe0099c05cc 100644 (file)
--- a/src/BufferView.h
+++ b/src/BufferView.h
@@ -23,6 +23,7 @@
 
 #include "support/types.h"
 
+#include <boost/tuple/tuple.hpp>
 #include <boost/utility.hpp>
 #include <boost/signal.hpp>
 
@@ -116,8 +117,10 @@ public:
        /// Save the current position as bookmark.
        /// if persistent=false, save to temp_bookmark
        void saveBookmark(bool persistent);
-       /// goto a specified position.
-       void moveToPosition(
+       /// goto a specified position, try par_id first, and then par_pit
+       /// return the par_pit and par_id of the new paragraph
+       boost::tuple<pit_type, int> moveToPosition(
+               pit_type par_pit, ///< Paragraph pit, used when par_id is zero or invalid.
                int par_id, ///< Paragraph ID, \sa Paragraph
                pos_type par_pos ///< Position in the \c Paragraph
                );

diff --git a/src/lyxfunc.C b/src/lyxfunc.C
index 09307e9366595e6833d0242619a2c054168c4f95..0484e8e15049e7bcbb7408fc128d680d91a9dd0d 100644 (file)
--- a/src/lyxfunc.C
+++ b/src/lyxfunc.C
@@ -242,6 +242,42 @@ void LyXFunc::handleKeyFunc(kb_action action)
 }
 
 
+void LyXFunc::gotoBookmark(unsigned int idx, bool openFile, bool switchToBuffer)
+{
+       BOOST_ASSERT(lyx_view_);
+       if (!LyX::ref().session().bookmarks().isValid(idx))
+               return;
+       BookmarksSection::Bookmark const & bm = LyX::ref().session().bookmarks().bookmark(idx);
+       BOOST_ASSERT(!bm.filename.empty());
+       string const file = bm.filename.absFilename();
+       // if the file is not opened, open it.
+       if (!theBufferList().exists(file)) {
+               if (openFile)
+                       dispatch(FuncRequest(LFUN_FILE_OPEN, file));
+               else
+                       return;
+       }
+       // open may fail, so we need to test it again
+       if (theBufferList().exists(file)) {
+               // if the current buffer is not that one, switch to it.
+               if (lyx_view_->buffer()->fileName() != file) {
+                       if (switchToBuffer)
+                               dispatch(FuncRequest(LFUN_BUFFER_SWITCH, file));
+                       else
+                               return;
+               }
+               // moveToPosition use par_id, and par_pit and return new par_id.
+               pit_type new_pit;
+               int new_id;
+               boost::tie(new_pit, new_id) = view()->moveToPosition(bm.par_pit, bm.par_id, bm.par_pos);
+               // if par_id or pit has been changed, reset par_pit and par_id
+               // see http://bugzilla.lyx.org/show_bug.cgi?id=3092
+               if (bm.par_pit != new_pit || bm.par_id != new_id)
+                       const_cast<BookmarksSection::Bookmark &>(bm).setPos(new_pit, new_id);
+       } 
+}
+
+
 void LyXFunc::processKeySym(LyXKeySymPtr keysym, key_modifier::state state)
 {
        lyxerr[Debug::KEY] << "KeySym is " << keysym->getSymbolName() << endl;
@@ -1126,6 +1162,9 @@ void LyXFunc::dispatch(FuncRequest const & cmd)
                // --- buffers ----------------------------------------
                case LFUN_BUFFER_SWITCH:
                        BOOST_ASSERT(lyx_view_);
+                       // update bookmark pit of the current buffer before switch
+                       for (size_t i = 0; i < LyX::ref().session().bookmarks().size(); ++i)
+                               gotoBookmark(i+1, false, false);                        
                        lyx_view_->setBuffer(theBufferList().getBuffer(argument));
                        break;
 
@@ -1664,33 +1703,19 @@ void LyXFunc::dispatch(FuncRequest const & cmd)
                case LFUN_WINDOW_CLOSE:
                        BOOST_ASSERT(lyx_view_);
                        BOOST_ASSERT(theApp());
+                       // update bookmark pit of the current buffer before window close
+                       for (size_t i = 0; i < LyX::ref().session().bookmarks().size(); ++i)
+                               gotoBookmark(i+1, false, false);
                        // ask the user for saving changes or cancel quit
                        if (!theBufferList().quitWriteAll())
                                break;
                        lyx_view_->close();
                        return;
 
-               case LFUN_BOOKMARK_GOTO: {
-                       BOOST_ASSERT(lyx_view_);
-                       unsigned int idx = convert<unsigned int>(to_utf8(cmd.argument()));
-                       if (!LyX::ref().session().bookmarks().isValid(idx))
-                               break;
-                       BookmarksSection::Bookmark const bm = LyX::ref().session().bookmarks().bookmark(idx);
-                       BOOST_ASSERT(!bm.filename.empty());
-                       string const file = bm.filename.absFilename();
-                       // if the file is not opened, open it.
-                       if (!theBufferList().exists(file))
-                               dispatch(FuncRequest(LFUN_FILE_OPEN, file));
-                       // open may fail, so we need to test it again
-                       if (theBufferList().exists(file)) {
-                               // if the current buffer is not that one, switch to it.
-                               if (lyx_view_->buffer()->fileName() != file)
-                                       dispatch(FuncRequest(LFUN_BUFFER_SWITCH, file));
-                               // BOOST_ASSERT(lyx_view_->buffer()->fileName() != file);
-                               view()->moveToPosition(bm.par_id, bm.par_pos);
-                       } 
+               case LFUN_BOOKMARK_GOTO:
+                       // go to bookmark, open unopened file and switch to buffer if necessary
+                       gotoBookmark(convert<unsigned int>(to_utf8(cmd.argument())), true, true);
                        break;
-               }
 
                case LFUN_BOOKMARK_CLEAR:
                        LyX::ref().session().bookmarks().clear();
@@ -2001,6 +2026,9 @@ void LyXFunc::closeBuffer()
        // save current cursor position
        LyX::ref().session().lastFilePos().save(FileName(lyx_view_->buffer()->fileName()),
                boost::tie(view()->cursor().pit(), view()->cursor().pos()) );
+       // goto bookmark to update bookmark pit.
+       for (size_t i = 0; i < LyX::ref().session().bookmarks().size(); ++i)
+               gotoBookmark(i+1, false, false);
        if (theBufferList().close(lyx_view_->buffer(), true) && !quitting) {
                if (theBufferList().empty()) {
                        // need this otherwise SEGV may occur while

diff --git a/src/lyxfunc.h b/src/lyxfunc.h
index 2e36ba19ab1bd846fd89f3405a94101e92256f19..14168adcefb632f64b4806739b8abfe7dab68082 100644 (file)
--- a/src/lyxfunc.h
+++ b/src/lyxfunc.h
@@ -75,6 +75,11 @@ public:
        docstring const getMessage() const { return dispatch_buffer; }
        /// Handle a accented char key sequence
        void handleKeyFunc(kb_action action);
+       /// goto a bookmark
+       /// openFile: whether or not open a file if the file is not opened
+       /// switchToBuffer: whether or not switch to buffer if the buffer is 
+       ///             not the current buffer
+       void gotoBookmark(unsigned int idx, bool openFile, bool switchToBuffer);
 
 private:
        ///

diff --git a/src/session.C b/src/session.C
index e7541b97b9baae5a870760f54c9de5774874aca8..51a210e83e1ccd794a812f7ab65bb05ca65cc382 100644 (file)
--- a/src/session.C
+++ b/src/session.C
@@ -244,12 +244,12 @@ void BookmarksSection::read(istream & is)
 
                try {
                        // read bookmarks
-                       // id, pos, file\n
-                       unsigned int id;
+                       // pit, pos, file\n
+                       pit_type pit;
                        pos_type pos;
                        string fname;
                        istringstream itmp(tmp);
-                       itmp >> id;
+                       itmp >> pit;
                        itmp.ignore(2);  // ignore ", "
                        itmp >> pos;
                        itmp.ignore(2);  // ignore ", "
@@ -261,7 +261,7 @@ void BookmarksSection::read(istream & is)
                        if (fs::exists(file.toFilesystemEncoding()) &&
                            !fs::is_directory(file.toFilesystemEncoding()) &&
                            bookmarks.size() < max_bookmarks)
-                               bookmarks.push_back(Bookmark(file, id, pos));
+                               bookmarks.push_back(Bookmark(file, pit, 0, pos));
                        else
                                lyxerr[Debug::INIT] << "LyX: Warning: Ignore bookmark of file: " << fname << endl;
                } catch (...) {
@@ -275,22 +275,22 @@ void BookmarksSection::write(ostream & os) const
 {
        os << '\n' << sec_bookmarks << '\n';
        for (size_t i = 0; i < bookmarks.size(); ++i) {
-               os << bookmarks[i].par_id << ", "
+               os << bookmarks[i].par_pit << ", "
                   << bookmarks[i].par_pos << ", "
                   << bookmarks[i].filename << '\n';
        }
 }
 
 
-void BookmarksSection::save(FileName const & fname, int par_id, pos_type par_pos, bool persistent)
+void BookmarksSection::save(FileName const & fname, pit_type par_pit, int par_id, pos_type par_pos, bool persistent)
 {
        if (persistent) {
-               bookmarks.push_back(Bookmark(fname, par_id, par_pos));
+               bookmarks.push_back(Bookmark(fname, par_pit, par_id, par_pos));
                if (bookmarks.size() > max_bookmarks)
                        bookmarks.pop_back();
                }
        else
-               temp_bookmark = Bookmark(fname, par_id, par_pos);
+               temp_bookmark = Bookmark(fname, par_pit, par_id, par_pos);
 }
 
 

diff --git a/src/session.h b/src/session.h
index 83e7f26b948f51e9b27abf7d0aa83eac533ff71b..f36730b6bb6f859e2a557d838f3ed7c6b319b4e4 100644 (file)
--- a/src/session.h
+++ b/src/session.h
@@ -179,15 +179,23 @@ public:
        public:
                /// Filename
                support::FileName filename;
-               /// Cursor paragraph Id
+               /// Cursor pit, will be saved/restored by .lyx/session
+               pit_type par_pit;
+               /// Cursor paragraph Id, used to lcoate bookmarks for opened files
                int par_id;
-               /// Cursor position
+               /// Cursor position within a paragraph
                pos_type par_pos;
                ///
                Bookmark() : par_id(0), par_pos(0) {}
                ///
-               Bookmark(support::FileName const & f, int id, pos_type pos)
-                       : filename(f), par_id(id), par_pos(pos) {}
+               Bookmark(support::FileName const & f, pit_type pit, int id, pos_type pos)
+                       : filename(f), par_pit(pit), par_id(id), par_pos(pos) {}
+               /// set bookmark par_id, this is because newly loaded bookmark
+               /// may have zero par_id and par_pit can change during editing, see bug 3092
+               void setPos(pit_type pit, int id) { 
+                       par_pit = pit;
+                       par_id = id;
+               }
        };
 
        ///
@@ -200,7 +208,7 @@ public:
 
        /// Save the current position as bookmark
        /// if save==false, save to temp_bookmark
-       void save(support::FileName const & fname, int par_id, pos_type par_pos, bool persistent);
+       void save(support::FileName const & fname, pit_type pit, int par_id, pos_type par_pos, bool persistent);
 
        /// return bookmark, return temp_bookmark if i==0
        Bookmark const & bookmark(unsigned int i) const;