>>>>> "Juergen" == Juergen Spitzmueller <[EMAIL PROTECTED]> writes:

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

Very good. Let's start from there, then.

  --- 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);
  +     ///

If you make this method virtual, then in replaceRefsIfUnique below you will
not have to test on lyxCode. 

  +void InsetCommand::replaceContents(std::string const & from, string const & 
to)
  +{
  +     if (getContents() != from)
  +             return;
  +     setContents(to);
  +}
  +
  +

Very good.

  +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));
  +     }
  +}

I would have thought that the 'else' part is enough. Is it necessary
to handle the case where there is no ','?

  -void Buffer::changeRefsIfUnique(string const & from, string const & to)
  +void Buffer::changeRefsIfUnique(string const & from, string const & to, 
InsetBase::Code code)
   {

As it is, the method works only for CITE_CODE and REF_CODE. I would
add:

BOOST_ASSERT(code == InsetBase::CITE_CODE || code == InsetBase::REF_CODE);

  +     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);
  +

A question I had is "why is the IfUnique" part useful? What are the
use cases?

But the code above could be good enough for now.

  +     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);
                  }
          }
   }

With a virtual replaceContents, there would be no need to test
lyxCode() == InsetBase::CITE_CODE. 

We are progressing :)

JMarc

Reply via email to