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
 
 #

Reply via email to