New bookmarks implementation:

* src/lyxfunc.C: handle LFUN_BOOKMARK_GOTO and LFUN_BOOKMARK_CLEAR
	* src/session.h/C: store bookmarks here.
	* src/lfuns.h, src/LyXAction.C: add LFUN_BOOKMARK_CLEAR
	* src/BufferView.h/C: remove saved_positions, move LFUN_BOOKMARK_GOTO to lyxfunc.C
	* src/MenuBackend.h/C: add Bookmarks menu type
	* lib/ui/stdmenus.ui: change bookmarks menu items


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

lib/ui/stdmenus.ui

@@ -424,17 +424,10 @@ Menuset
 	End
 
 	Menu "navigate_bookmarks"
-		Item "Go to Bookmark 1|1" "bookmark-goto 1"
+		Item "Save Bookmark|S" "bookmark-save 1"
-		Item "Go to Bookmark 2|2" "bookmark-goto 2"
+		Item "Clear Bookmarks|C" "bookmark-clear"
-		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"
 		Separator
-		Item "Save Bookmark 1|S" "bookmark-save 1"
+		Bookmarks
-		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"
 	End
 
 #

src/BufferView.C

@@ -103,9 +103,6 @@ namespace Alert = frontend::Alert;
 
 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 @@ BufferView::BufferView()
 	  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 @@ Change const BufferView::getCurrentChange() const
 }
 
 
-void BufferView::savePosition(unsigned int i)
+void BufferView::saveBookmark(bool persistent)
 {
-	if (i >= saved_positions_num)
+	LyX::ref().session().bookmarks().save(
-		return;
+		buffer_->fileName(),
-	BOOST_ASSERT(cursor_.inTexted());
+		cursor_.paragraph().id(),
-	saved_positions[i] = Position(buffer_->fileName(),
+		cursor_.pos(),
-				      cursor_.paragraph().id(),
+		persistent
-				      cursor_.pos());
+	);
-	if (i > 0)
+	if (persistent)
 		// emit message signal.
-		message(bformat(_("Saved bookmark %1$d"), i));
+		message(_("Save bookmark"));
 }
 
 
-void BufferView::restorePosition(unsigned int i)
+void BufferView::moveToPosition(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()) {
+	ParIterator par = buffer_->getParFromID(par_id);
-		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);
 	if (par == buffer_->par_iterator_end())
 		return;
 
-	setCursor(makeDocIterator(par, min(par->size(), saved_positions[i].par_pos)));
+	setCursor(makeDocIterator(par, min(par->size(), par_pos)));
-
-	if (i > 0)
-		// emit message signal.
-		message(bformat(_("Moved to bookmark %1$d"), i));
 }
 
 
-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 @@ FuncStatus BufferView::getStatus(FuncRequest const & cmd)
 		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);
@@ -760,11 +700,7 @@ bool BufferView::dispatch(FuncRequest const & cmd)
 		break;
 
 	case LFUN_BOOKMARK_SAVE:
-		savePosition(convert<unsigned int>(to_utf8(cmd.argument())));
+		saveBookmark(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: {
@@ -775,7 +711,8 @@ bool BufferView::dispatch(FuncRequest const & cmd)
 							 InsetBase::REF_CODE);
 			if (inset) {
 				label = inset->getParam("reference");
-				savePosition(0);
+				// persistent=false: use temp_bookmark
+				saveBookmark(false);
 			}
 		}
 

src/BufferView.h

@@ -102,15 +102,10 @@ public:
 	/// return the Scrollbar Parameters
 	ScrollbarParameters const & scrollbarParameters() const;
 
-	/// Save the current position as bookmark i
+	/// Save the current position as bookmark, if persistent=false, save to temp_bookmark
-	void savePosition(unsigned int i);
+	void saveBookmark(bool persistent);
-	/// Restore the position from bookmark i
+	/// goto a specified position
-	void restorePosition(unsigned int i);
+	void moveToPosition(int par_id, pos_type par_pos);
-	/// does the given bookmark have a saved position ?
-	bool isSavedPosition(unsigned int i);
-	/// save bookmarks to .lyx/session
-	void saveSavedPositions();
-
 	/// return the current change at the cursor
 	Change const getCurrentChange() const;
 
@@ -237,22 +232,6 @@ private:
 	/// 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);
 

src/LyXAction.C

@@ -102,6 +102,7 @@ void LyXAction::init()
 		{ 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 },

src/MenuBackend.C

@@ -215,6 +215,7 @@ Menu & Menu::read(LyXLex & lex)
 		md_item = 1,
 		md_branches,
 		md_documents,
+		md_bookmarks,
 		md_charstyles,
 		md_endmenu,
 		md_exportformats,
@@ -237,6 +238,7 @@ Menu & Menu::read(LyXLex & lex)
 		{ "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 @@ Menu & Menu::read(LyXLex & lex)
 			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 expandDocuments(Menu & tomenu)
 }
 
 
