Jean-Marc Lasgouttes wrote:
> Some comments:
>
>   -void Buffer::changeRefsIfUnique(string const & from, string const & to)
>   +void Buffer::changeRefsIfUnique(string const & from, string const & to,
> InsetBase::Code code)
>
> I do not like passing an inset code if it is not really generic (the
> codes are hardcoded later). If we follow this route, we should at
> least assert on code's value.

I don't understand.

>   +   if (code == InsetBase::CITE_CODE) {
>   +           vector<pair<string, string> > keys;
>   +           fillWithBibKeys(keys);
>   +           vector<pair<string, string> >::const_iterator bit  = 
> keys.begin();
>   +           vector<pair<string, string> >::const_iterator bend = keys.end();
>   +
>   +           for (; bit != bend; ++bit)
>   +                   labels.push_back(bit->first);
>
> I am sure there is a nice way of doing the counting without building
> additional lists, but we would need Lars' help for that :)

I'm all ears. Note though, that fillWithBibKeys also looks into child 
documents (like getLabelList), and this is what we want.

>   +   } else
>   +           getLabelList(labels);
>   +
>           if (lyx::count(labels.begin(), labels.end(), from) > 1)
>                   return;
>
> If we were able to make the code below truly generic, the code above
> could be done by two separate helper functions.

What does this buy?

>           ParIterator it = par_iterator_begin();
>           ParIterator end = par_iterator_end();
>
> Why not use an InsetIterator there? I know it was in the existing
> code, but it may be a good time to improve it.

OK. Done.

>   +                           if (contents == from) {
>                                           inset->setContents(to);
>                                           //inset->setButtonLabel();
>                                           changed_inset = true;
>   +                           } else if (code == InsetBase::CITE_CODE
>   +                                    && tokenPos(contents, ',', from) != 
> -1) {
>   +                                   vector<string> items = 
> getVectorFromString(contents);
>   +                                   replace(items.begin(), items.end(), 
> from, to);
>   +                                   
> inset->setContents(getStringFromVector(items));
>
> I'd rather have a InsetCommand::replaceContents(from, to) that is
> specialized in InsetCite. This way the code in the present method
> would be truly generic.

Good idea, actually. Done.
It would be easier if nextInset() would not return InsetBase, but the real 
inset. Any idea to simplify it further?

> Finally, there is a changed_inset variable that does not seem to be
> used (not your doing).

Yes, I'll remove that.

Attached is what I have so far. Please keep on commenting.

Jürgen 

> JMarc
Index: src/insets/insetlabel.C
===================================================================
--- src/insets/insetlabel.C	(Revision 15150)
+++ src/insets/insetlabel.C	(Arbeitskopie)
@@ -71,7 +71,7 @@
 		}
 		if (p.getContents() != params().getContents())
 			cur.bv().buffer()->changeRefsIfUnique(params().getContents(),
-						       p.getContents());
+						       p.getContents(), InsetBase::REF_CODE);
 		setParams(p);
 		break;
 	}
Index: src/insets/insetcommand.h
===================================================================
--- src/insets/insetcommand.h	(Revision 15150)
+++ src/insets/insetcommand.h	(Arbeitskopie)
@@ -72,6 +72,8 @@
                 p_.setContents(c);
         }
 	///
+	void replaceContents(std::string const & from, std::string const & to);
+	///
 	std::string const & getOptions() const { return p_.getOptions(); }
 	///
 	std::string const & getSecOptions() const { return p_.getSecOptions(); }
Index: src/insets/insetcite.h
===================================================================
--- src/insets/insetcite.h	(Revision 15150)
+++ src/insets/insetcite.h	(Arbeitskopie)
@@ -43,6 +43,8 @@
 		OutputParams const &) const;
 	///
 	void validate(LaTeXFeatures &) const;
+	///
+	void replaceContents(std::string const & from, std::string const & to);
 
 private:
 	virtual std::auto_ptr<InsetBase> doClone() const
Index: src/insets/insetbibitem.C
===================================================================
--- src/insets/insetbibitem.C	(Revision 15150)
+++ src/insets/insetbibitem.C	(Arbeitskopie)
@@ -61,10 +61,14 @@
 	case LFUN_INSET_MODIFY: {
 		InsetCommandParams p;
 		InsetCommandMailer::string2params("bibitem", cmd.argument, p);
-		if (!p.getCmdName().empty())
-			setParams(p);
-		else
+		if (p.getCmdName().empty()) {
 			cur.noUpdate();
+			break;
+		}
+		if (p.getContents() != params().getContents()) 
+			cur.bv().buffer()->changeRefsIfUnique(params().getContents(),
+						       p.getContents(), InsetBase::CITE_CODE);
+		setParams(p);
 		break;
 	}
 
Index: src/insets/insetcommand.C
===================================================================
--- src/insets/insetcommand.C	(Revision 15150)
+++ src/insets/insetcommand.C	(Arbeitskopie)
@@ -156,6 +156,14 @@
 }
 
 
