This patch uses both pit and id to locate a bookmark.
1. both pit and id are used to locate a position. Pit is saved by session.
2. moveToPosition now returns id and pit of the new position and
update bookmark if necessary. These happens:
* when id is invalid (Uwe's case), pit is used, and bookmark id is updated.
* when a paragraph is added/removed, id can still locate the
paragraph but pit needs to be updated.
3. when a buffer/window is closed, its bookmarks are updated (by going
to the bookmarks and get their pit.) This avoids the following
problem: set bookmark; go to the begining of the buffer add a few
paragraphs (id ok, pit changed); close buffer; open the buffer using
the bookmark, bookmark is shifted because id is now invalid, pit is
shifted.
You can see that I update bookmark when buffer switch, buffer close,
windows close, but I still can not update bookmark when File->exit is
triggered.
WHY CANNOT WE take the time to fire LFUN_BUFFER_CLOSE, and
LFUN_WINDOW_CLOSE when lyx exists? When lyx takes shortcuts to quit,
proper cleanup in these CLOSE events may be bypassed.
Bo
Index: src/session.C
===================================================================
--- src/session.C (revision 16638)
+++ src/session.C (working copy)
@@ -244,12 +244,12 @@
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 @@
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 @@
{
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);
}
Index: src/lyxfunc.C
===================================================================
--- src/lyxfunc.C (revision 16638)
+++ src/lyxfunc.C (working copy)
@@ -242,6 +242,42 @@
}
+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_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 @@
// --- buffers ----------------------------------------
case LFUN_BUFFER_SWITCH:
BOOST_ASSERT(lyx_view_);
+ // update bookmark pit.
+ 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 @@
case LFUN_WINDOW_CLOSE:
BOOST_ASSERT(lyx_view_);
BOOST_ASSERT(theApp());
+ // update bookmark pit.
+ 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 @@
// 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
Index: src/session.h
===================================================================
--- src/session.h (revision 16638)
+++ src/session.h (working copy)
@@ -179,6 +179,8 @@
public:
/// Filename
support::FileName filename;
+ /// Cursor pit
+ pit_type par_pit;
/// Cursor paragraph Id
int par_id;
/// Cursor position
@@ -186,8 +188,14 @@
///
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 invalid or zero par_id, see bug 3092
+ void setPos(pit_type pit, int id) {
+ par_pit = pit;
+ par_id = id;
+ }
};
///
@@ -200,7 +208,7 @@
/// 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;
Index: src/lyxfunc.h
===================================================================
--- src/lyxfunc.h (revision 16638)
+++ src/lyxfunc.h (working copy)
@@ -75,6 +75,11 @@
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:
///
Index: src/BufferView.C
===================================================================
--- src/BufferView.C (revision 16638)
+++ src/BufferView.C (working copy)
@@ -263,22 +263,10 @@
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);
}
}
@@ -535,6 +523,7 @@
{
LyX::ref().session().bookmarks().save(
FileName(buffer_->fileName()),
+ cursor_.pit(),
cursor_.paragraph().id(),
cursor_.pos(),
persistent
@@ -545,15 +534,31 @@
}
-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);
}
Index: src/BufferView.h
===================================================================
--- src/BufferView.h (revision 16638)
+++ src/BufferView.h (working copy)
@@ -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 @@
/// 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
);
