[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-26 20:49:51
[Download RAW message or body]

On Tuesday 26 March 2002 19:50, till busch wrote:
> On Tuesday 26 March 2002 13:59, David Faure wrote:
> > ...
> > I think the current implementation does the job quite well, and
> > "if it ain't broke, don't fix it" stands ;)
> 
> you mean: if it was not broken! it is broken.
There might be a few bugs to iron out, but we were talking about the overall
_DESIGN_ in our previous discussion. Sorry, I should have been more clear,
it looks like we have a misunderstanding here. I meant that the method
ed, iterating over changed paragraphs and textobjects in the background
(i.e. using a timer) is a valid and working solution - I thought we agreed
that this was indeed ok...

> or does it undeline the correct things for you?
> does it work correctly? do the undelines appear on the right place?
> does it produce masses of such debugging output:
> QGArray::at: Absolute index 656 out of range.

Hmm, all of this worked the last time I used it.... Let's check again.
Argl. Something changed. I'm sure it has, since I get an abort() at the above
warning (helps debugging), and I noticed this change, but the initial code wasn't
doing that!
... debugging ....
kspell seems to have a bug.... quite a complex one.
It appends '\n' which leads to an off-by-one error
(lastline is set to position of first '\n' +1, but length is one more, so this test succeeds:
if ((unsigned int)lastline<origbuffer.length())
Due to that there are two lines to check, the second one being empty.
The bug seems to be that for the empty line, the mispelling result of the first line
is emitted again....
leading to completely wrong 'pos' values in the spellCheckerMispelling slot :(

My test is a single "abcd" word.

kword: KSpell::check2 lastline=5< origbuffer.length()=6
kspell (kdelibs): [EOL](0) setting lastlastline to 5
[...]
kspell (kdelibs): lastpos=posinline(0)+lastlastline(5)+offset(0)

kword: KSpell::check3 lastline=i=6 qs='
kword: '
kspell (kdelibs): KPIO::readln
kspell (kdelibs): KSpell::check2 (-1b) : line=
kspell (kdelibs): KPIO::readln
kspell (kdelibs): KSpell::check2 (-1b) : line=
kspell (kdelibs): Empty
kspell (kdelibs): KPIO::readln
kspell (kdelibs): KSpell::check2 (8b) : line=# abcd 1
kword: KSpell::parseOneResponse # abcd 1
kword: KSpell::parseOneResponse qs2=# abcd 1

Ouch - that has been already emitted. No idea why it gets emitted again.
Ok, due to lack of maintainance on KSpell (and ugliness of code there),
I think this calls for a KOffice version of kspell, for now.
A clean rewrite - of kspell - wouldn't hurt, that's for sure.

> does the underline flash up for half a second if you type a word?
Not sure what you mean here.... I don't remember seeing something like that.

> well, maybe my cpu is broken, or my screen fails to draws red pixels on the 
> screen (maybe it's the graphics board, i don't know).
Sounds like the vertical layout problems, rather. Try with another font for now,
we have rounding problems that lead to lines being truncated it seems.

> so i'll just resign from trying to contribute anything that would fix these 
> non existing errors in a part of kword, that "ain't broke".

We definitely misunderstood each other. I certainly don't want to refrain you from
working on KWord's spell-check. I was merely defending my design solution,
since I believed - and still believe - that it's a good and efficient one, suited for
the task. However it appears kspell isn't up to the task [although I really don't
understand why it initially worked just fine, before the code moved to kotext].

Can we combine our solutions then? To my eyes the best solution would be
your new simple-and-clean KoSpell with my simple-way-of-knowing-what-to-check
(the bool in the parag and the bool in the textobject).

What do you think?

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