The attached addresses http://bugzilla.lyx.org/show_bug.cgi?id=3650. The
issues were two. (i) When the old inset was erased, the iterator became
invalid, and the attempt to increment it caused an abort; (ii) after
clearing that up, a different abort made it clear that the cursor
position needed to be updated so it wouldn't be past the end of the text.

Testing requested, as well as two commit OKs if it seems all right.

NOTE: Some other issues I noticed here, which I'll put in bugzilla if it
seems a good idea. (i) Take the same example file and open it. I get all
labels as [1], at least with the patch. (ii) Put the cursor at the end
and hit return. The new InsetBibitem has label [1] rather than [4],
which is what it seems it should have. (iii) Change the layout of one of
these paragraphs to Standard. Shouldn't the bibitem be erased? (This
could be handled in checkBiblio without too much effort.)

Richard

-- 
==================================================================
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==================================================================
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto

Index: Paragraph.h
===================================================================
--- Paragraph.h	(revision 18413)
+++ Paragraph.h	(working copy)
@@ -359,9 +359,11 @@
 	///
 	bool hfillExpansion(Row const & row, pos_type pos) const;
 
-	/// Check if we are in a Biblio environment.
-	/// \retval true if the cursor needs to be moved right.
-	bool checkBiblio(bool track_changes);
+	/// Check if we are in a Biblio environment and insert or
+	/// delete InsetBibitems as necessary.
+	/// \retval int 1 to move cursor right; -1 to move it left;
+	/// 0 to leave it where it is.
+	int checkBiblio(bool track_changes);
 
 public:
 	///
Index: Paragraph.cpp
===================================================================
--- Paragraph.cpp	(revision 18413)
+++ Paragraph.cpp	(working copy)
@@ -2592,7 +2592,7 @@
 }
 
 
-bool Paragraph::checkBiblio(bool track_changes)
+int Paragraph::checkBiblio(bool track_changes)
 {
 	// Add bibitem insets if necessary
 	if (layout()->labeltype != LABEL_BIBLIO)
@@ -2606,33 +2606,51 @@
 	docstring oldkey;
 	docstring oldlabel;
 
-	// remove bibitems in pos != 0
-	// restore them later in pos 0 if necessary
+	// remove a bibitem in pos != 0
+	// restore it later in pos 0 if necessary
 	// (e.g. if a user inserts contents _before_ the item)
-	InsetList::const_iterator it = insetlist.begin();
-	InsetList::const_iterator end = insetlist.end();
+	// we're assuming there's only one of these, which there
+	// should be.
+	bool erasedInset = false;
+	InsetList::iterator it = insetlist.begin();
+	InsetList::iterator end = insetlist.end();
 	for (; it != end; ++it)
 		if (it->inset->lyxCode() == Inset::BIBITEM_CODE
 		    && it->pos > 0) {
 			InsetBibitem * olditem = static_cast<InsetBibitem *>(it->inset);
 			oldkey = olditem->getParam("key");
 			oldlabel = olditem->getParam("label");
-			eraseChar(it->pos, track_changes);
+			erasedInset = eraseChar(it->pos, track_changes);
+			break;
 	}
 
-	if (hasbibitem)
-		return false;
-
+	//There was an InsetBibitem at the beginning, and we didn't
+	//have to erase one.
+	if (hasbibitem && !erasedInset)
+			return 0;
+	
+	//There was an InsetBibitem at the beginning and we did have to 
+	//erase one. So we give its properties to the beginning inset.
+	if (hasbibitem) {
+		InsetBibitem * inset = 
+			static_cast<InsetBibitem *>(insetlist.begin()->inset);
+		if (!oldkey.empty())
+			inset->setParam("key", oldkey);
+		inset->setParam("label", oldlabel);
+		return -1;
+	}
+	
+	//There was no inset at the beginning, so we need to create one with
+	//the key and label of the one we erased.
 	InsetBibitem * inset(new InsetBibitem(InsetCommandParams("bibitem")));
 	// restore values of previously deleted item in this par.
 	if (!oldkey.empty())
 		inset->setParam("key", oldkey);
-	if (!oldlabel.empty())
-		inset->setParam("label", oldlabel);
+	inset->setParam("label", oldlabel);
 	insertInset(0, static_cast<Inset *>(inset),
 		    Change(track_changes ? Change::INSERTED : Change::UNCHANGED));
 
-	return true;
+	return 1;
 }
 
 } // namespace lyx
Index: TextMetrics.cpp
===================================================================
--- TextMetrics.cpp	(revision 18413)
+++ TextMetrics.cpp	(working copy)
@@ -190,10 +190,16 @@
 	main_text_ = (text_ == &buffer.text());
 	bool changed = false;
 
-	// FIXME: this has nothing to do here and is the reason why text_ is not
-	// const.
-	if (par.checkBiblio(buffer.params().trackChanges))
+	// FIXME This check ought to be done somewhere else. It is the reason 
+	// why text_ is not	const. But then, where else to do it?
+	// Well, how can you end up with either (a) a biblio environment that
+	// has no InsetBibitem or (b) a biblio environment with more than one
+	// InsetBibitem? 
+	int const moveCursor = par.checkBiblio(buffer.params().trackChanges);
+	if (moveCursor > 0)
 		const_cast<Cursor &>(bv_->cursor()).posRight();
+	else if (moveCursor < 0 )
+		const_cast<Cursor &>(bv_->cursor()).posLeft();
 
 	// Optimisation: this is used in the next two loops
 	// so better to calculate that once here.

Reply via email to