Am 30.08.2010 um 11:48 schrieb Jürgen Spitzmüller:
> Stephan Witt wrote:
>> Now the patch is ready for review.
>> It compiles and works for me on mac and linux.
Thank you for review.
> Not a thorough review, but on a first glance I see many style issues. For
> instance,
>
> + if (from >= to) return 0;
>
> should be
>
> + if (from >= to)
> + return 0;
Ok, I'll correct that.
>
> also, take care about line length (~75 chars),
Try to do so...
> and do not remove FIXMEs that
> are still valid such as this:
>
I removed one FIXME which is outdated and one that might be still valid.
Perhaps I should have mentioned this, but I don't think that this should be
customizable.
> - // Ignore words with digits
> - // FIXME: make this customizable
> - // (note that hunspell ignores words with digits by default)
> [...]
> + if (d->speller_state_.needs_refresh_ || check_learned) {
> + // Ignore words with digits
> + if (!hasDigit(word))
>
>> One quirk is not solved: soft-hyphens disturb the position count of words
>> inside paragraph :(
>
> Probably also other special char insets such as ligature break. BTW did you
> see #6870? This strikes me related.
Yes, I got the CC-mail but didn't check the ticket contents.
The problem is related indeed.
The position in par is +1 for InsetSpecialChar but the conversion to docstring
is empty.
I've addressed this in Paragraph::spellCheck() already when checking word-wise.
The code to detect the special char insets is rather ugly. Any proposal to bet
it better?
if (Inset const * inset = owner_->getInset(pos)) {
InsetCode code = inset->lyxCode();
if (code == SPECIALCHAR_CODE) {
InsetSpecialChar const * special = (InsetSpecialChar const *) inset;
if (special->kind() == InsetSpecialChar::HYPHENATION ||
special->kind() == InsetSpecialChar::LIGATURE_BREAK)
// do something
}
}
I would prefer to add some code to the Inset class at least, e. g.
asInsetSpecialChar() like asInsetText().
>
>> 3. getSpellLanguage() implementation
>> Shouldn't the lyxrc.spellchecker_alt_lang variable be accompanied by a real
>> Language instance?
>
> Yes, this rc is bogus. Hardcoding a general alternative spelling language in
> prefs makes LyX unnecessarily monolingual. We should either allow alternative
> spelling languages for each language in prefs (where the alternative is only
> used if the respective companion language is requested) or we should move the
> setting to the document level. But I'd prefer the former approach, since
> documents are not necessarily monolingual either.
So I'll leave this code unchanged (as it is currently) and try to change that
later.
Stephan
