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

List:       koffice-devel
Subject:    Re: [patch] kword spell-checking
From:       till busch <buti () gmx ! at>
Date:       2002-03-26 11:04:43
[Download RAW message or body]

On Monday 25 March 2002 23:27, David Faure wrote:
> 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.

this is as i described below. you can send as much as you want to ispell. it 
will check the things you sent it, and give you the output as soon as it 
checked something. when it is done checking one line, the ispell process 
prints and empty newline to stdout, which says that ispell is done checking 
one line. so the deal is: you can send as much as you want, as long as you 
keep track of the order. ispell is designed that way and kspell should 
support this, and it does not. well, my kospell is a very limited version of 
kspell, i wouldn't say it's a fork at all.

> > > 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?

there is always (well, mostly) a work-around for bugs (glitches). we can wait, 
when looping around each paragraph in all KTextFrameSet (which imho can 
become a very large number). consider a 200-pages document with 20 parags 
each, that's 4000 paragraphs. now the user types one letter, the bgspellcheck 
has to run around 3999 paragraphs until it gets to the one which 
needSpellCheck(). ok, i admit you could set the nextParagraphToCheck to a 
value that makes sense, though if two paragraphs change quickly enough, you 
will miss one change, or even setMisspelled() the wrong one (without a 
queue).

> > 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.

well, i'd really prefer to fix kspell. the real problem is that kde release is 
too near to make changes that affect the behaviour of the lib. and it must be 
done. kspell does not at all fullfill what it says in its documentation. the 
code is not clean/readable at all either.

> > 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.

and if it's got to fire 3999 times just to find one paragraph it actually has 
to check, you can't tell me that this is a good implementation. is it?

> > 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 guess these hooks really should be added to kword. it should at least emit 
some signal if a paragraph changes.
as far as i can tell it doesn't slow things down at all. it's just sending one 
word of text to the pipe. we don't wait for an answer, when it comes, we 
apply possible changes. the other way is to check if any of the 4000 
paragraphs needsSpellCheck() and then send the whole paragraph to ispell - 
and i don't believe this is any faster.

ok. i shall summarize how i want the spellchecker to work:
- check the wole document when loaded (maybe it's good to delay things a bit 	         
here, so we don't take cpu time the user needs for input -- so yes, the 
initial implementation makes more sense here)
- check on each modification (targeted at the actual change, no "background 
looping" around 4000 paragraphs). i think it should be easy to add hooks in 
KoTextObject, since everything that concerns text is done there. (as far as i 
can tell now). this needs a kspell that correctly emits a done() for each 
check().

well. that's it. i hope you understand the points i wanted to make. and 
consider the 4000-paragraphs thing a stupid example (ony there to show what i 
mean).

greetings
- till

p.s. i didn't see you on #kde in the past days.. would be nice to discuss 
things there.
p.p.s. i don't want to flame anyone, i'm just looking for a good solution.

_______________________________________________
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]