+void InsetCommand::replaceContents(std::string const & from, string const & to)
+{
+	if (getContents() != from)
+		return;
+	setContents(to);
+}
+
+
 InsetCommandMailer::InsetCommandMailer(string const & name,
 				       InsetCommand & inset)
 	: name_(name), inset_(inset)
Index: src/insets/insetcite.C
===================================================================
--- src/insets/insetcite.C	(Revision 15150)
+++ src/insets/insetcite.C	(Arbeitskopie)
@@ -28,14 +28,19 @@
 
 #include <boost/filesystem/operations.hpp>
 
+#include <algorithm>
+
 using lyx::support::ascii_lowercase;
 using lyx::support::contains;
 using lyx::support::getVectorFromString;
+using lyx::support::getStringFromVector;
 using lyx::support::ltrim;
 using lyx::support::rtrim;
 using lyx::support::split;
+using lyx::support::tokenPos;
 
 using std::endl;
+using std::replace;
 using std::string;
 using std::ostream;
 using std::vector;
@@ -430,3 +435,16 @@
 		break;
 	}
 }
+
+
+void InsetCitation::replaceContents(string const & from, string const & to)
+{
+	if (getContents() == from)
+		setContents(to);
+	else if (tokenPos(getContents(), ',', from) != -1) {
+		vector<string> items = getVectorFromString(getContents());
+		replace(items.begin(), items.end(), from, to);
+		setContents(getStringFromVector(items));
+	}
+}
+
Index: src/mathed/math_hullinset.C
===================================================================
--- src/mathed/math_hullinset.C	(Revision 15150)
+++ src/mathed/math_hullinset.C	(Arbeitskopie)
@@ -1078,7 +1078,7 @@
 				numbered(r, true);
 			string old = label(r);
 			if (str != old) {
-				cur.bv().buffer()->changeRefsIfUnique(old, str);
+				cur.bv().buffer()->changeRefsIfUnique(old, str, InsetBase::REF_CODE);
 				label(r, str);
 			}
 			break;
Index: src/buffer.C
===================================================================
--- src/buffer.C	(Revision 15150)
+++ src/buffer.C	(Arbeitskopie)
@@ -53,6 +53,7 @@
 
 #include "insets/insetbibitem.h"
 #include "insets/insetbibtex.h"
+#include "insets/insetcite.h"
 #include "insets/insetinclude.h"
 #include "insets/insettext.h"
 
@@ -67,7 +68,7 @@
 #include "support/lyxalgo.h"
 #include "support/filetools.h"
 #include "support/fs_extras.h"
-# include "support/gzstream.h"
+#include "support/gzstream.h"
 #include "support/lyxlib.h"
 #include "support/os.h"
 #include "support/path.h"
@@ -1612,31 +1613,32 @@
 }
 
 
-void Buffer::changeRefsIfUnique(string const & from, string const & to)
+void Buffer::changeRefsIfUnique(string const & from, string const & to, InsetBase::Code code)
 {
 	// Check if the label 'from' appears more than once
 	vector<string> labels;
-	getLabelList(labels);
 
+	if (code == InsetBase::CITE_CODE) {
+		vector<pair<string, string> > keys;
+		fillWithBibKeys(keys);
+		vector<pair<string, string> >::const_iterator bit  = keys.begin();
+		vector<pair<string, string> >::const_iterator bend = keys.end();
+
+		for (; bit != bend; ++bit)
+			labels.push_back(bit->first);
+	} else
+		getLabelList(labels);
+
 	if (lyx::count(labels.begin(), labels.end(), from) > 1)
 		return;
 
-	InsetBase::Code code = InsetBase::REF_CODE;
-
-	ParIterator it = par_iterator_begin();
-	ParIterator end = par_iterator_end();
-	for ( ; it != end; ++it) {
-		bool changed_inset = false;
-		for (InsetList::iterator it2 = it->insetlist.begin();
-		     it2 != it->insetlist.end(); ++it2) {
-			if (it2->inset->lyxCode() == code) {
-				InsetCommand * inset = static_cast<InsetCommand *>(it2->inset);
-				if (inset->getContents() == from) {
-					inset->setContents(to);
-					//inset->setButtonLabel();
-					changed_inset = true;
-				}
-			}
+	for (InsetIterator it = inset_iterator_begin(inset()); it; ++it) {
+		if (it->lyxCode() == code && it->lyxCode() == InsetBase::CITE_CODE) {
+			InsetCitation & inset = dynamic_cast<InsetCitation &>(*it);
+			inset.replaceContents(from, to);
+		} else if (it->lyxCode() == code) {
+			InsetCommand & inset = dynamic_cast<InsetCommand &>(*it);
+			inset.replaceContents(from, to);
 		}
 	}
 }
Index: src/buffer.h
===================================================================
--- src/buffer.h	(Revision 15150)
+++ src/buffer.h	(Arbeitskopie)
@@ -343,7 +343,7 @@
 	///
 	StableDocIterator getAnchor() const { return anchor_; }
 	///
-	void changeRefsIfUnique(std::string const & from, std::string const & to);
+	void changeRefsIfUnique(std::string const & from, std::string const & to, InsetBase::Code code);
 
 private:
 	/** Inserts a file into a document

Reply via email to