+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 @@ void MenuBackend::expand(Menu const & frommenu, Menu & tomenu,
 			expandDocuments(tomenu);
 			break;
 
+		case MenuItem::Bookmarks:
+			expandBookmarks(tomenu);
+			break;
+
 		case MenuItem::ImportFormats:
 		case MenuItem::ViewFormats:
 		case MenuItem::UpdateFormats:

src/MenuBackend.h

@@ -44,6 +44,8 @@ public:
 		/** 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

src/lfuns.h

@@ -372,6 +372,8 @@ enum kb_action {
 	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
 };

src/lyxfunc.C

@@ -630,6 +630,19 @@ FuncStatus LyXFunc::getStatus(FuncRequest const & cmd) const
 		// these are handled in our dispatch()
 		break;
 
+	// FIXME: will move to the front of this function when SWITCH_TO_BUFFER
+	// is valid for a newly created window 
+	case 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()))));
+		break;
+
+	// FIXME: will move to the front of this function when SWITCH_TO_BUFFER
+	// is valid for a newly created window 
+	case LFUN_BOOKMARK_CLEAR:
+		flag.enabled(LyX::ref().session().bookmarks().size() > 0);
+		break;
+
 	default:
 		if (!getLocalStatus(cur, cmd, flag))
 			flag = view()->getStatus(cmd);
@@ -1035,8 +1048,6 @@ void LyXFunc::dispatch(FuncRequest const & cmd)
 				// 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();
@@ -1343,7 +1354,7 @@ void LyXFunc::dispatch(FuncRequest const & cmd)
 			// FIXME Should use bformat
 			setMessage(_("Opening child document ") +
 					 makeDisplayPath(filename) + "...");
-			view()->savePosition(0);
+			view()->saveBookmark(false);
 			string const parentfilename = lyx_view_->buffer()->fileName();
 			if (theBufferList().exists(filename))
 				lyx_view_->setBuffer(theBufferList().getBuffer(filename));
@@ -1661,6 +1672,31 @@ void LyXFunc::dispatch(FuncRequest const & cmd)
 			// We return here because lyx_view does not exists anymore.
 			return;
 
+		case LFUN_BOOKMARK_GOTO: {
+			BOOST_ASSERT(lyx_view_);
+			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))
+				dispatch(FuncRequest(LFUN_FILE_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.
+				// FIXME: swtich buffer to a newly created window will crash lyx
+				// because of invalid view.
+				if (lyx_view_->buffer()->fileName() != bm.filename)
+					dispatch(FuncRequest(LFUN_BUFFER_SWITCH, bm.filename));
+				// BOOST_ASSERT(lyx_view_->buffer()->fileName() != bm.filename);
+				view()->moveToPosition(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);

src/session.C

@@ -211,22 +211,19 @@ void BookmarksSection::read(istream & is)
 			break;
 		getline(is, tmp);
 		// read bookmarks
-		// bookmarkid, id, pos, file\n
+		// id, pos, file\n
-		unsigned int num;
 		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))
+		if (bookmarks.size() < max_bookmarks && fs::exists(fname))
-			bookmarks.push_back(boost::tie(num, fname, id, pos));
+			bookmarks.push_back(Bookmark(fname, id, pos));
 	} while (is.good());
 }
 
@@ -234,20 +231,39 @@ void BookmarksSection::read(istream & is)
 void BookmarksSection::write(ostream & os) const
 {
 	os << '\n' << sec_bookmarks << '\n';
-	for (BookmarkList::const_iterator bm = bookmarks.begin();
+	for (size_t i = 0; i < bookmarks.size(); ++i) {
-		bm != bookmarks.end(); ++bm) {
+		os << bookmarks[i].par_id << ", "
-		// save bookmark number, id, pos, fname
+		   << bookmarks[i].par_pos << ", "
-		os << bm->get<0>() << ", "
+		   << bookmarks[i].filename << '\n';
-			<< bm->get<2>() << ", "
-			<< bm->get<3>() << ", "
-			<< bm->get<1>() << '\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];
 }
 
 
@@ -301,7 +317,6 @@ string const SessionInfoSection::load(string const & key, bool release)
 }
 
 
-
 Session::Session(unsigned int num) :
 	last_files(num)
 {

src/session.h

@@ -173,32 +173,68 @@ private:
 class BookmarksSection : SessionSection
 {
 public:
-	///
+	/// bookmarks
-	typedef boost::tuple<unsigned int, std::string, unsigned int, pos_type> Bookmark;
+	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
+	BookmarksSection::BookmarksSection() : max_bookmarks(20), bookmarks(0) {}
+
+	/// Save the current position as bookmark
+	/// if save==false, save to temp_bookmark
+	void save(std::string const & fname, int par_id, pos_type par_pos, bool persistent);
+
+	/// return bookmark, return temp_bookmark if i==0
+	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
+	/// c.f. ./frontends/controllers/ControlRef.C
+	/// FIXME: a separate LFUN may be a better solution
+	Bookmark temp_bookmark;
+
 	/// a list of bookmarks
 	BookmarkList bookmarks;
+
+	///
+	unsigned int const max_bookmarks;
 };