[PATCH] sonnet cleanup patch 1

[Zack Rusin]

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

Can you let me know what ways?

> I am really interested in sonnet, I want to make sonnet usable for KDE
> 4 and  add some features for KDE 4.1

Wicked! That's great to hear :)

Some comments and observations about the patch:

> Index: sonnet/plugins/aspell/kspell_aspelldict.cpp
> ===================================================================
> --- sonnet/plugins/aspell/kspell_aspelldict.cpp (revision 729074)
> +++ sonnet/plugins/aspell/kspell_aspelldict.cpp (working copy)
> @@ -42,6 +42,7 @@
>      else
>          m_speller = to_aspell_speller( possible_err );
>
> +    name="aspell";
>  }

Why do you need the name of the speller in the dict itself? It's held in the 
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.

> Index: kdeui/sonnet/dialog.cpp
> ===================================================================
> --- kdeui/sonnet/dialog.cpp     (revision 729263)
> +++ kdeui/sonnet/dialog.cpp     (working copy)
> @@ -72,8 +72,8 @@
>      d->checker = checker;
>
>      initGui();
> -    initConnections();
> -    setMainWidget( d->wdg );
> +//    initConnections();
> +    setMainWidget( d->wdg); //new QLabel("trusmis",this)); //d->wdg );
>  }

Any reason why you disabled the connections? Without them it won't work.

>  void TestDialog::check( const QString& buffer )
>  {
> -    Sonnet::Dialog *dlg = new Sonnet::Dialog(
> +    dlg = new Sonnet::Dialog(
>          new BackgroundChecker(this), 0);
>      connect(dlg, SIGNAL(done(const QString&)),
>              SLOT(doneChecking(const QString&)));
>      dlg->setBuffer(buffer);
> -    dlg->show();
> +    dlg->exec();
>  }

What's the reason for this change?


> Index: kdecore/sonnet/speller.cpp
> ===================================================================
> --- kdecore/sonnet/speller.cpp  (revision 729074)
> +++ kdecore/sonnet/speller.cpp  (working copy)
> @@ -49,18 +49,11 @@
>
>          dict = loader->createSpeller(language);
>      }
> -    bool isValid()
> -    {
> -        if (settings->modified()) {
> -            recreateDict();
> -            settings->setModified(false);
> -        }
> -        return dict;
> -    }

I don't think the isValid changes really give us anything. Furthermore now 
looking at it, recreating dictionary on changing of the default language 
doesn't make sense (it won't change anything because the language used by the 
current speller has already been set). We should likely just stop even 
calling recreateDict() in that case because I don't think that changing of 
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.

> Index: kdecore/sonnet/loader.cpp
> ===================================================================
> --- kdecore/sonnet/loader.cpp   (revision 729074)
> +++ kdecore/sonnet/loader.cpp   (working copy)
> @@ -74,7 +74,7 @@
>  {
>      kDebug()<<"Removing loader : "<< this;
>      d->plugins.clear();
> -    delete d->settings; d->settings = 0;
> +    delete d->settings;
>      delete d;
>  }

Any particular reason for this change?

z