Am 01.09.2010 um 17:38 schrieb Jean-Marc LASGOUTTES:
> Stephan Witt <st.w...@gmx.net> writes:
>> I've made some changes again... shorten lines as Jürgen suggested and
>> formatting of if/while. And I added code to handle the soft-hyphens
>> and friends.
>>
>> So I attach it again and hope I can put it in later. JMarc, can you
>> have a look, please?
>
> See below. Nice work.
Thank you.
>
>> +int AppleSpellChecker::numMisspelledWords() const
>> +{
>> + return numMisspelledWordsAppleSpeller(d->speller);
>> +}
>
> It might be nice, short of having proper objc++ files, to rename the
> objc helper functions to something more obvious like
> AppleSpeller_numMisspelledWords (or even AS_numMisspelledWords).
Yes, you're right. I'll change the names.
>
>> --- src/Text2.cpp (Revision 35252)
>> +++ src/Text2.cpp (Arbeitskopie)
>> @@ -356,6 +356,9 @@
>> Font f = tm.displayFont(pit, pos);
>> f.update(font, language, toggleall);
>> setCharFont(pit, pos, f, tm.font_);
>> + // font change may change language...
>> + // spell checker has to know that
>> + pars_[pit].requestSpellCheck();
>> }
>> }
>>
>
>
>> - void setMisspelled(bool misspelled) { misspelled_ = misspelled; }
>> - ///
>> - bool isMisspelled() const { return misspelled_; }
>> - ///
>
> Yay!
>
>
>> Index: src/Paragraph.cpp
>> ===================================================================
>> --- src/Paragraph.cpp (Revision 35252)
>> +++ src/Paragraph.cpp (Arbeitskopie)
>> @@ -73,8 +73,107 @@
>> char_type const META_INSET = 0x200001;
>> };
>>
>> +
>> /////////////////////////////////////////////////////////////////////
>> //
>> +// Paragraph::SpellRanges
>> +//
>> +/////////////////////////////////////////////////////////////////////
>> +
>> +class Paragraph::SpellCheckerState {
>
> What is the real name? Does it need to be in Paragraph, or can it be in
> Paragraph::Private? Or can it even be toplevel?
It's all about two questions:
* does the paragraph need a check of spelling
* where are the error locations (ranges)
I thought, Paragraph::SpellCheckerState is private...
>
>> +private:
>> + /// store the ranges as map of FontSpan and spell result pairs
>> + typedef map<FontSpan, SpellChecker::Result> Ranges;
>> + typedef map<FontSpan, SpellChecker::Result>::const_iterator
>> RangesIterator;
>> + Ranges ranges_;
>
> typedef map<FontSpan, SpellChecker::Result> Ranges;
> typedef Ranges::const_iterator const_iterator;
>
>> + void eraseCoveredRanges(FontSpan const fp)
>
>> + void correctRangesAfterPos(pos_type pos, int offset)
>
> I do not see why you need that. After all, the spellchecking is done
> linearly in the paragraph, and thus all you have to do is to append to
> the Range object (actually, a list<> and some push_back would be
> enough).
My intention was to shift all positions to left or right on delete or insert of
a char/inset.
This especially will be useful, when it comes to real "spell check as you
type". Then not the whole paragraph
needs to be checked again - only the word at the current position. But I'm not
there...
As it is now, the flow of control is
* insert a char
* positions are corrected, paragraph spelling is marked dirty
* redraw paints the row and triggers the spell check
* the spell checker replaces the range items and
* finds the errors again at the already corrected positions in list
> I would have a std::list that lists the ranges for misspelled words.
> Wouldn't that be enough?
I'm not so sophisticated with STL containers. :(
At some point I use the find() operation and std::list does not have it.
I have to admit - I was too lazy to try this as I considered this myself.
If the general approach is ok I'll try an alternative implementation with
std::list.
>
>> + Language * locateSpellRange(
>> + pos_type & from, pos_type & to,
>> + Positions & softbreaks) const;
>
> Maybe:
> Language * locateSpellRange(pos_type & from, pos_type & to,
> Positions & softbreaks) const;
>
>> + void setMisspelled(pos_type from, pos_type to, SpellChecker::Result
>> state)
>
> As I wrote, this one is probably not needed either.
Why? It's in Paragraph::Private...
You mean the additional checks can go and I should call
speller_state_.setRange() instead?
>
>> @@ -297,6 +425,7 @@
>> // Add a new entry in the fontlist_.
>> fontlist_.set(fcit->pos() - beg, fcit->font());
>> }
>> + requestSpellCheck();
>> }
>
> I guess the question with this change and the others is whether you have
> covered all the possibilities for modifying the paragraph. Time will
> tell, I guess.
Yes, I hope I have covered all - but Murphy will have the last word.
And this is exactly the biggest risk is this business.
>
>> + while (last < to && !owner_->isWordSeparator(last)) {
>> + if (owner_->getInset(last)) {
>> + softbreaks.insert(softbreaks.end(), last);
>> + }
>> + ++last;
>> + }
>
> Can you explain what the softbreaks variable is for?
I called it softbreaks because I detected the problem with soft-hyphens and
it's the same with ligature-breaks.
They have their own position as insets but are "missing" in the result of
Paragraph::asString().
So one has to increment all positions in spell checker string matches by the
number of softbreaks before.
The assumption here is that every inset not being an word separator has the
"asString()" length of 0 and the real length of 1.
If that's not true I have to make the Positions type and the offset computation
more complex.
>
>> + /// start position and length of misspelled word at index
>> + virtual void misspelledWord(
>> + int index,
>> + int & start, int & length) const
>> + {
>> + /// index is used here to make the compiler happy
>> + if (index == 0)
>> + start = 0;
>> + length = 0;
>> + }
>> +
>
> Why not make this misspelledWord method pure virtual?
Because I didn't want to force every spell checker to implement that method.
As it is now only spell checker which "canCheckParagraph" needs to implement
that method.
Currently it is only the apple native one.
> BTW, If you want to avoid warnings about variables, all you have to do is:
>
> virtual void misspelledWord(int /*index*/, int & /*start*/,
> int & /*length*/) const
>
Ah. But I want to assign "length = 0" to be safe here. So I'll make only index
a comment.
Stephan
