Allan> ###
Allan> The rest is mainly for Angus. I took a quick look at FormCitation.C and
Allan> noticed something which will be a problem when we move to having multiple
Allan> LyXViews (ie. multiple main windows):
Allan>  static vector<string> citekeys;
Allan>  ...
Allan> 
Allan> These 3 variables at file scope will be accessed by each instance of the
Allan> citation dialog (one per LyXView).  Unless they can really be shared they
Allan> should be made class members.  If they can be shared then they should
Allan> probably use a semaphore to indicate when someone is updating them.
Allan> 
Allan> I don't think any of these are shareable unless the two files in the
Allan> different windows use the same bibliography database (likely but not
Allan> guaranteed).
Allan> 
Allan> There are similar variables in FormToc.C.

Good point. I'll make the necessary changes.

Allan> ###
Allan> The code for setting minsize and maxsize of the dialogs is cute but should
Allan> be in build() not update() IMO.  Then you can get rid of the static ints
Allan> and still only do the initialization once: FormToc, FormIndex, FormUrl

This is true for FormToc, FormIndex, FormUrl but not for FormCitation where the
dialog height may change dependent on the size of the Bibliography database.

Again, I'll make the necessary changes.

Allan> ###
Allan> When checking if the contents of the InsetCommandParams of an inset will
Allan> change in {FormCitation,FormUrl}::apply() why not just use:
Allan>  if ( params != inset_->params() ) {
Allan>          ...
Allan>  }
Allan> 
Allan> instead of testing each thing separately.  You shouldn't need to define an
Allan> operator!= (IIRC) because the compiler should be able to synthesize its
Allan> own.  Writing one won't take long anyway.

Very good idea. (Again, I'll make the necessary changes.)

Incidentally, and in reply to an earlier email (some advice please), I think
that the Dispatch method I was suggesting for the TOC should definitely go in
BufferView, not Buffer. Something similar will exist for a LFUN_REF_GOTO call.
These things are view dependent, not content dependent.

And finally, it's good to have someone taking an active interest in this stuff
again. It's been a little lonely ;-(

Angus

Reply via email to