Bug 100513 – [EFL][WK2] Get rid of C'ism in text checker API

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Chris Dumez]

The current text checker code (ewk_text_checker and WebKitTextChecker) is very C like although the rest of our Ewk code has been ported to C++ style already. We need to port the text checker code as well for consistency.

[Chris Dumez]

Created attachment 170911 [details]
Patch

[Mikhail Pozdnyakov]

View in context: https://bugs.webkit.org/attachment.cgi?id=170911&action=review

> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:93
> +    Ewk_Text_Checker::initialize();

do not see any instance of Ewk_Text_Checker, maybe it could be just set of functions in a namespace?

> Source/WebKit2/UIProcess/API/efl/ewk_settings.cpp:241
> +    Vector<String> languages = Ewk_Text_Checker::availableSpellCheckingLanguages();

const ref here?

> Source/WebKit2/UIProcess/API/efl/ewk_text_checker.cpp:116
> +            WKRetainPtr<WKStringRef> suggestion(AdoptWK, WKStringCreateWithUTF8CString(static_cast<char*>(item)));

static_cast<const char*> ?

> Source/WebKit2/UIProcess/API/efl/ewk_text_checker.cpp:118
> +            free(item);

could we use OwnPtr<const char> as scope pointer here instead?

> Source/WebKit2/UIProcess/API/efl/ewk_text_checker.cpp:121
> +        Vector<String> guesses = textCheckerEnchant()->getGuessesForWord(toImpl(word)->string());

const ref?

[Chris Dumez]

Created attachment 170916 [details]
Patch

Take Mikhail's feedback into consideration. I'm using a namespace now.
Asking r? again since this is not a minor revision.

[Kenneth Rohde Christiansen]

Comment on attachment 170916 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=170916&action=review

> Source/WebKit2/UIProcess/API/efl/ewk_text_checker.cpp:211
> +
> +#define EWK_TEXT_CHECKER_CALLBACK_SET(TYPE_NAME, NAME)  \
> +void ewk_text_checker_##NAME##_cb_set(TYPE_NAME cb)     \
> +{                                                       \
> +    clientCallbacks().NAME = cb;      \
>  }

so these are all internal?

[Chris Dumez]

Comment on attachment 170916 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=170916&action=review

>> Source/WebKit2/UIProcess/API/efl/ewk_text_checker.cpp:211
>>  }
> 
> so these are all internal?

I'm not sure I understand the question. This preexisting macro is used to generate the implementation for the public C API.

[Kenneth Rohde Christiansen]

Comment on attachment 170916 [details]
Patch

Yeah sure. my mistake

[WebKit Review Bot]

Comment on attachment 170916 [details]
Patch

Clearing flags on attachment: 170916

Committed r132657: <http://trac.webkit.org/changeset/132657>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.