first things first... > One thing before I move on to the code. Can I have your formal agreement > that you agree to licence your contributions under the terms of the GNU > General Public Licence (GPL) version 2 or later? It's enough to reply to > this mail. I'll add a reference to your "yes" to > http://www.lyx.org/about/blanket-permission.php >
Sure: I grant permission to license any and all contributions I've made to LyX under the Gnu GPL version 2 or later. > > working on this, i'm afraid i stumbled over a bug in the thesaurus > > controller (but didn't do enough research to be able to cure it): > > 1. highlight some word in the main window > > 2. open the thesaurus dialog, choose a synonym and click replace > > 3. choose another synonym, click replace again - it won't work > > > > i checked this with the qt frontend. same thing, so it's apparently in > > the controller's replace function... (didn't look any further into that, > > admittedly...) > > If you do look in the controller (I did for the first time just now!) > you'll see that oldstr_, the string you're replacing, is set when the > dialog is opened, in initialiseParams. > > Call replace(newstr). oldstr_ is replaced throughout your document. > Call replace(newstr2). There's no oldstr_ left to replace. > > I'd suppose that oldstr_ should be replaced by newstr at the end of the > first call to replace(newstr), but I'll leave it to you to think about > this further. > as far as i can see, only the current occurence of oldstr_ is replaced... assigning oldstr_ = newstr afterwards solves the problem if there is only one occurence of oldstr_, but is not sufficient if there are more than one: in (anon)::replace (lyxfind.C) -- which is indirectly called by LFUN_WORD_REPLACE -- ::find is called, so the next occurence of oldstr_ is selected automatically. there seems to have been a parameter 'bool once' in 1.3.6's LyxFind, but it looks like it has vanished since... as far as i can see, changing back the thesaurus's behavior would require such a parameter in the replace LFUN -- and i'm not sure if i'm in the position to solely decide about interface changes of that kind... other options i can think of are leaving the thesaurus's behavior this spellchecker-like way (but i doubt this is desirable) or calling the find LFUN with fw=false (which i'd consider a waste of time and resources). i don't know if there are any ready-to-use LFUNs (instead of LFUN_WORD_REPLACE, that is), though. > > is gtk support planned for 1.4.0? > > No. The frontend is clearly a work-in-progress. > > > and, one more question: is there a way to enable compilation for all > > three frontends? i used the qt ui/sources as a template, but seeing it > > in live action would have been quite useful in parallel. > > Here I use: > configure --with-version-suffix \ > --disable-stdlib-debug \ > --with-frontend='qt xforms gtk' \ > --with-qt-includes='/usr/include/qt3' > thanks, that helped a lot. > On to the code... > > I've never looked in GTK's Dialogs.C before. First thing I see when I do so > is that the code initially sets the ButtonController to be the XForms one > and then overwrites this in the creation of many of the individual > dialogs. I think that I'd move > > dialog->bc().view(new xformsBC(dialog->bc())); > > to those dialogs that actually need it... > done that for now. > By the way, your GThesaurus.patch has a CVS conflict in the ChangeLog. > That's those <<<<<<< .... ========== ... >>>>>>>> bits. CVS is unsure > what's going on here so adds the placeholders as pointers for you to > examine. > oops, seems i messed around with cvs update and diff a little too much... i'll take better care in the future, i promise :) > GThesaurus.h: > /** This class provides a GTK+ implementation of the FormSearch Dialog. > I don't think it does... > oops again. couldn't that damn clipboard or gedit be more intelligent (than me)? > You might add some more doxygen comments (start /// or /**) to describe > what individual functions do. Most are obvious I guess, but I'd need to > look at the implementation to guess what the three non-virtual functions > do. > ok, i added some, hopefully sufficient. > GThesaurus.C: > no real comments. > A stylistic one: use tab-indentation please. > What happens if you remove the applylock_ and readonly lines entirely? (I'm > unfamiliar with them although I do know the button controller code.) > > I don't believe that you need 'bc().refreshReadOnly();'. In face, I'd > remove applylock, readonly and bc().valid(false) entirely from doBuild. > Their settings should be set, if at all, in update(). > > Have a look in controllers/Dialogs.C to get a handle on the overall logic > behind the Controller-View split. Without this overview, you're fighting > with one arm behind your back and one eye closed. > > Well done! > thanks for the warm welcome. i hope i'll gradually require less attention, but at the moment it certainly helps a lot getting started... i hope i covered all your and John's suggestions and the whole thing uses the right controller methods now. applylock_ remains there -- i couldn't make up anything better to prevent the dialog from inserting text if none was selected when it was opened... bernie
GThesaurus.tgz
Description: application/compressed-tar
