Attached is a new version with LFUN_BOOKMARK_CLEAR. The bookmarks work well except that
1. assert error when lyx exits 2. when open a new empty window, bookmark menu works fine but go to bookmark will fail. Both problems relate to invalid lyx_view_ (I guess). Since I am not sure how buffer/view/lyx_view/multi-win interact. Abdel, could you please have a look at this part: case LFUN_BOOKMARK_GOTO: { unsigned int idx = convert<unsigned int>(to_utf8(cmd.argument())); BookmarksSection::Bookmark const bm = LyX::ref().session().bookmarks().bookmark(idx); BOOST_ASSERT(!bm.filename.empty()); // if the file is not opened, open it. if (!theBufferList().exists(bm.filename)) open(bm.filename); // open may fail, so we need to test it again if (theBufferList().exists(bm.filename)) { // if the current buffer is not that one, switch to it. if (lyx_view_->buffer()->fileName() != bm.filename) dispatch(FuncRequest(LFUN_BUFFER_SWITCH, bm.filename)); BOOST_ASSERT(lyx_view_->buffer()->fileName() != bm.filename); view()->restorePosition(bm.par_id, bm.par_pos); } break; } The test case would be: 1. open a codument 2. save bookmark 3. open window 4. go to bookmark (the document should open in the new window, with the correct position) Cheers, Bo
Index: src/LyXAction.C =================================================================== --- src/LyXAction.C (revision 15641) +++ src/LyXAction.C (working copy) @@ -102,6 +102,7 @@ { LFUN_APPENDIX, "appendix", Noop }, { LFUN_BOOKMARK_GOTO, "bookmark-goto", ReadOnly }, { LFUN_BOOKMARK_SAVE, "bookmark-save", ReadOnly }, + { LFUN_BOOKMARK_CLEAR, "bookmark-clear", ReadOnly }, { LFUN_BREAK_LINE, "break-line", Noop }, { LFUN_BREAK_PARAGRAPH, "break-paragraph", Noop }, { LFUN_BREAK_PARAGRAPH_KEEP_LAYOUT, "break-paragraph-keep-layout", Noop }, Index: src/session.C =================================================================== --- src/session.C (revision 15641) +++ src/session.C (working copy) @@ -211,22 +211,19 @@ break; getline(is, tmp); // read bookmarks - // bookmarkid, id, pos, file\n - unsigned int num; + // id, pos, file\n unsigned int id; pos_type pos; string fname; istringstream itmp(tmp); - itmp >> num; - itmp.ignore(2); // ignore ", " itmp >> id; itmp.ignore(2); // ignore ", " itmp >> pos; itmp.ignore(2); // ignore ", " itmp >> fname; // only load valid bookmarks - if (fs::exists(fname)) - bookmarks.push_back(boost::tie(num, fname, id, pos)); + if (bookmarks.size() < max_bookmarks && fs::exists(fname)) + bookmarks.push_back(Bookmark(fname, id, pos)); } while (is.good()); } @@ -234,23 +231,42 @@ void BookmarksSection::write(ostream & os) const { os << '\n' << sec_bookmarks << '\n'; - for (BookmarkList::const_iterator bm = bookmarks.begin(); - bm != bookmarks.end(); ++bm) { - // save bookmark number, id, pos, fname - os << bm->get<0>() << ", " - << bm->get<2>() << ", " - << bm->get<3>() << ", " - << bm->get<1>() << '\n'; + for (size_t i = 0; i < bookmarks.size(); ++i) { + os << bookmarks[i].par_id << ", " + << bookmarks[i].par_pos << ", " + << bookmarks[i].filename << '\n'; } } -void BookmarksSection::save(Bookmark const & bookmark) +void BookmarksSection::save(std::string const & fname, int par_id, pos_type par_pos, bool persistent) { - bookmarks.push_back(bookmark); + if (persistent) { + bookmarks.push_front(Bookmark(fname, par_id, par_pos)); + if (bookmarks.size() > max_bookmarks) + bookmarks.pop_back(); + } + else + temp_bookmark = Bookmark(fname, par_id, par_pos); } +bool BookmarksSection::isValid(unsigned int i) const +{ + // i == 0, or in the queue + return i <= bookmarks.size(); +} + + +BookmarksSection::Bookmark const & BookmarksSection::bookmark(unsigned int i) const +{ + if (i == 0) + return temp_bookmark; + else + return bookmarks[i-1]; +} + + void SessionInfoSection::read(istream & is) { string tmp; @@ -301,7 +317,6 @@ } - Session::Session(unsigned int num) : last_files(num) { Index: src/lyxfunc.C =================================================================== --- src/lyxfunc.C (revision 15641) +++ src/lyxfunc.C (working copy) @@ -352,6 +352,13 @@ flag.message(from_utf8(N_("Exiting"))); flag.enabled(true); return flag; + } else if (cmd.action == LFUN_BOOKMARK_GOTO) { + // bookmarks can be valid even if there is no opened buffer + flag.enabled(LyX::ref().session().bookmarks().isValid(convert<unsigned int>(to_utf8(cmd.argument())))); + return flag; + } else if (cmd.action == LFUN_BOOKMARK_CLEAR) { + flag.enabled(LyX::ref().session().bookmarks().size() > 0); + return flag; } LCursor & cur = view()->cursor(); @@ -1035,8 +1042,6 @@ // save cursor Position for opened files to .lyx/session LyX::ref().session().lastFilePos().save(lyx_view_->buffer()->fileName(), boost::tie(view()->cursor().pit(), view()->cursor().pos()) ); - // save bookmarks to .lyx/session - view()->saveSavedPositions(); } LyX::ref().quit(); @@ -1342,7 +1347,7 @@ // FIXME Should use bformat setMessage(_("Opening child document ") + makeDisplayPath(filename) + "..."); - view()->savePosition(0); + view()->savePosition(false); string const parentfilename = lyx_view_->buffer()->fileName(); if (theBufferList().exists(filename)) lyx_view_->setBuffer(theBufferList().getBuffer(filename)); @@ -1660,6 +1665,28 @@ // We return here because lyx_view does not exists anymore. return; + case LFUN_BOOKMARK_GOTO: { + unsigned int idx = convert<unsigned int>(to_utf8(cmd.argument())); + BookmarksSection::Bookmark const bm = LyX::ref().session().bookmarks().bookmark(idx); + BOOST_ASSERT(!bm.filename.empty()); + // if the file is not opened, open it. + if (!theBufferList().exists(bm.filename)) + open(bm.filename); + // open may fail, so we need to test it again + if (theBufferList().exists(bm.filename)) { + // if the current buffer is not that one, switch to it. + if (lyx_view_->buffer()->fileName() != bm.filename) + dispatch(FuncRequest(LFUN_BUFFER_SWITCH, bm.filename)); + BOOST_ASSERT(lyx_view_->buffer()->fileName() != bm.filename); + view()->restorePosition(bm.par_id, bm.par_pos); + } + break; + } + + case LFUN_BOOKMARK_CLEAR: + LyX::ref().session().bookmarks().clear(); + break; + default: { BOOST_ASSERT(lyx_view_); view()->cursor().dispatch(cmd); Index: src/session.h =================================================================== --- src/session.h (revision 15641) +++ src/session.h (working copy) @@ -173,32 +173,68 @@ class BookmarksSection : SessionSection { public: - /// - typedef boost::tuple<unsigned int, std::string, unsigned int, pos_type> Bookmark; + /// bookmarks + class Bookmark { + public: + /// Filename + std::string filename; + /// Cursor paragraph Id + int par_id; + /// Cursor position + pos_type par_pos; + /// + Bookmark() : par_id(0), par_pos(0) {} + /// + Bookmark(std::string const & f, int id, pos_type pos) + : filename(f), par_id(id), par_pos(pos) {} + }; /// - typedef std::vector<Bookmark> BookmarkList; + typedef std::deque<Bookmark> BookmarkList; public: + /// constructor, set max_bookmarks + /// allow 20 regular bookmarks, and bookmarks[0] for special purposes + /// c.f. ./frontends/controllers/ControlRef.C + BookmarksSection::BookmarksSection() : max_bookmarks(20), bookmarks(0) {} + + /// Save the current position as bookmark + /// if save==false, save to bookmarks[0] + void save(std::string const & fname, int par_id, pos_type par_pos, bool persistent); + + /// return bookmark, exclude the temp bookmark + Bookmark const & bookmark(unsigned int i) const; + + /// does the given bookmark have a saved position ? + bool isValid(unsigned int i) const; + /// + unsigned int size() const { return bookmarks.size(); } + + /// clear all bookmarks + void clear() { bookmarks.clear(); } + + /// void read(std::istream & is); /// void write(std::ostream & os) const; - /** save a bookmark - @bookmark bookmark to be saved - */ - void save(Bookmark const & bookmark); - /** return bookmark list. Non-const container is used since bookmarks will be cleaned after use. */ BookmarkList & load() { return bookmarks; } private: + /// temp bookmark (previously saved_positions[0]) + /// this is really ugly + Bookmark temp_bookmark; + /// a list of bookmarks BookmarkList bookmarks; + + /// + unsigned int const max_bookmarks; }; Index: src/BufferView.C =================================================================== --- src/BufferView.C (revision 15641) +++ src/BufferView.C (working copy) @@ -103,9 +103,6 @@ namespace { -unsigned int const saved_positions_num = 20; - - /// Return an inset of this class if it exists at the current cursor position template <class T> T * getInsetByCode(LCursor & cur, InsetBase::Code code) @@ -129,17 +126,6 @@ intl_(new Intl) { xsel_cache_.set = false; - - saved_positions.resize(saved_positions_num); - // load saved bookmarks - BookmarksSection::BookmarkList & bmList = LyX::ref().session().bookmarks().load(); - for (BookmarksSection::BookmarkList::iterator bm = bmList.begin(); - bm != bmList.end(); ++bm) - if (bm->get<0>() < saved_positions_num) - saved_positions[bm->get<0>()] = Position( bm->get<1>(), bm->get<2>(), bm->get<3>() ); - // and then clear them - bmList.clear(); - intl_->initKeyMapper(lyxrc.use_kbmap); } @@ -514,74 +500,32 @@ } -void BufferView::savePosition(unsigned int i) +void BufferView::savePosition(bool persistent) { - if (i >= saved_positions_num) - return; - BOOST_ASSERT(cursor_.inTexted()); - saved_positions[i] = Position(buffer_->fileName(), - cursor_.paragraph().id(), - cursor_.pos()); - if (i > 0) + LyX::ref().session().bookmarks().save( + buffer_->fileName(), + cursor_.paragraph().id(), + cursor_.pos(), + persistent + ); + if (persistent) // emit message signal. - message(bformat(_("Saved bookmark %1$d"), i)); + message(_("Save bookmark")); } -void BufferView::restorePosition(unsigned int i) +void BufferView::restorePosition(int par_id, pos_type par_pos) { - if (i >= saved_positions_num) - return; - - string const fname = saved_positions[i].filename; - cursor_.clearSelection(); - if (fname != buffer_->fileName()) { - Buffer * b = 0; - if (theBufferList().exists(fname)) - b = theBufferList().getBuffer(fname); - else { - b = theBufferList().newBuffer(fname); - // Don't ask, just load it - lyx::loadLyXFile(b, fname); - } - if (b) - setBuffer(b); - } - - ParIterator par = buffer_->getParFromID(saved_positions[i].par_id); + ParIterator par = buffer_->getParFromID(par_id); if (par == buffer_->par_iterator_end()) return; - setCursor(makeDocIterator(par, min(par->size(), saved_positions[i].par_pos))); - - if (i > 0) - // emit message signal. - message(bformat(_("Moved to bookmark %1$d"), i)); + setCursor(makeDocIterator(par, min(par->size(), par_pos))); } -bool BufferView::isSavedPosition(unsigned int i) -{ - return i < saved_positions_num && !saved_positions[i].filename.empty(); -} - -void BufferView::saveSavedPositions() -{ - // save bookmarks. It is better to use the pit interface - // but I do not know how to effectively convert between - // par_id and pit. - for (unsigned int i=1; i < saved_positions_num; ++i) { - if ( isSavedPosition(i) ) - LyX::ref().session().bookmarks().save( boost::tie( - i, - saved_positions[i].filename, - saved_positions[i].par_id, - saved_positions[i].par_pos) ); - } -} - void BufferView::switchKeyMap() { if (!lyxrc.rtl_support) @@ -663,10 +607,6 @@ break; } - case LFUN_BOOKMARK_GOTO: - flag.enabled(isSavedPosition(convert<unsigned int>(to_utf8(cmd.argument())))); - break; - case LFUN_CHANGES_TRACK: flag.enabled(true); flag.setOnOff(buffer_->params().trackChanges); @@ -763,10 +703,6 @@ savePosition(convert<unsigned int>(to_utf8(cmd.argument()))); break; - case LFUN_BOOKMARK_GOTO: - restorePosition(convert<unsigned int>(to_utf8(cmd.argument()))); - break; - case LFUN_LABEL_GOTO: { docstring label = cmd.argument(); if (label.empty()) { @@ -775,7 +711,8 @@ InsetBase::REF_CODE); if (inset) { label = inset->getParam("reference"); - savePosition(0); + // false: use bookmark 0 + savePosition(false); } } Index: src/MenuBackend.C =================================================================== --- src/MenuBackend.C (revision 15641) +++ src/MenuBackend.C (working copy) @@ -215,6 +215,7 @@ md_item = 1, md_branches, md_documents, + md_bookmarks, md_charstyles, md_endmenu, md_exportformats, @@ -237,6 +238,7 @@ { "branches", md_branches }, { "charstyles", md_charstyles }, { "documents", md_documents }, + { "bookmarks", md_bookmarks }, { "end", md_endmenu }, { "exportformats", md_exportformats }, { "floatinsert", md_floatinsert }, @@ -293,6 +295,10 @@ add(MenuItem(MenuItem::Documents)); break; + case md_bookmarks: + add(MenuItem(MenuItem::Bookmarks)); + break; + case md_toc: add(MenuItem(MenuItem::Toc)); break; @@ -464,6 +470,22 @@ } +void expandBookmarks(Menu & tomenu) +{ + lyx::BookmarksSection const & bm = LyX::cref().session().bookmarks(); + + for (size_t i = 1; i <= bm.size(); ++i) { + if (bm.isValid(i)) { + docstring const label = convert<docstring>(i) + ". " + + makeDisplayPath(bm.bookmark(i).filename, 20) + + char_type('|') + convert<docstring>(i); + tomenu.add(MenuItem(MenuItem::Command, label, FuncRequest(LFUN_BOOKMARK_GOTO, + convert<docstring>(i)))); + } + } +} + + void expandFormats(MenuItem::Kind kind, Menu & tomenu, Buffer const * buf) { if (!buf && kind != MenuItem::ImportFormats) { @@ -770,6 +792,10 @@ expandDocuments(tomenu); break; + case MenuItem::Bookmarks: + expandBookmarks(tomenu); + break; + case MenuItem::ImportFormats: case MenuItem::ViewFormats: case MenuItem::UpdateFormats: Index: src/lfuns.h =================================================================== --- src/lfuns.h (revision 15641) +++ src/lfuns.h (working copy) @@ -372,6 +372,8 @@ LFUN_WINDOW_NEW, // Abdel 20061021 LFUN_WINDOW_CLOSE, // Abdel 20061023 LFUN_UNICODE_INSERT, // Lgb 20061022 + // 285 + LFUN_BOOKMARK_CLEAR, // bpeng 20061031 LFUN_LASTACTION // end of the table }; Index: src/BufferView.h =================================================================== --- src/BufferView.h (revision 15641) +++ src/BufferView.h (working copy) @@ -102,15 +102,10 @@ /// return the Scrollbar Parameters ScrollbarParameters const & scrollbarParameters() const; - /// Save the current position as bookmark i - void savePosition(unsigned int i); + /// Save the current position as bookmark, if persistent=false, save to bookmark 0 + void savePosition(bool persistent); /// Restore the position from bookmark i - void restorePosition(unsigned int i); - /// does the given bookmark have a saved position ? - bool isSavedPosition(unsigned int i); - /// save bookmarks to .lyx/session - void saveSavedPositions(); - + void restorePosition(int par_id, pos_type par_pos); /// return the current change at the cursor Change const getCurrentChange() const; @@ -237,23 +232,7 @@ /// Estimated average par height for scrollbar int wh_; /// - class Position { - public: - /// Filename - std::string filename; - /// Cursor paragraph Id - int par_id; - /// Cursor position - pos_type par_pos; - /// - Position() : par_id(0), par_pos(0) {} - /// - Position(std::string const & f, int id, pos_type pos) - : filename(f), par_id(id), par_pos(pos) {} - }; /// - std::vector<Position> saved_positions; - /// void menuInsertLyXFile(std::string const & filen); /// this is used to handle XSelection events in the right manner Index: src/MenuBackend.h =================================================================== --- src/MenuBackend.h (revision 15641) +++ src/MenuBackend.h (working copy) @@ -44,6 +44,8 @@ /** This is the list of opened Documents, typically for the Documents menu. */ Documents, + /** This is the bookmarks */ + Bookmarks, /// Toc, /** This is a list of viewable formats Index: lib/ui/stdmenus.ui =================================================================== --- lib/ui/stdmenus.ui (revision 15641) +++ lib/ui/stdmenus.ui (working copy) @@ -424,17 +424,10 @@ End Menu "navigate_bookmarks" - Item "Go to Bookmark 1|1" "bookmark-goto 1" - Item "Go to Bookmark 2|2" "bookmark-goto 2" - Item "Go to Bookmark 3|3" "bookmark-goto 3" - Item "Go to Bookmark 4|4" "bookmark-goto 4" - Item "Go to Bookmark 5|5" "bookmark-goto 5" + Item "Save Bookmark|S" "bookmark-save 1" + Item "Clear Bookmarks|C" "bookmark-clear" Separator - Item "Save Bookmark 1|S" "bookmark-save 1" - Item "Save Bookmark 2" "bookmark-save 2" - Item "Save Bookmark 3" "bookmark-save 3" - Item "Save Bookmark 4" "bookmark-save 4" - Item "Save Bookmark 5" "bookmark-save 5" + Bookmarks End #