[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: [patch] kword spell-checking
From:       David Faure <david () mandrakesoft ! com>
Date:       2002-03-25 22:27:38
[Download RAW message or body]

On Monday 25 March 2002 22:36, till busch wrote:
> >  - but the initial method did this by simply looking at "what's the next
> > text object [frameset, in kword] to be checked". That way is more dynamic
> > than a queue or a list, which means that when deleting a text object we
> > don't need to look in the queue or list to see if it's still there
> > (dangling pointer if it is!). We only need to check if it's the current
> > object being checked, but that's all.
> 
> this is a real advantage of the initial method. so i guess i can set it up 
> again. though the idea of checking multiple things at the same time seems 
> necessary to me.

Multiple things at the same time? I don't see how this is possible.
When ispell is working on something, you have to wait for it to finish with
that before asking it to check something else. Unless you want to start
multiple ispell processes at the same time? I don't think this is necessary.

> > What was missing in kspell that makes it necessary to have kospell?
> > I'm not opposed to the idea, but since I fixed kspell as I thought needed,
> > I'm wondering what's still missing there.
> 
> well. especially kspell didn't really work in non-blocking mode.
It's non-blocking, no problem there. I think you mean it didn't allow
multiple queries in parallel....

> it couldn't emit a done() for each check() -- at least not in the right order. 
Yup, that's why we can just wait for a done() before the next check()
 - what's the problem with that?

> not, if you sent multiple check()'s without waiting for the done() inbetween.
> there are other (less striking) problems with the current kspell 
> implementation. eg: checkWord() produces tons of errors and just breaks after 
> some checks.
Ah, that's quite possible, I'm not sure who uses checkWord (didn't need it
in the initial implementation).

> also spell-as-you-type doesn't need dialogs, nor suggestions 
> which i believe cause (at least some) overhead in kspell. (parsing the line, 
> creating a QStringList [especially the way this is done -- without split()]).

Right about no need for a dialog, this should be fixed in kspell, skipping the
dialog code when we don't want a dialog. The current implementation of
"no dialog" is really just a quick hack.
I don't agree about no need for suggestions though. We might want them
in the RMB popupmenu. Hmm, ok, we might want to do a checkWord()
when opening the RMB instead of storing all suggestions in memory......
Ok... this would be better implemented as a kspell bool in the future,
saying "don't bother with suggestions". I'm not very happy with a fork
of kspell, you see - forks always suck, one has to duplicate bugfixes,
run diffs to check if all bugfixes are in sync etc. But if we have no other way
for now, we'll have to fork indeed.
If the only reasons for forking are performance improvements (such as
those you cite), then I don't think it's worth it. Better fix in kdelibs,
and users will get the improvements when upgrading kdelibs, but will
still get a working feature with kdelibs-3.0, so no problem. Better that than
creating a maintainance headache.

> > Design-wise I like the use of the paragraph visitor class there.... but I
> > think the concepts of "what's the next object to check" (note the bool that
> > was added to skip unchanged framesets etc.) and "wait a bit until checking
> > the next object" should remain. Unless you found flaws with those concepts
> > ? Please elaborate, we can't make a decision at this point without a
> > comparison of the two methods ;)
> 
> ok, i like the idea, though i didn't like the implementation that "looped" 
> (with QTimers) forever, i prefer calling something like checkDocument() once 
> when loading, and then following any changes in the document. i think this is 
> more logical then checking, checking, checking, checking..

A QTimer whose slot quickly realizes that it has nothing to do, is quite cheap.
It's not "checking, checking" as in "sending text to ispell" everytime: as long
as the text hasn't changed, no textobject will be marked as "to be checked"
so it'll just return. This can then be optimized easily anyway, by stopping the timer
when there's nothing to check, and restarting it when marking a textobject
as "to be checked". Such stuff can easily be added to such a design.

> so.. i can easily readd the QTimer concept for the initial check of the 
> document, still i would very much prefer to follow modifications in the 
> document directly. (which already works for typing - in my patch)
Yes, I considered doing a "check the last word that was typed", but then
this misses pasting, dropping text, etc. and it might slow down typing
unless some delaying happens (if words are checked one by one).
Marking paragraphs as "dirty" whenever their text was changed, and then
checking the dirty paragraphs (in the dirty textobjects ;) sounded simpler
to me, since it works for loading text, typing, pasting, undoing, etc. etc.
If the hooks have to be less lowlevel (than kotextparag::insert) then it's
much easier to forget a place where a call should be added....
Both approaches would work of course, but IMHO unless a problem is found
with the "simpler" method, there's no particular reason to go for a more
complex method ;)

> i'll send you a crypt()-ed password (to your personal address) for cvs-acces, 
> but i still won't commit anything until everything is clear :)

Okay ;)

-- 
David FAURE, david@mandrakesoft.com, faure@kde.org
http://people.mandrakesoft.com/~david/, http://www.konqueror.org/
KDE, Making The Future of Computing Available Today
_______________________________________________
koffice-devel mailing list
koffice-devel@mail.kde.org
http://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread]