Am 14.09.2010 um 09:42 schrieb Abdelrazak Younes:
> On 09/14/2010 09:30 AM, Stephan Witt wrote:
>> Am 14.09.2010 um 08:32 schrieb Abdelrazak Younes:
>>
>>
>>> On 09/14/2010 08:06 AM, Stephan Witt wrote:
>>>
>>>> Am 13.09.2010 um 23:16 schrieb Pavel Sanda:
>>>>
>>>>
>>>>
>>>>> Stephan Witt wrote:
>>>>>
>>>>>
>>>>>> Yes. I think so, too. Perhaps you can wait another two days with alpha6
>>>>>> until
>>>>>> the developers on platforms I do not have (Windows and Cygwin) have
>>>>>> verified
>>>>>> that it works at the first glance at least.
>>>>>>
>>>>>>
>>>>> of course i'm not going to commit a6 right after this patch ;)
>>>>>
>>>>>
>>>> Ok, I did it.
>>>>
>>>> Known things are for now:
>>>> * when last word inside a table is misspelled
>>>> the ignore button of spell checker dialog sometimes doesn't move out of
>>>> the table
>>>> * spell checker marks hidden by change tracking marks
>>>>
>>>>
>>> * Too many nesting levels :-P
>>>
>> What do you mean?
>>
>
> Things like:
>
> for ()
> if ()
> while ()
> if ()
This I don't like too. But sometimes I do it to avoid another private method.
>
>>
>>> Remember: return early, continue early. Please simplify that.
>>>
>> Where exactly? I changed to the "return at end" paradigma in LyX.cpp because
>> of the need to place common code at end.
>> Did you refer to that?
>>
>
> No, for example:
>
> +void Paragraph::Private::markMisspelledWords(
> + pos_type const& first, pos_type const& last,
> + SpellChecker::Result result,
> + docstring const& word,
> + Positions const& softbreaks)
> +{
> + pos_type snext = first;
> + if (SpellChecker::misspelled(result)) {
>
> Please continue instead:
>
> + if (!SpellChecker::misspelled(result))
> continue;
>
> And unindent the rest.
>
>
>>> The new code seems quite long and complicated to ingurgitate (which was
>>> actually the reason why nobody found the will nor the energy to review it).
>>>
>>> * It seems to me that the new SpellCheckerState could share a lot of code
>>> with the font stuff... couldn't we clean that up a bit?
>>>
>> Yes. I've planned that. But as I tried to stay with the misspelled flag in
>> font I've learned that the font span stuff is fragile. When I called the old
>> FontList::setMisspelled() method with wide ranges (spanning multiple words)
>> LyX crashed suddenly when collecting language items for menus. I couldn't
>> find the reason for that as the call stack of the crash was out of sync.
>> Debugging with memory guards didn't help either.
>>
>> It would be nice if you can give some hint how the cleanup should be done...
>>
>
> I was thinking of a base class inherited by FontSpan (not sure of the name)
> and SpellCheckerState. Or just some helper template functions... I think this
> could be also used for TrackChanges and character styles.
>
>> Some simple refactoring I did already - but didn't want to commit as I
>> didn't send it to list for review.
>>
>
> Simple refactoring/cleanup of 10 or 20 lines, you can commit straight away
> unless Pavel declares a freeze period.
I'll do that later. I don't expect any... but in case of compiler errors on any
platform they should be corrected before.
Stephan
