[webkit-reviews] review denied: [Bug 48287] Refactoring: Spellchecking related static functions could form a class : [Attachment 71978] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 28 03:14:57 PDT 2010


Kent Tamura <tkent at chromium.org> has denied MORITA Hajime
<morrita at google.com>'s request for review:
Bug 48287: Refactoring: Spellchecking related static functions could form a
class
https://bugs.webkit.org/show_bug.cgi?id=48287

Attachment 71978: Patch
https://bugs.webkit.org/attachment.cgi?id=71978&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=71978&action=review

> WebCore/WebCore.xcodeproj/project.pbxproj:-21238
> -			developmentRegion = English;

Do not delete this line.  You need the latest Xcode to avoid the deletion.

> WebCore/editing/Editor.cpp:1661
> +    TextCheckingHelper checking(client(), spellingSearchRange);

The name "checking" looks curious to me.  Should it be "checkingHelper" or
"checker"?

> WebCore/editing/Editor.cpp:2092
> +    TextCheckingHelper checking(client(), searchRange);

ditto.

> WebCore/editing/Editor.cpp:2177
> +    TextCheckingHelper checking(client(), grammarRange);

ditto.

>> WebCore/editing/TextCheckingHelper.cpp:438
>> +{
> 
> Will it be possible to split this function into two one that checks grammar
and another that doesn't check grammar?

I think it will be done in the next step.

> WebCore/editing/TextCheckingHelper.h:38
> +    int findFirstGrammarDetail(const Vector<GrammarDetail>& grammarDetails,
int badGrammarPhraseLocation, int /*badGrammarPhraseLength*/, int startOffset,
int endOffset, bool markAll);

No need to comment out the argument name.


More information about the webkit-reviews mailing list