<br><br><div><span class="gmail_quote">On 10/26/07, <b class="gmail_sendername">Zack Rusin</b> <<a href="mailto:zack@kde.org">zack@kde.org</a>> wrote:</span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

On Thursday 25 October 2007 12:51:51 pm Jordi wrote:<br>> Today 12:51:51 pm<br>>   <br>> Some cleanup of sonnet.<br>> Much more to be done before KDE 4, right now it is failing for me in<br>> some ways :/<br>

<br>Can you let me know what ways?</blockquote><div><br><br>The code as the current HEAD can not run test_dialog <br>That's why  I was playing with it and left the connections commented out (didn't mean to go to the patch)

The dialog didn't appear if I call it with show(); Calling exec () will appear but crash due a problem with libenchant.   Konqueror should be using sonnet, but I the menu does not show up.  </div> Other apps should be ported to sonnet and tested.

<br><br>I'm sure I will discover more :P<br><br><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">> I am really interested in sonnet, I want to make sonnet usable for KDE

<br>> 4 and add some features for KDE 4.1<br><br>Wicked! That's great to hear :)<br><br>Some comments and observations about the patch:<br><br>> Index: sonnet/plugins/aspell/kspell_aspelldict.cpp<br>> ===================================================================

<br>> --- sonnet/plugins/aspell/kspell_aspelldict.cpp (revision 729074)<br>> +++ sonnet/plugins/aspell/kspell_aspelldict.cpp (working copy)<br>> @@ -42,6 +42,7 @@<br>>      else<br>>          m_speller = to_aspell_speller( possible_err );

<br>><br>> +    name="aspell";<br>>  }<br><br>Why do you need the name of the speller in the dict itself? It's held in the<br>Client, so if for some reason you need it in the dict then it'd be better to

simply change the Loader to propagate it to the speller that it created.</blockquote><div> The loader will use the clients to create the dict. So the right place should be the clients code in the plugins themselves. I'll change it in the next patch.

<br><br> </div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">> Index: kdeui/sonnet/dialog.cpp<br>> ===================================================================

<br>> --- kdeui/sonnet/dialog.cpp     (revision 729263)<br>> +++ kdeui/sonnet/dialog.cpp     (working copy)<br>> @@ -72,8 +72,8 @@<br>>      d->checker = checker;<br>><br>>      initGui();<br>> -    initConnections();

<br>> -    setMainWidget( d->wdg );<br>> +//    initConnections();<br>> +    setMainWidget( d->wdg); //new QLabel("trusmis",this)); //d->wdg );<br>>  }<br><br>Any reason why you disabled the connections? Without them it won't work.

</blockquote><div><br>Sorry, I was testing and left this commented out. Should not be commented<br><br> </div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

>  void TestDialog::check( const QString& buffer )<br>>  {<br>> -    Sonnet::Dialog *dlg = new Sonnet::Dialog(<br>> +    dlg = new Sonnet::Dialog(<br>>          new BackgroundChecker(this), 0);<br>>      connect(dlg, SIGNAL(done(const QString&)),

<br>>              SLOT(doneChecking(const QString&)));<br>>      dlg->setBuffer(buffer);<br>> -    dlg->show();<br>> +    dlg->exec();<br>>  }<br><br>What's the reason for this change?</blockquote>

<div><br><br>The difference should be that exec makes the dialog modal by default, while show() will not IIRC. <br>See the  comment above about problems found.<br>Making the dialog modal will be an usability problem?<br><br>

</div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">> Index: kdecore/sonnet/speller.cpp<br>> ===================================================================

<br>> --- kdecore/sonnet/speller.cpp  (revision 729074)<br>> +++ kdecore/sonnet/speller.cpp  (working copy)<br>> @@ -49,18 +49,11 @@<br>><br>>          dict = loader->createSpeller(language);<br>>      }

<br>> -    bool isValid()<br>> -    {<br>> -        if (settings->modified()) {<br>> -            recreateDict();<br>> -            settings->setModified(false);<br>> -        }<br>> -        return dict;

<br>> -    }<br><br>I don't think the isValid changes really give us anything. Furthermore now<br>looking at it, recreating dictionary on changing of the default language<br>doesn't make sense (it won't change anything because the language used by the

<br>current speller has already been set). We should likely just stop even<br>calling recreateDict() in that case because I don't think that changing of<br>the "default language" should ever change the language of the currently used

speller. Same probably applies to the Client. In this case though we likely should have setClient() and client() methods that would act like setLanguage/language do.</blockquote><div> See that I delete the isValid() method. Previously the dict was recreated with every change and that includes adding a new word to the dictionary!

<br><br>You are right, any change to the current spelling language should be made by setLanguage so recreateDict() is not needed in setDefaultLanguage().  I guess that can be needed if you are spellcheking and in the middle of that you go to settings (old Kcontrol) and change the default spelling language. 

Yes, setting the client is nice, I'll include it in the next patch.   Now each method check if a dict could be created for this client/language. To avoid the check for every word, two solutions: -Create a dummy plugin that just return nothing that will be used if not dict is available, so we always have a valid dict.

<br>-Return an error to the user and stop spellchecking<br><br>comments will be appreciated <br></div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

> Index: kdecore/sonnet/loader.cpp<br>> ===================================================================<br>> --- kdecore/sonnet/loader.cpp   (revision 729074)<br>> +++ kdecore/sonnet/loader.cpp   (working copy)

<br>> @@ -74,7 +74,7 @@<br>>  {<br>>      kDebug()<<"Removing loader : "<< this;<br>>      d->plugins.clear();<br>> -    delete d->settings; d->settings = 0;<br>> +    delete d->settings;

<br>>      delete d;<br>>  }<br><br>Any particular reason for this change?</blockquote><div><br>we are deleting d, so we are deleting the pointer to settings, we don't need to set it to 0.<br> </div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

z<br></blockquote></div><br>