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